Discussion :

[oprian]

Bonjour,

Le programme dont les sources sont ci-dessous s'arrête de façon inattendue, mais je n'arrive pas à comprendre pourquoi.

Code main.cpp :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
#include <iostream>
#include <vector>
 
#include "B.hpp"
#include "A.hpp"
 
 
using namespace std;
 
 
int main(){
 
    vector<A> listeDeA;
 
    int nbA=10;
 
    // We create the agents
    for(int i=0;i<nbA;i++){
        cout << "-------- Creation of the A number " << i << "/" << nbA-1 << " --------" << endl;
        A a;
        cout << "1" << endl;
        listeDeA.push_back(a);
        cout << "2" << endl << endl;
    }
    cout << "OK" << endl;
 
    return EXIT_SUCCESS;
 
}

Code A.hpp :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
#ifndef _A_HPP_
#define _A_HPP_
 
 
class A{
 
    public:
 
        B* b;
 
        //////////
 
        A();
        ~A();
 
};
 
 
#endif

Code A.cpp :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
#include <iostream>
 
#include "B.hpp"
#include "A.hpp"
 
using namespace std;
 
 
A::A(){
    cout << "In the A constructor" << endl;
    b=new B();
}
 
 
A::~A(){
    cout << "In the A destructor" << endl;
    if(b!=NULL){
        delete b;
    }
}

Code B.hpp :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
#ifndef _B_HPP_
#define _B_HPP_
 
class B{
 
    private:
 
        int *n1;
        float **n2;
        float **n3;
        float ***n4;
 
    public:
 
        B();
        ~B();
 
};
 
 
 
 
#endif

Code B.cpp :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
#include <iostream>
#include "B.hpp"
 
 
using namespace std;
 
 
 
B::B(){
    cout << "In the B constructor (" << this << ")" << endl;
}
 
 
B::~B(){
    cout << "In the B destructor (" << this << ")" << endl;
}

Lorsque je lance le programme, il s'arrête à la 7e itération du for de main.cpp.

Le code retourné est -1073741819.

Merci de votre aide !

[ram-0000]

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
 
A a;
listeDeA.push_back(a);

Dans la 1ere ligne, tu créés un objet A et dans le constructeur de A, tu alloues un objet B ==> OK

Dans la 2eme ligne, tu copies l'objet A dans la liste et pour cela, tu utilises le constructeur par copie de A. Tu n'as pas créé de constructeur de copie explicite donc le compilateur utilise le constructeur de copie par défaut qui copie les variables membres.

Donc tu te retrouves avec 2 objets, la variable locale "a" et la liste qui pointe sur le même objet B. Forcément cela va poser un problème lorsque tu vas détruire la variable "a" puis ensuite la liste, tu vas avoir un double delete du même objet.

[oprian]

Merci de votre réponse, mais pour éviter le double delete j'ai bien écrit :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
if(b!=NULL){
        delete b;
    }

[ram-0000]

Non, tu as 2 objets, ta variable locale "a" et dans ta liste qui pointent vers la même variable B.

Donc lorsque tu détruis les 2 variables, tu as bien un double delete.

[JolyLoic]

Par ailleurs, écrire :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
if(b!=NULL){
        delete b;
    }

Est inutile : Si b vaut NULL, delete b; est une expression bien valide, au comportement défini, et qui ne fait rien.

[oprian]

Par ailleurs, écrire :
Code :

if(b!=NULL){
delete b;
}


Est inutile : Si b vaut NULL, delete b; est une expression bien valide, au comportement défini, et qui ne fait rien.

Ah, merci de me le signaler ! C'est bien ce que je pensais avant que quelqu'un me dise le contraire !

J'ai compris pour l'histoire du double delete.

Dans mon cas, comment éviter cela ?

Si je fais :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

listeDeA.push_back(A());

