Don’t mix C++ smart pointers with references

As I did in the past, I will use this post as means to remember and to push the following principle deeper in my head – and hopefully in yours as a reader and C++ programmer:

Do not mix smart pointers with references in your C++ programms.

Of course I knew that before I created this little helper library, that was supposed to make it easier to send data asynchronous over an existing connection. Here is the situation (simplified):

class A
{
  ...
  void doStuff();

  private:
     // a private shared_ptr to B
    boost::shared_ptr<B> _bPointer;
};

class C
{
  public:
    C(B& b) : _b(b)
    {}

    ~C()
    {
      _bRef.resetSomeValueToDefault();
    }

  private:
     // a private reference to B which is set in the ctor
    B& _bRef;
};

void A::doStuff()
{
  createBpointerIfNotExisting();
  C myC(*_bPointer);
  myC.someMethodThatDoesSomethingWithB();
  if (someCondition) {
    // Delete this B instance.
    // A new instance will be created next time
    _bPointer.reset();
  }
}

So class A has a shared pointer of B which is given as a reference to an instance of class C in method A::doStuff. Class C stores the B instance as reference and interacts with it during its lifetime, which ends at the end of A::doStuff.

The last interaction occurrs at the very end of its life – in the destructor.

I highlighted the most important facts, but I’ll give you a few more moments …

The following happens (in A::doStuff):

  • line 29: if no instance of B exists (i.e. _bPointer is null), a new B instance is created and held in _bPointer
  • line 30: instance myC of C is created on the stack. A reference of B is given as ctor parameter
  • line 32-35: if “someCondition” is true, _bPointer is reseted which means that the B instance gets deleted
  • line 37: A::doStuff() ends and myC goes out of scope
  • line 19: the destructor of C is called and _bRef is accessed
  • since the B instance does not exist any more … memory corruption!!!

The most annoying thing with this kind of errors is that the program crashes somewhere, but almost never where the error actually occurred. This means, that you get stack traces pointing you right into some rock-solid 3rd party library which had never failed since you know and use it, or to some completely unrelated part in your code that had worked without any problems before and hasn’t been changed in years.

I even had these classes unit tested before I integrated them. But for some strange reason – maybe because everything gets reset after each test method – the bug never occurred in the tests.

So always be very cautious when you mix smart pointers with references, and when you do, make sure you have your object lifetimes completely under control!

5 thoughts on “Don’t mix C++ smart pointers with references

  1. If I can pick a nit, the copy cost of pointer wrappers, std or boost, is not small. I think the implied message, to not mix smart points with reference, is indeed a solution, but a weak one. Rather, what I take from this post as the lesson, is regardless of the user type (smart pointer or Foo or Bar) member variables should almost never be references for many similar reasons. Outside modification. Destruction. Etc.

    Thanks for the post, though I would love if you edited it to not dissuade the use of references with smart pointers.

    Also, a tool such as valgrind, can give you great clues about bad behavior such as this. Highly recommended.

  2. The key shot in the foot is:

    C myC(*_bPointer);

    You have manufactured a reference from the pointed to object through a pointer.

    To work correctly and harmoniously

    class C should hold a

    const boost::shared_ptr& _bRef;

    and it can be constructed:

    C myC(_bPointer);

  3. It’s not so much the reference that’s the problem (though you did finger that what you did caused a ghost in the machine) it’s that you deleted the smart pointer arbitrarily. If you’re mixing references and smart pointers, you are probably trying to avoid Object Slicing in some way (where you need to stash a collection of objects that has a base class as it’s interface edge) and you have to simply adhere to strict rules on the smart pointers- namely not yanking any instance out from underneath the whole thing. What was given in this example…heh…shows a lack of attention to what’s actually going on in the cowpiler’s actions and it’s base framework.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s