Avoiding indirect cyclic references when using shared_ptr and weak_ptr

Question

I'm currently putting together an application that relies heavily on shared_ptr and everything looks good so far - I've done my homework and have a pretty good idea of some of the pitfalls of using shared_ptrs.

One of the most recognised problems with shared_ptr is cyclic dependencies - these issues can be solved by storing weak_ptrs that don't affect the lifetime of objects up the chain. However, I'm struggling to get my head around times where it's necessary to store a pointer to an external object via a weak_ptr - I'm not sure whether it's forbidden, discouraged, or whether it's safe.

The following diagram describes what I mean (black arrows indicate shared_ptr; dashed indicate weak_ptr):

alt text http://img694.imageshack.us/img694/6628/sharedweakptr.png

• A parent contains shared_ptrs to two children, both of which point back to the parent using a weak_ptr.
• In the constructor of the first child I retrieve via the parent weak_ptr the pointer to the second child and store it locally.

The code looks like this:

#include <boost/shared_ptr.hpp>
#include <boost/weak_ptr.hpp>
#include <boost/make_shared.hpp>
#include <boost/enable_shared_from_this.hpp>

class child;
class child2;
class parent;

class parent : public boost::enable_shared_from_this<parent>
{
public:
    void createChildren()
    {
        _child2 = boost::make_shared<child2>(shared_from_this());
        _child = boost::make_shared<child>(shared_from_this());
    }

    boost::shared_ptr<child> _child;
    boost::shared_ptr<child2> _child2;
};

class child
{
public:
    child(boost::weak_ptr<parent> p)
    {
        _parent = p;
        _child2 = boost::shared_ptr<parent>(p)->_child2; // is this safe?
    }

    boost::weak_ptr<parent> _parent;
    boost::shared_ptr<child2> _child2;
};

class child2
{
public:
    child2(boost::weak_ptr<parent> p)
    {
        this->_parent = p;
    }

    boost::weak_ptr<parent> _parent;
};

int main()
{
    boost::shared_ptr<parent> master(boost::make_shared<parent>());
    master->createChildren();
}

I've tested this and it seems to work ok (I don't get any reports of memory leaks), however my question is: Is this safe? And if not, why not?

Answer 1

The child constructor appears to be safe the way you are calling it. However its not safe in general.

The issue is due to passing in a weak_ptr as the argument in the child constructor. This means you need to worry about whether the weak pointer is for an object that no-longer exists. By changing this parameter to a shared_ptrs and converting to a weak_ptr when storing we know that the object still exists. Here's the change:

child(boost::shared_ptr<parent> p)
{
    _parent = p;
    _child2 = p->_child2; // This is this safe
}