What Are Potential Dangers When Using Boost::Shared_Ptr

What are potential dangers when using boost::shared_ptr?

Cyclic references: a shared_ptr<> to something that has a shared_ptr<> to the original object. You can use weak_ptr<> to break this cycle, of course.

---

I add the following as an example of what I am talking about in the comments.

class node : public enable_shared_from_this<node> {
public :
    void set_parent(shared_ptr<node> parent) { parent_ = parent; }
    void add_child(shared_ptr<node> child) {
        children_.push_back(child);
        child->set_parent(shared_from_this());
    }

    void frob() {
        do_frob();
        if (parent_) parent_->frob();
    }

private :
    void do_frob();
    shared_ptr<node> parent_;
    vector< shared_ptr<node> > children_;
};

In this example, you have a tree of nodes, each of which holds a pointer to its parent. The frob() member function, for whatever reason, ripples upwards through the tree. (This is not entirely outlandish; some GUI frameworks work this way).

The problem is that, if you lose reference to the topmost node, then the topmost node still holds strong references to its children, and all its children also hold a strong reference to their parents. This means that there are circular references keeping all the instances from cleaning themselves up, while there is no way of actually reaching the tree from the code, this memory leaks.

class node : public enable_shared_from_this<node> {
public :
    void set_parent(shared_ptr<node> parent) { parent_ = parent; }
    void add_child(shared_ptr<node> child) {
        children_.push_back(child);
        child->set_parent(shared_from_this());
    }

    void frob() {
        do_frob();
        shared_ptr<node> parent = parent_.lock(); // Note: parent_.lock()
        if (parent) parent->frob();
    }

private :
    void do_frob();
    weak_ptr<node> parent_; // Note: now a weak_ptr<>
    vector< shared_ptr<node> > children_;
};

Here, the parent node has been replaced by a weak pointer. It no longer has a say in the lifetime of the node to which it refers. Thus, if the topmost node goes out of scope as in the previous example, then while it holds strong references to its children, its children don't hold strong references to their parents. Thus there are no strong references to the object, and it cleans itself up. In turn, this causes the children to lose their one strong reference, which causes them to clean up, and so on. In short, this wont leak. And just by strategically replacing a shared_ptr<> with a weak_ptr<>.

Note: The above applies equally to std::shared_ptr<> and std::weak_ptr<> as it does to boost::shared_ptr<> and boost::weak_ptr<>.

What are potential dangers when using boost::shared_ptr?

Cyclic references: a shared_ptr<> to something that has a shared_ptr<> to the original object. You can use weak_ptr<> to break this cycle, of course.

---

I add the following as an example of what I am talking about in the comments.

class node : public enable_shared_from_this<node> {
public :
    void set_parent(shared_ptr<node> parent) { parent_ = parent; }
    void add_child(shared_ptr<node> child) {
        children_.push_back(child);
        child->set_parent(shared_from_this());
    }

    void frob() {
        do_frob();
        if (parent_) parent_->frob();
    }

private :
    void do_frob();
    shared_ptr<node> parent_;
    vector< shared_ptr<node> > children_;
};

In this example, you have a tree of nodes, each of which holds a pointer to its parent. The frob() member function, for whatever reason, ripples upwards through the tree. (This is not entirely outlandish; some GUI frameworks work this way).

The problem is that, if you lose reference to the topmost node, then the topmost node still holds strong references to its children, and all its children also hold a strong reference to their parents. This means that there are circular references keeping all the instances from cleaning themselves up, while there is no way of actually reaching the tree from the code, this memory leaks.

class node : public enable_shared_from_this<node> {
public :
    void set_parent(shared_ptr<node> parent) { parent_ = parent; }
    void add_child(shared_ptr<node> child) {
        children_.push_back(child);
        child->set_parent(shared_from_this());
    }

    void frob() {
        do_frob();
        shared_ptr<node> parent = parent_.lock(); // Note: parent_.lock()
        if (parent) parent->frob();
    }

private :
    void do_frob();
    weak_ptr<node> parent_; // Note: now a weak_ptr<>
    vector< shared_ptr<node> > children_;
};

Here, the parent node has been replaced by a weak pointer. It no longer has a say in the lifetime of the node to which it refers. Thus, if the topmost node goes out of scope as in the previous example, then while it holds strong references to its children, its children don't hold strong references to their parents. Thus there are no strong references to the object, and it cleans itself up. In turn, this causes the children to lose their one strong reference, which causes them to clean up, and so on. In short, this wont leak. And just by strategically replacing a shared_ptr<> with a weak_ptr<>.

Note: The above applies equally to std::shared_ptr<> and std::weak_ptr<> as it does to boost::shared_ptr<> and boost::weak_ptr<>.

Is boost::interprocess::shared_ptr threadsafe (and interprocess-safe)?

The reference count used in boost::interprocess:shared_ptr is implemented using an atomic counter defined in boost/interprocess/detail/atomic.hpp with the refcount logic mainly implemented by boost/interprocess/smart_ptr/detail/sp_counted_base_atomic.hpp. The intent is to have the refcount be handled in a thread (and interprocess) safe manner.

The atomic operation implementations differ depending on the specific target platform (Windows uses the Win32 Interlocked APIs, some platforms use various inline assembly, etc). It might be helpful to know what platform you're targeting. I suppose that you may be running into a bug in the refcount handling, though I wouldn't count on it.

