shared_ptr, shared_from_this et double destruction

Bonjour à tous,

Je ne comprends pas ce qui se passe, j'ai des shared-ptr qui pointent sur le même pointeur et pourtant il y a double destruction. Pourquoi, dans l'exemple suivant, à la sortie du scope de p2, mon objet Test est détruit ?

Voici le code minimal :

 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

#include<iostream>
#include<boost/shared_ptr.hpp>
using namespace std;

struct Test {
    int member;
    Test(): member(123) {
        cout << "Construction" << endl;
    }
    ~Test () {
        cout << "Destruction" << endl;
    }
    void print (int line = -1) {
        cout << "Affichage à la ligne " << line << ": member=" << member << endl;
    }
};

int main (int argc, char** argv) {
    Test* x = new Test();
    x->print(__LINE__);
    shared_ptr<Test> p1(x);
    p1->print(__LINE__);
    {
        shared_ptr<Test> p2(x);
        p2->print(__LINE__);
    }
    p1->print(__LINE__);
    x->print(__LINE__);
    return 0;
}

ET voici le résultat :

1
2
3
4
5
6
7
8

Construction
Affichage à la ligne 20: member=123
Affichage à la ligne 22: member=123
Affichage à la ligne 25: member=123
Destruction
Affichage à la ligne 27: member=2955088
Affichage à la ligne 28: member=2955088
Destruction

Voici le raisonnement que j'avais et qui est apparament incorrect :

1. JE construit un objet Test non encore protégé
2. Je crée un shared_ptr p1 qui pointe sur mon objet; son compteur de référence interne est maintenant à 1.
3. JE crée le shared_ptr p2; normalement le compteur de référence est censé passer à 2 puisque c'est le même pointeur vers le même objet Test
4. ON arrive à la fin du scope de p2, le shared_ptr est détruit, le compteur de référence devrait passer de 2 à 1 et donc mon objet Test ne devrait pas être détruit.
5. L'objet Test devrait seulement être détruit en quittant le scope de p1, faisant passer le compteur de référence interne de 1 à 0, à la fin de la fonction main mais pas avant.

Il s'agit bien sûr d'un code minimal reproduisant le problème. Dans le cas réel, mon code s'approche plus de ceci (j'ai tout de même largement simplifié) :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22

class Test: std::enable_shared_from_this<Test>  {
void methode1 ();
void methode2 ();
};

void fonction (shared_ptr<Test> test) {
//...
}

void Test::methode1 () {
fonction( shared_from_this() );
}

void Test::methode2 () {
//..
}

vector<shared_ptr<Test>> tests;
tests.push_back(make_shared<Test>(...));
Test& t = *tests[0];
t.methode1(); // OK
t.methode2(); // boum! L'objet à été détruit à la fin de methode1, parce que j'ai construit un autre shared_ptr pointant sur le même objet

Que faire ? Y a-t-il une solution ?

Est-ce que j'ai raté quelque chose d'important dans l'utilisation des shared_ptr et/Ou de enable_shared_from_this ?

Ca me paraît un peu ridicule, si je ne peux pas créer plusieurs shared_ptr qui pointent sur le même objet; je dois forcément avoir fait quelque chose de faux.

Merci pour vos réponses.

Salut,

Oui ! Pour le premier exemple en tout cas. Dans la doc de boost (ici) :

Best Practices

A simple guideline that nearly eliminates the possibility of memory leaks is: always use a named smart pointer variable to hold the result of new. Every occurence of the new keyword in the code should have the form:

shared_ptr<T> p(new Y);

De façon plus explicite ici (maintenant que ça a été normalisé) car je ne trouve pas mieux dans la doc de boost :

Notes

La possession d'un objet peut être partagée avec un autre shared_ptr seulement en le copiant ou en l'utilisant pour le constructeur par copie d'un autre shared_ptr. Construire un nouveau shared_ptr en utilisant le pointeur nu sous-jacent (en utilisant par exemple get()) conduit à un comportement indéfini.

Cependant dans le «vrai» code c'est plus compliqué car on est bien censé avoir un partage avec shared_from_this() (ici), donc le problème n'est pas a priori celui du premier code :

Requires: enable_shared_from_this<T> must be an accessible base class of T. *this must be a subobject of an instance t of type T . There must exist at least one shared_ptr instance p that owns t.

Returns: A shared_ptr<T> instance r that shares ownership with p.

Donc personnellement je ne vois pas le problème dans ce code, désolé. Le tableau tests est bien toujours «en vie» lors de l'appel de la méthode ?

Edit:

Version cppreference :

Note that prior to calling shared_from_this on an object t, there must be a std::shared_ptr that owns t.

shared_from_this returns a shared_ptr which shares ownership of *this

Ton premier code est faux, le compteur de référence est géré par le shared_ptr, si tu créées un nouveau shared_ptr il faut le faire depuis le premier pour qu'ils partagent la même ressource (et le même compteur), sinon tu te retrouves avec deux shared_ptr qui gèrent tout les deux la même ressource sans avoir connaissance de l'autre.

C'est justement l'intérêt de shared_from_this de pouvoir récupérer le shared_ptr directement depuis la ressource.

Ton second code n'est pas assez complet pour reproduire le problème, https://ideone.com/ist4jH fonctionne.

La possession d'un objet peut être partagée avec un autre shared_ptr seulement en le copiant ou en l'utilisant pour le constructeur par copie d'un autre shared_ptr. Construire un nouveau shared_ptr en utilisant le pointeur nu sous-jacent (en utilisant par exemple get()) conduit à un comportement indéfini.

