CODEWITHSUNDEEP
c++c++11
user9885108
user9885108

Reputation: 59

Destruction of each other in c++ shared_ptr

everyone. I find a problem in c++ deconstruction. I will show you my code below:

#include <iostream>
#include <memory>
#include <vector>

using namespace std;

class B;

class A{
public:
    typedef shared_ptr<A> Ptr;

    shared_ptr<B> b;

    int index;

    A(const shared_ptr<B>& b_, int index_):b(b_), index(index_){}

    ~A(){cout << "A is deleted." << endl;
         cout << "A id is " << index << endl;}



};

class B{
public:
    typedef shared_ptr<B> Ptr;

    vector<A::Ptr> all_a;

    void addA(){
        for(int i=0; i<10; i++){
            auto a = make_shared<A>(Ptr(this), i);

            all_a.push_back(a);
        }

    }

    int index;

    B(int index_):index(index_){

    }

    ~B(){
        cout << "B is deleted." << endl;
        cout << "B id is " << index << endl;
    }


};



int main(){

    B::Ptr b = make_shared<B>(5);

    b->addA();

    return 0;
}

A part of results is is 

... 
B is deleted.
B id is 5
A is deleted.
A id is 8
B is deleted.
B id is 5
A is deleted.
A id is 9
B is deleted.
B id is 5
double free or corruption (out)
[1]    29094 abort (core dumped)  ./a.out

What is it happened with my code, how can i debug this problem. I thought the shared_ptr has some bug in this case.

Upvotes: 5

Views: 2097

Answers (3)

Kemin Zhou
Kemin Zhou

Reputation: 6881

This is clearly noted in the C++ reference:

The ownership of an object can only be shared with another shared_ptr by copy constructing or copy assigning its value to another shared_ptr. Constructing a new shared_ptr using the raw underlying pointer owned by another shared_ptr leads to undefined behavior.

I remember the old saying: if everything fails read the instruction first.

You also cannot mix plain pointer and shared_ptr in your code. You may be holding on a plain pointer that has already been freed by the shared_ptr using the plain pointe.

Upvotes: 0

Wintermute
Wintermute

Reputation: 44023

There's one problem at play and another that'll pop up when you fix it.

Part 1: shared_ptrs that don't share

So, the reason you see a lot of destructor calls for the same B object and crash with a double-free error is this line:

auto a = make_shared<A>(Ptr(this), i);

In particular, Ptr(this) makes a shared_ptr<B> from this that stands alone -- it cannot know that there are other shared_ptrs somewhere else that feel responsible for the same object, it has its own reference count, and it'll believe that no one else is using the object it points to when that goes to zero. For all intents and purposes, it behaves like a unique_ptr because it never shares its reference count with another shared_ptr (except temporaries).

Unfortunately, you don't know a shared_ptr that manages the object you're working on inside the member function. Fortunately, there's something in the standard library to help with that: std::enable_shared_from_this It works like this:

class B : public std::enable_shared_from_this<B> {

// ...

   auto a = make_shared<A>(shared_from_this(), i);

Note that if you do this, the code expects that B will only ever be manged by shared_ptrs. Although, really, it already does that, so that's okay.

Part 2: Round and round we go -- cyclic reference counts

Now, once you put that into your program, it'll not crash anymore. It'll also not print anything, as if nothing were ever deleted. What gives?

In fact, nothing is ever deleted, and the reason is that as far as all those shared_ptrs are concerned, their objects never go out of use. We've stumbled upon the main reason that reference-counting does not a full garbage collector make.

Your B object knows 10 A objects, but I'll consider only one of them -- it's just the thing I'm about to show 10 times. They have shared_ptrs to each other, and there are however many external shared_ptr that keep them alive as long as they are needed. Once all those external references are gone, you expect them to be deleted, but consider what the situation at that point looks like:

+-----+   shared_ptr<A>   +-----+
|     | ----------------> |     |
|  b  |                   |  a  |
|     | <---------------- |     |
+-----+   shared_ptr<B>   +-----+

b has a shared_ptr to a, so a has a reference count of 1. a has a shared_ptr to b, so b has a reference count of 1. We're caught in a catch-22: a is in use because b is use, and b is in use because a is in use, so neither is ever deleted.

When working with shared_ptrs, this kind of setup absolutely has to be avoided, and that's why weak_ptr exists. I have to guess which way you want the ownership mechanics to go, but for example you could have non-owning references from the A to the B objects like so:

class A{
    // ...
    weak_ptr<B> b;

and in B:

void addA(){
    for(int i=0; i<10; i++){
      auto a = make_shared<A>(weak_from_this(), i);
      all_a.push_back(a);
    }
}

Then the objects in all_a will not keep the B object that owns them alive. To use it, you would write

std::shared_ptr<B> b2 = b.lock();

to have a shared_ptr that keeps it alive while you're working with it.

Upvotes: 8

M.M
M.M

Reputation: 141554

When you do shared_ptr<B>(this) twice, you do not get 2 shared_ptr which manage this successfully. You get 2 shared_ptr with separate control blocks each of which thinks they are the only manager of this . Inevitably this will lead to a double delete or worse.

One way to solve this problem is to use shared_from_this() instead of this; see here for example.

Upvotes: 1

Related Questions