[boost] [shared_ptr] dangerous implementation of move constructor

From: Joe Gottman (jgottman_at_[hidden])
Date: 2007-04-13 23:53:44

    I see that rvalue reference support has just been added to the 
shared_ptr template, and I think that there's a subtle bug in the 
implementation.  It is implemented as follows:

shared_ptr( shared_ptr && r ): px( r.px ), pn() // never throws
        {
            pn.swap( r.pn );
        }

The problem is that after the move constructor has been called the moved 
-from shared_ptr, r, is in a dangerously inconsistent state.  Its has an 
empty shared count, but it contains a non-null pointer.  Usually it 
should be either destructed or assigned to shortly after the move 
constructor is called, in which case all will be well.  However, if it 
remains unchanged then there is the danger that someone might try to 
dereference it after r.px has been deleted.  This, of course, could 
result in a hard-to-track access violation.  To see what I mean, 
consider the following rather silly code:

     shared_ptr<int> r(new int(1));
     shared_ptr<int> s(std::move(r));
     s.reset();
     if (r) { //r.get() != 0, so this returns true
        cout << *r; // Uh-oh
     }

The solution to this problem is simple: just add one more line of code 
to set r.px to NULL.

shared_ptr( shared_ptr && r ): px( r.px ), pn() // never throws
        {
            r.px = 0;
            pn.swap( r.pn );
        }

Now r will be an empty shared_ptr and the danger will be averted.

Joe Gottman