I've restricted the above answer to the area you wanted specifically addressed:

The question of threadsafety is about the reference count -- i.e., whether copying/destroying shared pointers to the same thing in different processes at the same time is permitted. Not access to the same shared pointer in different threads, and not access to the pointee.

That said, I'd look at bugs introduced possibly by the items you mention above or by somehow creating 'independent' boost::interprocess:shared_ptr objects (where different shared_ptrs refer to the same object using different refcounts). This situation can be easy to have happen if you have some code that continues to use and/or pass around the raw object pointer.

Is it OK to use boost::shared ptr in DLL interface?

According to Scott Meyers in Effective C++ (3rd Edition), shared_ptrs are safe across dll boundaries. The shared_ptr object keeps a pointer to the destructor from the dll that created it.

In his book in Item 18 he states, "An especially nice feature of
tr1::shared_ptr is that it automatically uses its per-pointer deleter
to eliminate another potential client error, the "cross-DLL problem."
This problem crops up when an object is created using new in one
dynamically linked library (DLL) but is deleted in a different DLL. On
many platforms, such cross-DLL new/delete pairs lead to runtime
errors. tr1::shared_ptr avoid the problem, because its default deleter
uses delete from the same DLL where the tr1::shared_ptr is created."

Tim Lesher has an interesting gotcha to watch for, though, that he mentions here. You need to make sure that the DLL that created the shared_ptr isn't unloaded before the shared_ptr finally goes out of scope. I would say that in most cases this isn't something you have to watch for, but if you're creating dlls that will be loosely coupled then I would recommend against using a shared_ptr.

Another potential downside is making sure both sides are created with compatible versions of the boost library. Boost's shared_ptr has been stable for a long while. At least since 1.34 it's been tr1 compatible.

safety of passing boost::shared_ptr reference

This usage is safe, in that whatever the shared_ptr<> passed in through the reference will have it's refcount reduced (assuming that the shared_ptr<> returned from seeker->second->Copy() isn't a shared_ptr<> to the same object) and therefore the object it will be pointing to might be deleted.

Specifically, you aren't creating a second shared_ptr<> from a raw pointer (which is an error because it would create a second, unrelated shared_ptr<> with a separate refcount, and therefore a second owner of the object).

Whether your function provides the behavior you want depends on what you want.

Do boost::shared_ptrT and boost::shared_ptrconst T share the reference count?

It is perfectly safe.

The following code sample:

#include <iostream>
#include <boost/shared_ptr.hpp>

int main(int, char**)
{
  boost::shared_ptr<int> a(new int(5));
  boost::shared_ptr<const int> b = a;

  std::cout << "a: " << a.use_count() << std::endl;
  std::cout << "b: " << b.use_count() << std::endl;

  return EXIT_SUCCESS;
}

Compiles and run fine, and is perfectly correct. It outputs:

a: 2
b: 2

The two shared_ptr share the same reference counter.

---

Also:

#include <iostream>
#include <boost/shared_ptr.hpp>

class A {};
class B : public A {};

int main(int, char**)
{
    boost::shared_ptr<A> a(new B());
    boost::shared_ptr<B> b = boost::static_pointer_cast<B>(a);

    std::cout << "a: " << a.use_count() << std::endl;
    std::cout << "b: " << b.use_count() << std::endl;

    return EXIT_SUCCESS;
}

Behave the same way. You must, however never build your shared_ptr using a construct like this:

boost::shared_ptr<A> a(new B());
boost::shared_ptr<B> b(static_cast<B*>(a.get()));

a.get() gives the raw pointer and loses all information about reference counting. Doing this, you'll end up with two distinct (not linked) shared_ptr that use the same pointer but different reference counters.

should I continue to use boost::shared_array or use boost::shared_ptrtype [] instead?

If the code is correct and working, I would not do the subject change.

If I had spare time and desire to do the change nevertheless, I would migrate to std::shared_ptr<T[]> straight away.

boost::shared_ptr drop-in replacement

Step 1: Build a class like this, and replace usage of boost::shared_ptr<T> with it.

template<typename T>
struct trivial_ptr {
  T* t;
  template<typename U>
  void reset( U* p ) {t=p;}
  void reset( T* p = NULL ) { t=p; }
  template<typename U>
  trivial_ptr<T>& operator=(trivial_shared_ptr<U>const& o) {
    t = o.t;
    return *t;
  }
  explicit trivial_ptr( T* p ):t(p) {}
  ...
};

this class is not intended to run, but just to compile with the correct interface. Once you have compiled, you can ensure you know what parts of the boost::shared_ptr interface you are using. (Are you using custom deleters? etc -- the problem could be harder, or easier, and the above can help test it)

Once you are there, you can work out how hard it will be to write up a shared_ptr<T> that handles multiple threads accessing the same variable at the same time.

Now, this is extremely messy. If one thread resets a given shared_ptr while another thread reads from it, the pointer read from may be completely invalid by the time the reading thread has access to it. In effect, you need to guard all access to the underlying pointer in a mutex, which is utterly impractical.

On the other hand, if what you have is multiple readers, and never a reader and a writer, you are in better shape -- in theory, you could fix the problem with the use of appropriate locks on the reference count code.

However, what you actually describe seems to involve multiple threads both reading and writing to the same variable. And that is fundamentally broken in a way that mere thread safety on the shared_ptr variable is not going to fix.

---

Related Topics