Libération de mémoire

Bonjour,

J'ai un pb qui survient lors de la libération de mémoire
allouée pour les objets d'une classe, mais je n'arrive absolument pas à
savoir pourquoi, (lorsque je quitte l'appli un message d'erreur
de Windows s'affiche).

Le code étant extrêmement long et compliqué je vais simplifier.
La classe en question alloue un tableau d'objets lors de sa
création:

Code:

---

1 2 3 4 5 6

MaClasse(int taille) { pObj = new Obj[taille]; ... }

---

et libère la mémoire allouée lors de sa destruction:

Code:

---

1 2	~MaClasse(){delete[] pObj;}

---

Quelque part dans une autre classe 2 objets MaClasse sont créés:

Code:

---

1 2 3

MaClasse *p1 = new Maclasse(50); MaClasse *p2 = new Maclasse(50);

---

et sont détruits dans le destructeur de cette même autre classe:

Code:

---

1 2 3

delete p1; delete p2;

---

Le pb apparement vient qu'il y a 2 instanciations/libérations.
Si je n'instancie qu'un seul objet je n'ai pas d'erreur
lors de la désallocation.

Est ce que quelqu'un aurait eu ce genre de pb ou aurait
une idée ?


D'avance merci.

Ta classe gère la durée de vie d'une ressource, tu as donc d'autres critères à vérifier:

• soit elle doit être explicitement non-copiable et non-assignable (constructeur de copie et opérateur d'affectation déclarés privés et non-définis),
• soit elle est copiable, et doit donc avoir un constructeur de copie (et un opérateur d'affectation aussi, si elle est assignable) clairement défini. Si elle possède aussi un opérateur d'affectation, je conseille l'idiome copy-and-swap.

De plus, pour être parfaitement exception-safe, ta classe ne doit gérer "elle-même" la durée de vie que d'une seule ressource. (en gros, un seul new dans le corps du constructeur, un seul delete dans le corps du destructeur).

Le code que tu montres n'as pas de raison à priori de ne pas fonctionner. Donc, je pense comme Medinoc: quelque part tu dois avoir une copie de ta variable et ceci n'est pas gérer correctement.

Oui je crois qu'en effet la classe (celle qui alloue un tableau) n'est pas en cause.
J'ai déjà effectué un 1er traçage mais je n'ai rien remarqué de suspect.
C'est vrai que le code est long et complexe, je pense qu'il faut donc que je surveille le code pas à pas mais bon, pas évident...:aie:

Si quelqu'un à une idée/intuition qu'il n'hésite pas.

Ben l'idée, c'est ce qu'à dit Médinoc au début: l'opérateur = et le constructeur par copie sont-ils gérés? Probablement non, et l'erreur est ici.

L'opérateur = et le constructeur par copie ne sont effectivement pas
gérés, mais le truc c'est qu'ils ne sont pas utilisés dans mon code (enfin je crois).
Tout se déroule (en gros) comme dans l'exemple que j'ai donné plus haut, aussi je ne pense pas que l'opérateur = et le constructeur par copie interviennent dans ce genre de code (mais je peux me tromper :oops:)

Pour être absolument sûr qu'ils ne sont pas utilisés, rends ta classe non-copiable et non-assignable, en les déclarant private sans les définir.

Citation:

---

Envoyé par tintin72

enfin je crois

---

Probablement à un endroit dont tu ne te doutes même pas. Comme dit Médinoc, pour en être sûr déclare les en private et compile. Tu le verras tout de suite.

Salut
~MaClasse(){delete[] pObj;}
Je pense qu'il faudrait ajouter une fonction membre qui supprime l'objet créer par un pointeur.

Code:

---

1 2 3 4 5 6

void CMaClasse::Clear() { if(pObj != NULL) // tester si le pointeur n'est pas NULL delete[] pObj; }

---

On detruit l'objet pObj
Ensuite on detruit l'instance de CMaClass
Cordialement

Citation:

---

Envoyé par buggen25

Salut
~MaClasse(){delete[] pObj;}
Je pense qu'il faudrait ajouter une fonction membre qui supprime l'objet créer par un pointeur.

Code:

---

1 2 3 4 5 6

void CMaClasse::Clear() { if(pObj != NULL) // tester si le pointeur n'est pas NULL delete[] pObj; }

---

On detruit l'objet pObj
Ensuite on detruit l'instance de CMaClass
Cordialement

---

1. Ça ne sert à rien ici, ça n'est pas le problème.
2. Inutile de tester NULL avant, delete[] NULL est garanti ne rien faire.
3. Par contre, tu oublies de mettre le pointeur à NULL, ce qui fait qu'il risque d'être deleté deux fois.

Ce n'est pas la peine de venir aider si c'est pour donner du code dangereux...

Citation:

---

Pour être absolument sûr qu'ils ne sont pas utilisés, rends ta classe non-copiable et non-assignable, en les déclarant private sans les définir.

---

Pourrais tu me dire comment faire pour les déclarer private sans les définir, (un petit exemple).
J'avoue que je nage un peu là. :oops:

Code:

---

1 2 3 4 5 6 7 8 9 10 11

class MaClasse { MaClasse(Maclasse const &); MaClasse& operator=(MaClasse const &); };   // Ou alors   class MaClasse : boost::noncopyable { };

---

Code:

---

1 2 3 4 5 6 7 8

class MaClasse { ... ... private: MaClasse(MaClasse const &); MaClasse & operator= (MaClasse const &); };

---

Et rien d'autre.
Ils sont déclarés, privés, mais définis nulle part. Donc, si quelque chose tente de les utiliser, il y aura une erreur de compilation ou d'édition de liens (selon qu'on tente de l'utiliser depuis la classe elle-même ou depuis l'extérieur).

Edit : Grillé, grillé et regrillé :aie:

Code:

---

1 2 3 4 5 6 7 8

class A { private: A(const A&); A& operator=(const A&); };

---

Edit2 :

Pourquoi avez-vous écrit tous les deux : MaClasse(MaClasse const &) ? 8O
Y a t-il une différence avec l'usuel: MaClasse(const MaClasse&) ?

Merci pour vos réponses rapides ;)
Malheureusement ça compile sans pb :?
Aucun warning, rien. J'ai bien peur d'avoir affaire
à une erreur plutot vicieuse.
Bon j'y retourne.

Citation:

---

Envoyé par Arzar

Pourquoi avez-vous écrit tous les deux : MaClasse(MaClasse const &) ? 8O
Y a t-il une différence avec l'usuel: MaClasse(const MaClasse&) ?

---

Aucune différence, mais pour le compilo, c'est le X const & qui est l'usuel : Le const X & est un cas particulier autorisé pour le type le plus à gauche, mais en temps normal, le const est à droite du type :
X const * const * pcpcX;

Citation:

---

Envoyé par tintin72

Malheureusement ça compile sans pb :?
Aucun warning, rien.

---

Ca n'empêche pas de gérer correctement ton pointeur ;) !

Citation:

---

Envoyé par tintin72

J'ai bien peur d'avoir affaire
à une erreur plutot vicieuse.

---

Alors c'est peut dû au fait que tes objets sont détruits 2 fois pour une autre raison : tu fais explicitement 2 fois un delete sur la même adresse. La seconde hypothèse, c'est le charcutage mémoire! Enfin, à ce stade, il faut un peu plus de code pour t'apporter de l'aide.
Bon courage.

Citation:

---

La seconde hypothèse, c'est le charcutage mémoire!

---

Qu'est ce que le "charcutage mémoire" exactement ?

Citation:

---

Envoyé par Arzar

Pourquoi avez-vous écrit tous les deux : MaClasse(MaClasse const &) ? 8O
Y a t-il une différence avec l'usuel: MaClasse(const MaClasse&) ?

---

Parce que pour nous, c'est ça l'écriture usuelle ?
Il n'y a pas de différence en l'occurrence. La règle est que const s'applique à ce qui est placé avant lui, sauf s'il apparait en début, auquel cas il s'applique à ce qui est place derrière. Cette règle est surtout utile dans des écritures genre int const * (pointeur sur entier constant ou pointeur constant sur entier ?).

Écrire int const i = 42; au lieu de const int i = 42; permet de se placer dans le cas général, et non dans un cas particulier. C'est une question d'habitude.

Citation:

---

Envoyé par tintin72

Qu'est ce que le "charcutage mémoire" exactement ?

---

Tu as alloué les octets 5 à 10 et tu écris sur l'octet 11 ou 4. Bref, là où tu n'as pas le droit.

Je viens de trouver le pb !
Au cours d'une énième compilation mon ordi à carrément planté, je préfère même pas savoir pourquoi. Bref au redémarrage, pour
plus de sécurité je vire le .exe pour refaire une compile propre.
Et là plus de pb.8O
J'ai recompilé plusieurs fois pour être sûr mais non, plus d'erreur.
Je sais pas ce qui a bien pu se passer dans l'exe au cours des précédentes
compiles mais c'est la 1ere fois que ça m'arrive.
En tout cas je m'en souviendrais pour la prochaine fois. ;)

Citation:

---

Envoyé par Médinoc

1. Ça ne sert à rien ici, ça n'est pas le problème.
2. Inutile de tester NULL avant, delete[] NULL est garanti ne rien faire.
3. Par contre, tu oublies de mettre le pointeur à NULL, ce qui fait qu'il risque d'être deleté deux fois.

Ce n'est pas la peine de venir aider si c'est pour donner du code dangereux...

---

c'est pas du code dangeureux, Il faut faire des initialisations.
De plus je fais jamais confiance a une fonction qu'on ne peut appeler explicitement (le destructeur), alors je la laisse vide (la plupart du temps )

pour la remarque numero 3, je ne vais pas appeler delete dans le destructeur

Donc ce n'est pas aussi dangereux !
ps : sans rancune :)

Destructeur ou pas, si tu appelles deux fois Clear(), tu cours à la catastrophe.
Remettre un pointeur à NULL après l'avoir invalidé est un des principes de base de la programmation défensive: Même si tu crois que ça ne change rien, fais-le.

Salut, c'est ça que je voulait dire, j'ai compilé et ça marche

Code:

---

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

#include <stdio.h> class MaClass{ int * pOne; int * pTwo; public :   MaClass(){ pOne = new int[10]; pTwo = new int[20]; pOne[5]=1; pTwo[5]=2; }; ~MaClass(){}; void Print(){ printf("Pointer one :%d\n",pOne[5]); printf("Pointer Two :%d",pTwo[5]); }; void Clear(){ if( pOne != NULL) delete[]pOne; if( pTwo != NULL) delete[]pTwo; } };   int main(int argc,char * argv[]) { MaClass cls; cls.Print(); scanf("%d"); cls.Clear(); return 0; }

---

Si tu avais pensé à remettre le pointeur à NULL ensuite, ce code ne serait pas buggué.

Code:

---

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 37 38 39

#include <stdio.h> class MaClass{ int * pOne; int * pTwo; public :   MaClass(){ pOne = new int[10]; pTwo = new int[20]; pOne[5]=1; pTwo[5]=2; }; ~MaClass(){}; void Print(){ printf("Pointer one :%d\n",pOne[5]); printf("Pointer Two :%d",pTwo[5]); }; void Clear(){ if( pOne != NULL) delete[]pOne; if( pTwo != NULL) delete[]pTwo; } };   void Oups(MaClass & cls) { cls.Print(); cls.Clear(); }   int main(int argc,char * argv[]) { MaClass cls; Oups(cls); { int unEntier; scanf("%d", &unEntier); } cls.Clear(); //Oups! J'ai appelé Clear() deux fois! return 0; }

---

Et en plus, ce code est tout sauf exception-safe.

Citation:

---

Envoyé par Médinoc

Si tu avais pensé à remettre le pointeur à NULL ensuite, ce code ne serait pas buggué.

Code:

---

cls.Clear(); //Oups! J'ai appelé Clear() deux fois!

---

Et en plus, ce code est tout sauf exception-safe.

---

Je n'avait pas dit que j'allait l'appeler deux fois :mrgreen:. de plus je viens juste de l'écrire avec devC++.
En ce que cocèrne l'exception-safe, je passe en mode release, ensuite je vérifie bien qu'il n'affiche pas le message "Envoyer le message a micro$oft" ou "Access viloation to adress 0x0000000".

Citation:

---

Envoyé par buggen25

Je n'avait pas dit que j'allait l'appeler deux fois :mrgreen:.

---

Mais rien n'empêche de le faire.

Par contre, une bonne programmation défensive limite les dégâts quand c'est fait. Et tu ne comprends pas ce que c'est que l'Exception-safety, notamment parce qu'il n'y a rien dans le code qui en lance une. Sache en tout cas que ça ne se limite pas au non-plantage.

Je sais ce que sa veut dire, ça ressemble aux exceptions génèrés par des classes comme CDatabase, ou CRecordSet . pour cela il faut les "catcher".Vu que ce code n'utilise pas d'API, il n'ypas de soucis, pour un projet réelle j'utiliserais pas ce genre de code.:D

Citation:

---

Envoyé par buggen25

De plus je fais jamais confiance a une fonction qu'on ne peut appeler explicitement (le destructeur), alors je la laisse vide (la plupart du temps )

---

Tu raisonnes à l'envers.
Un bon programmeur ne fais jamais confiance à une fonction de nettoyage qu'il risque d'oublier d'appeler.

Considère ce code:

Code:

---

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 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54

#include <exception> #include <iostream> #using namespace std;   class MonException : public std::exception { };   class MaClass { int * pOne; int * pTwo; public : MaClass() : pOne(NULL), pTwo(NULL) { pOne = new int[10]; pTwo = new int[20]; pOne[5]=1; pTwo[5]=2; }; ~MaClass(){ }; void Print() const { cout << "Pointer one : " << pOne[5] << endl; cout << "Pointer two : " << pTwo[5] << endl; }; void Clear() { delete[] pOne, pOne=NULL; delete[] pTwo, pTwo=NULL; } };   void FonctionQuiLanceUneException(void) { //... blablabla, du code if( /* un truc */ ) throw MonException; //... blablabla, du code }   void UneFonction(void) { MaClass monObjet; monObjet.Print();   FonctionQuiLanceUneException();   monObjet.Clear(); //Oups! Cette fonction ne sera pas appelée! //Quel dommage que je n'aie pas donné un bon //destructeur à ma classe! }   int main(void) { try { UneFonction(); } catch(MonException const & e) { cout << "Exception attrapee." << endl; } return 0; }

---

Avec ça, si la fonction lance son exception, la fonction Clear() ne sera pas appelée. Tu sais, les destructeurs ont une raison d'être: Celle d'être appelé même en cas d'exception...

PS: Le simple opérateur new lève une exception en cas d'échec, sur tout système un tant soi peu conforme à la norme (e.g, gcc, Visual 2005 Pro et supérieur). C'est plus pratique et moins contraignant que de tester à chaque fois le retour de malloc().
Donc si tu fais des new, tu dois apprendre à faire du code exception-safe.
Pareil si tu fais des dynamic_cast<> avec des références.

Citation:

---

Envoyé par Médinoc

Tu raisonnes à l'envers.
Un bon programmeur ne fais jamais confiance à une fonction de nettoyage qu'il risque d'oublier d'appeler.

---

++++

J'ai lu en diagonale, mais je partage cet avis. Un destructeur c'est fait pour ça!

Citation:

---

Envoyé par Arzar

Pourquoi avez-vous écrit tous les deux : MaClasse(MaClasse const &) ? 8O
Y a t-il une différence avec l'usuel: MaClasse(const MaClasse&) ?

---

C'est la même chose en fait 8-)

Citation:

---

Par contre, tu oublies de mettre le pointeur à NULL, ce qui fait qu'il risque d'être deleté deux fois.

---

Ce qui ne sert strictement à rien dans un destructeur.

Citation:

---

Envoyé par loufoque

Ce qui ne sert strictement à rien dans un destructeur.

---

Mais justement, ce n'est pas dans le destructeur qu'il l'a fait...

Citation:

---

Envoyé par Médinoc

Mais justement, ce n'est pas dans le destructeur qu'il l'a fait...

---

Mettre le pointeur à NULL dans le destructeur est une pratique courante et utile (en cas de sur libération !).

A bientôt ;)

Quand j'ai des craintes de double-destruction, j'ai plutôt tendance à rajouter un booléen bDestroyed (ou plus sûr, une valeur magique) et mettre une assertion dessus...

Dans le code du premier message, le seul delete que je vois est dans le destructeur.

En effet, mais c'était surtout le "conseil" de buggen25 que je critiquais.

Citation:

---

Envoyé par Médinoc

Tu raisonnes à l'envers.
Un bon programmeur ne fais jamais confiance à une fonction de nettoyage qu'il risque d'oublier d'appeler.

---

Oui je réflechit à l'envers. mais il y a une chose dont je suis sure utliser le destructeur pour desallouer quelque chose n'est pas tout le temps la solution miracle.

Code:

---

1 2 3

if ( bTheDatabeiseConnected ) pRows = new Row[iHugeNumberOfRows];

---

Il faut attendre que le destructeur le fasse. et surtout quand on a besoin de réutiliser pRows. c'est un autre problème
Pour les exceptions, il y a toujours une exception qui confirme la règle.

Code:

---

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

#include <stdio.h> #include <limits.h> struct LimitException{ }; char ToChar(int ii) { char ch; if( ii > UCHAR_MAX || ii < 0 ) throw LimitException(); else return ii; }   int main(int argc, char * argv[]) { try { printf("C'est le caractère : %c\n",ToChar(-500)); } catch(LimitException& X) { printf("Exception, plage de valeurs"); } int AnIntegerPlease; scanf("%d",AnIntegerPlease); return 0; }

---

Je penses que tu confond le fait qu'un pointeur peut avoir une durée de vie inférieur à l'objet qui le contient et la libération de l'adresse dans le destructeur. Moi je pense qu'il faut toujours libérer la mémoire dans le destructeur (c'est fait pour ça!), mais rien ne t'empêche d'avoir d'autres endroits où la mémoire est libérée selon tes besoins. L'un n'empêche pas l'autre.

Citation:

---

Envoyé par 3DArchi

Je penses que tu confond le fait qu'un pointeur peut avoir une durée de vie inférieur à l'objet qui le contient et la libération de l'adresse dans le destructeur. Moi je pense qu'il faut toujours libérer la mémoire dans le destructeur (c'est fait pour ça!), mais rien ne t'empêche d'avoir d'autres endroits où la mémoire est libérée selon tes besoins. L'un n'empêche pas l'autre.

---

Je suis tout a fait d'accord avec toi (c'est une première), l'un n'empèche pas l'autre. Si tu alloue dynamiquement un membre de cette classe dans le constructeur, et que le cycle de vie d'un membre de l'objet est conçu pour s'arreter a la destruction de l'objet, on laisse le destructeur faire son travail. et c'est tout :).