C++ shared_ptr pointeur <Base>acces violation - Stack Overflow

I'm using shared_ptr<Base> for some sort of tree list with derived classes. But I'm getting a pointer acces violation when my tree gets destructed.

My code looks something like this, besides, this actually similates my runtime error:

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


class Base;
typedef std::shared_ptr<Base> pBase;
class Derived;

class Base {
public:        
    std::vector<pBase> children;

    pBase parent;

    Base() {}
    virtual ~Base() {}

    virtual void doSomething() {}
    void add(pBase i);
};

class Derived : public Base {
    void doSomething() {
        // Do something...
    }
};

void Base::add(pBase i) {
    i->parent = pBase(this);
    children.push_back(i);
}


int main() {
    pBase tree = pBase(new Derived());
    pBase child(new Derived());
    child->add(pBase(new Derived()));
    tree->add(child);
}

Also when I add the following lines to Base::~Base: std::cout << "destruct " << name << std::endl;

And implement a std::string called name in Base which is different for each instance, I can see that the destructor is called multiple times (because of the Base::parent reference I think). That ofcourse triggered my error, but I still don't understand why it happens because shared_ptr<Base> is expected to count its references before actually destroying it!!?

I hope some can tell me what I'm doing wrong! But more important, how I can fix this!

Look at this line in add()

i->parent = pBase(this);

Each time you call add, you're creating a new shared pointer to this. These shared pointer are separate - that is, they are NOT 'shared' as you think. So, the first time you delete a child, it's parent gets deleted (because it's a shared pointer). Hence your code blows up.

Try (as a start) making parent a plain dumb pointer.

Base *parent;

Just to add to the others' answers: The canonical way to do what you want in the line

i->parent = pBase(this);

is to use std::enable_shared_from_this. You

1. Derive Base from it

   class Base : std::enable_shared_from_this<Base> {
   




2. Ensure that every Base instance is owned by a std::shared_ptr. That is OK in your case, since you create the objects in expressions like

   pBase child(new Derived());
   




3. Use shared_from_this() instead of this when you want a std::shared_ptr. The problematic line will then become

   i->parent = shared_from_this();
   

Here

i->parent = pBase(this);

you create a smart pointer from a plain old pointer to an object which you didn't get directly from new. Never do this.

As @Roddy explained, you get separate smart pointer objects, with separate reference counters. Two reference counters for one pointee won't work.

In your case, it's probably ok to make parent a normal pointer, as @Roddy proposed. This way, you don't get in trouble with cyclic references. Just be sure that you never access the parent pointer after you deleted the parent. No problem if you delete all the children together with the parent (which happens automatically, unless you store more smart pointers to them, somewhere else)

If you want to initialize a smart pointer, you've got two choices, basically: Use smart pointers in every interface. Unfortunately that doesn't work for "this" because that's an implicit parameter. You would need to pass the smart pointer, which you already created, to the method manually, in an extra parameter. Like this:

tree->add(tree, child);

Which is kind of ugly, so you may want to consider making "add" a static method, so you won't need to pass the parent twice.

The other choice: Use another kind of smart pointer, like boost::intrusive_ptr, where you can store the reference count in the pointee. This way, you are able to find the reference count, even if you've got only a dumb pointer like "this".

Edit: The answer by @jpalecek, below, is better. Use that one. Sebastian.

I'm using shared_ptr<Base> for some sort of tree list with derived classes. But I'm getting a pointer acces violation when my tree gets destructed.

My code looks something like this, besides, this actually similates my runtime error:

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


class Base;
typedef std::shared_ptr<Base> pBase;
class Derived;

class Base {
public:        
    std::vector<pBase> children;

    pBase parent;

    Base() {}
    virtual ~Base() {}

    virtual void doSomething() {}
    void add(pBase i);
};

class Derived : public Base {
    void doSomething() {
        // Do something...
    }
};

void Base::add(pBase i) {
    i->parent = pBase(this);
    children.push_back(i);
}


int main() {
    pBase tree = pBase(new Derived());
    pBase child(new Derived());
    child->add(pBase(new Derived()));
    tree->add(child);
}

Also when I add the following lines to Base::~Base: std::cout << "destruct " << name << std::endl;

And implement a std::string called name in Base which is different for each instance, I can see that the destructor is called multiple times (because of the Base::parent reference I think). That ofcourse triggered my error, but I still don't understand why it happens because shared_ptr<Base> is expected to count its references before actually destroying it!!?

I hope some can tell me what I'm doing wrong! But more important, how I can fix this!

Look at this line in add()

i->parent = pBase(this);

Each time you call add, you're creating a new shared pointer to this. These shared pointer are separate - that is, they are NOT 'shared' as you think. So, the first time you delete a child, it's parent gets deleted (because it's a shared pointer). Hence your code blows up.

Try (as a start) making parent a plain dumb pointer.

Base *parent;

Just to add to the others' answers: The canonical way to do what you want in the line

i->parent = pBase(this);

is to use std::enable_shared_from_this. You

1. Derive Base from it

   class Base : std::enable_shared_from_this<Base> {
   




2. Ensure that every Base instance is owned by a std::shared_ptr. That is OK in your case, since you create the objects in expressions like

   pBase child(new Derived());
   




3. Use shared_from_this() instead of this when you want a std::shared_ptr. The problematic line will then become

   i->parent = shared_from_this();
   

Here

i->parent = pBase(this);

you create a smart pointer from a plain old pointer to an object which you didn't get directly from new. Never do this.

As @Roddy explained, you get separate smart pointer objects, with separate reference counters. Two reference counters for one pointee won't work.

In your case, it's probably ok to make parent a normal pointer, as @Roddy proposed. This way, you don't get in trouble with cyclic references. Just be sure that you never access the parent pointer after you deleted the parent. No problem if you delete all the children together with the parent (which happens automatically, unless you store more smart pointers to them, somewhere else)

If you want to initialize a smart pointer, you've got two choices, basically: Use smart pointers in every interface. Unfortunately that doesn't work for "this" because that's an implicit parameter. You would need to pass the smart pointer, which you already created, to the method manually, in an extra parameter. Like this:

tree->add(tree, child);

Which is kind of ugly, so you may want to consider making "add" a static method, so you won't need to pass the parent twice.

The other choice: Use another kind of smart pointer, like boost::intrusive_ptr, where you can store the reference count in the pointee. This way, you are able to find the reference count, even if you've got only a dumb pointer like "this".

Edit: The answer by @jpalecek, below, is better. Use that one. Sebastian.

Source