Discussion :

[CSIE_Angel#5]

Bonjoru tout le monde.

Dans un programme plus vaste, j'ai un problème avec un constructeur/desctructeur. Le crash vient au moment de libérer la mémoire quand tous les destructeurs sont appelés en chaine.

(je ne mets pas tous les include pour na pas polluer, include "string.h", include des classes.h)

Code :

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

1
2
3
4
5
6
7
8
9
10
11
 
//Programme principale
void main()
{
    CTestB loc_test_b = CTestB();
    {
        CTest loc_test2 = CTest(loc_test_b);
 
        //delete loc_test2; test effectué en faisant un new a la ligne précédente
    }
};

Classe principale encapsulant un sous classe B

Code :

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
 
class CTest
{
public:
    CTest();
    CTest(CTestB&);
    ~CTest();
private:
    CTestB m_test_B;
};
 
//Code
CTest::CTest()
{
    m_test_B = CTestB();
}
 
CTest::CTest(CTestB& par_test_b)
{
    m_test_B = CTestB(par_test_b);
}
 
CTest::~CTest()
{
    //delete m_test_B;
}

Contenu de la classe b :

Code :

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
30
31
32
33
34
35
36
 
class CTestB
{
public:
    CTestB();
    ~CTestB();
 
    CTestB(const CTestB& par_cls_testB);
    char* mGetAddr() const {return addr;};
private:
    char *addr;
 
};
 
//CODE
CTestB::CTestB()
{
    addr = new char[20];
    strncpy(addr, "init", 5);
}
 
CTestB::CTestB(const CTestB& par_cls_testB)
{
    addr = new char[20];
    strncpy(addr, par_cls_testB.mGetAddr(), 5);
    strncat(addr, "copy", 5);
}
 
CTestB::~CTestB()
{
    if(addr != NULL)
    {
        delete addr;
        addr = NULL;
    }
}

Le problème survient au delete addr. Ce pointeur a déjà été libéré ... Grrr, l'erreur doit être basique mais je m'y perd.

Quelqu'un voit il mieux que moi ?

[screetch]

regle des 3 :
si tu as besoin d'un detructeur non trivial, ou bien d'un constructeur de copie non trivial, ou bien d'un opérateur d'affectation non trivial, alors tu as surement besoin des 3

il te manque l'opérateur =

[Arzar]

Pour plus d'info sur la règle des trois/forme orthodoxe de Coplien/Règle des Big Four, voir aussi la faq

[CSIE_Angel#5]

Merci beaucoup mais comme a chaque fois que je repose a plat mon problème je trouve souvent la solution...

Est ce que la suite est vrai ?

En sortie du constructeur CTest, la donnée m_Test_B qui est locale est détruite. Le pointeur addr est donc désalloué.
Ensuite dans le programme principale, quand on sort du bloc où CTest est construit(juste après le }), le destructeur est automatiquement appelé. Le destructeur de CTest appelle le destructeur de CTestB qui tente de désalloué une nouvelle fois addr.

J'ai donc changer CTestB m_test_B en CTestB* m_test_B et fait les allocation correspondantes.

[3DArchi]

En sortie du constructeur CTest, la donnée m_Test_B qui est locale est détruite. Le pointeur addr est donc désalloué.

m_Test_B est membre de CTest. Donc m_Test_B ne sera détruit que quand l'objet CTest sera détruit et non en sortie du contructeur.

Ensuite dans le programme principale, quand on sort du bloc où CTest est construit(juste après le }), le destructeur est automatiquement appelé. Le destructeur de CTest appelle le destructeur de CTestB qui tente de désalloué une nouvelle fois addr.

Cf commentaires dans le code :

Code :

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

1
2
3
4
5
    CTestB loc_test_b = CTestB(); // Première instance
    {
        CTest loc_test2 = CTest(loc_test_b); // Copie de loc_test_b dans loc_test2.m_Test_B 
    } // destruction de loc_test2 qui détruit m_Test_B qui libère addr 
// sortie du main : destruction de loc_test_b qui libère (une seconde fois) addr

J'ai donc changer CTestB m_test_B en CTestB* m_test_B et fait les allocation correspondantes.

Définir correctement l'opérateur = et le constructeur par copie sont une meilleur solution que de réintroduire un pointeur qui te provoquera le même genre de problème plus tard. Cf F.A.Q Comment gérer proprement des allocations / désallocations de ressources ? Le RAII !, les entrées de la F.A.Q proposées par Arzar ci-dessus et les Pointeurs intelligents.

[screetch]

non, m_test_B n'est pas locale, elle appartient a (this) et (this) vit encore.

cette ligne:

Code :

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

m_test_B = CTestB();

ne fait pas ce que tu penses.
Elle crée un objet temporaire TestB (grace au contructeur par defaut),
puis appelle l'opérateur d'affectation pour ecraser le contenu de m_test_B, puis efface le temporaire qui n'est plus utilisé.

Ce qui se passe c'est que tu appelles l'opérateur =. or tu n'as pas ecrit cet opérateur; son implémentation par défaut va etre de copier le contenu de TestB, membre a membre, donc de copier le pointeur contenant les données. On va donc avoir deux objets qui pointent vers la meme donnée; lorsque le temporaire est detruit, son destructeur va liberer la mémoire.

[JolyLoic]

J'ai donc changer CTestB m_test_B en CTestB* m_test_B et fait les allocation correspondantes.

C'est vrai, mais tu as une solution incomplète, et peut-être même erronée.

En fait, comme on te l'a indiqué, le problème fondamental vient du fait que quand tu copie un objet de type CTestB, ça se passe mal, car la copie n'est pas gérée (et tu te retrouves avec deux pointeurs à posséder la même zone mémoire, qui vont donc tous deux la libérer => boom).

Tu as temporairement corrigé ce problème, en ne faisant plus de copie de l'objet dans ton programme, mais ce problème existe encore, et ne demande qu'à refaire surface.

Tu as globalement deux solutions à ça :
- Soit ça a du sens de copier CTestB, et tu lui fourni un constructeur de copie (la règle des 3 citée dans la FAQ), après que fait ce constructeur ? A priori il recopie le buffer. Si ça a du sens que plusieurs copies de CTestB se partagent la même zone mémoire, tu remplaces le pointeur sur cette zone par un pointeur intelligent qui gère le partage (et du coup, plus besoin de destructeur, ni de constructeur de copie explicite).

- Soit ça n'a pas de sens de copier CTestB, et dans ce cas, il vaut mieux interdire la copie, afin d'éviter qu'elle ait lieu par inadvertance comme c'était le cas dans ton code : Tu déclares le constructeur de copie en privé, et tu ne l'implémente même pas.

[CSIE_Angel#5]

Ok, je vais prendre en compte la remarque des 3 !

mais une question en suivant :
quand j'écris et que j'ai défini un constructeur par copie et l'opérateur = :

Code :

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

m_test_B = CTestB();

Quelles sont les fonctions réellement appelées :
1°) Appel au constructeur par défaut CTestB
2°) puis appel de operator= (dans lequel je ferais certainement une copie du contenu du pointeur addr)
3°) destruction de l'objet créé en 1°) ?


Mais dans ce cas que fait :

Code :

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

m_test_B = CTestB(par_test_b);

1°) appel du constructeur par copie
2°) ??? y'a t'il un appel a l'opérateur = ?

merci