Boost logo

Boost :

From: Joe Gottman (jgottman_at_[hidden])
Date: 2007-04-14 19:33:26

Peter Dimov wrote:
> Joe Gottman wrote:
>> 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( );
>> }
>> 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 problem with the above code does indeed exist, but it is not a bug in
> the implementation (let alone a subtle one) - in the sense that the
> implementation is deliberately written in the way it's written, to not
> provide a guarantee about the consistency of r after the move. You are right
> that we can clear r.px easily for shared_ptr, but it's more important to
> think about the general case: is the user allowed to assume anything about
> the state of an object after it has been moved from?
> void f( T t1 )
> {
> T t2( std::move( t1 ) );
> // What happens if I use t1 here?
> }
> Using t1 after the move looks like a programming error to me.

    I disagree. Suppose we have a function foo(shared_ptr<int> &&p)
that might or might not move from p. For instance, foo might be one of
several potential consumers of the shared_ptr. In this case, having the
move constructor automatically zero out p would be a nice way to let the
client know that p was actually moved from. It would enable code like
the following:

shared_ptr<int> p(new int(1));
if (p) { // foo didn't want it, try the next potential consumer
} else {
     return; // foo consumed our shared_ptr, so we are done.

Also, leaving the shared_ptr in an inconsistent state can cause
intermittent crashes far away from where the move occurred. This is
unfair to the person who will be debugging the crash, who may well be
someone different from the person who wrote the code with the move

Joe Gottman

Boost list run by bdawes at, gregod at, cpdaniel at, john at