La même chose se produit (car c'est un temporaire anonyme)...

Suis-je obligé d'avoir un vector de pointeurs ?

[koala01]

Salut, et bienvenue sur le forum

C'est tout le problème du constructeur de copie fourni par le compilateur (et qui est utilisé)

Je vais considérer la variable a comme étant celle que tu crées toi même et la variable a' comme étant la copie créé en utilisant le constructeur par copie créé par le compilateur:

a alloue la mémoire pour son membre b...

On obtiens donc a.b = 0x784520 (par exemple... c'est l'adresse qui a été trouvée lors de l'allocation )

quand a' est créé, c'est une copie "membre à membre" qui est effectuée...

Comme un pointeur n'est jamais que "l'adresse à laquelle on va retrouver l'instance d'un objet", lorsque a'.b est créé, l'adresse est tout simplement recopiée

on a donc a'b = 0x784520

comme a' est une variable tout à fait indépendante de a, quand a est détruite (parce que l'on sort de la portée dans laquelle elle a été déclarée), le test if(b!=NULL) est bel et bien vérifié (a.b ==...0x784520 et ne vaut donc pas NULL )

Pour ton malheur, quand il s'agit de détruire l'instance a', le test if (b!=NULL) est également vérifié (vu que a'.b ==...0x784520 ... aussi)...

Seulement, voilà... l'adresse qui avait été allouée dynamiquement en 0x784520, elle a déjà été libérée dynamiquement lors de la destruction de a ...

Et tu as donc bel et bien une double libération de la mémoire .

Pour éviter ce fait, il existe plusieurs solutions (classées par ordre de préférence)

1. éviter l'utilisation de pointeur lorsque ce n'est pas nécessaire...: finalement, pourquoi ne pas, tout simplement, déclarer le membre b comme... un objet tout à fait normal ainsi, chaque instance de A aura son propre membre, et il n'y aura plus de risque de double libération
2. Si ce n'est pas possible, veiller à ce que la durée de vie du membre b ne dépende pas de l'instance de A: donner la responsabilité de la destruction du membre à une classe tierce (principe des patterns observer et mediator, par exemple),
3. (ex-aequo avec (2) )Interdire l'utilisation du constructeur par copie en le déclarant (sans le définir) dans l'accessibilité private:... il faut alors aussi prévoir de faire pareil avec l'opérateur d'affectation
4. Définir le constructeur par copie pour ta classe A, en veillant à ce que le membre b de la copie ne pointe pas sur le membre b de l'original (nécessite l'allocation dynamique d'un nouveau membre pour la copie )... Mais prévoir également de définir l'opérateur d'affectation

Dans l'ordre d'esprit du (1), il faut signaler que le problème est également présent avec les pointeur de pointeur (de pointeur (de pointeur) )

[oprian]

Pour éviter ce fait, il existe plusieurs solutions (classées par ordre de préférence)

1. éviter l'utilisation de pointeur lorsque ce n'est pas nécessaire...: finalement, pourquoi ne pas, tout simplement, déclarer le membre b comme... un objet tout à fait normal ainsi, chaque instance de A aura son propre membre, et il n'y aura plus de risque de double libération
2. Si ce n'est pas possible, veiller à ce que la durée de vie du membre b ne dépende pas de l'instance de A: donner la responsabilité de la destruction du membre à une classe tierce (principe des patterns observer et mediator, par exemple),
3. (ex-aequo avec (2) )Interdire l'utilisation du constructeur par copie en le déclarant (sans le définir) dans l'accessibilité private:... il faut alors aussi prévoir de faire pareil avec l'opérateur d'affectation
4. Définir le constructeur par copie pour ta classe A, en veillant à ce que le membre b de la copie ne pointe pas sur le membre b de l'original (nécessite l'allocation dynamique d'un nouveau membre pour la copie )... Mais prévoir également de définir l'opérateur d'affectation

Dans l'ordre d'esprit du (1), il faut signaler que le problème est également présent avec les pointeur de pointeur (de pointeur (de pointeur) )

Merci, c'est exactement ce que je cherchais

finalement, pourquoi ne pas, tout simplement, déclarer le membre b comme... un objet tout à fait normal

Dans mon cas, c'est pour avoir un truc plus propre, n'allouer la mémoire que lorsqu'on en a besoin (dans le code duquel est parti mon problème bien sûr).

J'ajouterais à ta liste de solutions, bien que ce soit moins élégant, l'utilisation de listeDeA comme un vector<A*> :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
 
for(int i=0;i<nbA;i++){
   listeDeA.push_back(new A());
}

Merci à tous