Ton premier code est faux, le compteur de référence est géré par le shared_ptr, si tu créées un nouveau shared_ptr il faut le faire depuis le premier pour qu'ils partagent la même ressource (et le même compteur), sinon tu te retrouves avec deux shared_ptr qui gèrent tout les deux la même ressource sans avoir connaissance de l'autre.

Merci, voilà qui explique très clairement le pourquoi du premier code. Je trouve que c'est quand même dommage et illogique, je pensais que les comptages de références étaient globaux mais en fait apparament non, chaque shared_ptr embarque son propre compteur indépendant des autres. C'est bon à savoir.

Donc personnellement je ne vois pas le problème dans ce code, désolé. Le tableau tests est bien toujours «en vie» lors de l'appel de la méthode ?

Oui, c'est un tableau sous forme de variable globale, qui n'est donc jamais détruit. Je m'assure évidemment aussi qu'il n'est pas vide.

Du coup mon problème réel ne doit pas être lié à shared_ptr ou shared_from_this.

Lu'!

Imagine que tu veuilles construire le comportement que tu cites et calcule la surcoût à l'utilisation, tu verras que ce n'est pas du tout illogique . Par ailleurs, lors que tu crées un pointeur shared, passe par make_shared (et make_unique pour les pointeurs uniques).

D'ailleurs, pourquoi make_shared est arrivé dès la norme C++11 alors qu'on a du attendre C++14 pour make_unique ? Ca fait longtemps que je me pose la question.

H.S : Mon premier post sur ZdS, ça fait plaisir

Parce qu'il a été "oublié".

L'un des intérêts de shared_ptr, avant sa normalisation, était d'offrir un pointeur intelligent plus efficace en allouant en un bloc la ressource et le compteur. L'autre intérêt était d'éviter d'écrire un new.

Quand unique_ptr a été introduit, l'intérêt d'un make_unique n'a pas sauté aux yeux puisque à première vue il ne reste que le second intérêt qui est purement stylistique.

Une fois le C++11 passé, il s'est avéré qu'il y avait un autre intérêt à utiliser une fonction de création à la place d'utiliser un new : une plus grande resistance aux exceptions (google avec GotW 102 si tu veux le détail). D'où l'introduction en C++14 du make_unique.

D'accord ! Merci bien !

Imagine que tu veuilles construire le comportement que tu cites et calcule la surcoût à l'utilisation, tu verras que ce n'est pas du tout illogique

En réalité, si on analyse un peu, shared_ptr, c'est rien de plus qu'un bête int associé au pointeur.

Alors qu'effectivement, dans la version que je propose, il faut un central répertoriant tous les pointeurs managés. Avant de découvrir comment utiliser shared_ptr, je m'étais amusé à faire quelque chose dans ce goût-là, avec une unordered_map<void*,int> en variable globale. J'ai arrêté quand j'avais des bugs aléatoires et quand j'ai du mettre des CRITICAL_SECTION partout pour y remédier, mais l'idée de base y était. Ca ne doit pas être si lent que ça vu que c'est en gros pourtant ce que fait python non ?

En fait, c'est brillant de simplicité. Mais je ne voyais pas les choses comme ça.

Entre temps j'ai trouvé où était mon problème, ça n'a absolument rien à voir avec shaered_ptr. JE suis en train de faire un logiciel qui embarque python, et je créais un objet python depuis C++ sans réserver le GIL auparavant. L'objet python en question a un shared_ptr dans ses membres. La bonne blague c'est que ça plantait carrément à un autre endroit… et du coup je pensais sérieusement que j'avais fait une bêtise avec shared_ptr et enable_shared_from_this.

Merci à tous.

Voilà. Je peux maintenant retourner me craquer avec mon code qui génère des wrappers automatiques.

En fait, le "control bloc" cree par shared_ptr contient effectivement un compteur pour les shared_ptr et un pointeur vers la ressource managée, mais il contient aussi un compteur pour les weak_ptr et peut contenir des pointeurs de fonction pour un deleter et un allocator personnalise. (la norme ne définie pas la structure du control bloc)

Merci gbdivers.

En fait, plus je réfléchis à mon code, plus je suis en train de me dire que shared_ptr n'est peut-être pas la meilleure solution pour mon cas finalement.

Je m'aperçois qu'en fait, c'est le vector global qui est le maître absolu. Si un objet n'est plus dans le vector, alors ça ne fait pas de sens qu'il existe encore ailleurs.

En fait il faudrait un mécanisme qui fait que, quand je supprime un objet du vector, en plus de le delete, que ça mette tous les pointeurs qui pointaient sur cet objet à NULL. Je suppose que ce n'est pas vraiment faisable.

Euh, si, c'est le rôle des pointeurs intelligents (en général).

Si tu as un vector<shared>, tu peux partager des weak, ce qui te permet d'éviter les dangling. Tu peux aussi avoir un vector<unique> et partager des "vector<unique>&" (mais si le vector est ré-alloué, tu ne pourras pas le savoir) ou partager des "reference_wrapper" (mais tu ne peux plus détecter un libération).

On est justement entrain d'en parler sur Dvp, tu peux voir mes explications ici et ici (et comme cela, tu verras aussi les avis contraires, tu pourras te faire une meilleure idée du problème).

Si tu as un vector<shared>, tu peux partager des weak, ce qui te permet d'éviter les dangling. Tu peux aussi avoir un vector<unique> et partager des "vector<unique>&" (mais si le vector est ré-alloué, tu ne pourras pas le savoir) ou partager des "reference_wrapper" (mais tu ne peux plus détecter un libération).

Merci, c'est finalement ce que j'ai fait, shared_ptr dans le vector et weak_ptr ailleurs. Ca fonctionne bien comme solution.