From: Howard Hinnant (howard.hinnant_at_[hidden])
Date: 2007-04-14 19:12:55
On Apr 14, 2007, at 1:28 PM, 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( 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 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.
Fwiw Peter asked me to look at this. Imho after a move from an
object it should be in a self-consistent state. All internal
invariants should hold. This answer is purposefully vague, and could
be used to support either outcome.
A very reasonable invariant for a shared_ptr r would be that if (r)
then *r is safe.
Another very reasonable invariant for a shared_ptr r would be that
after a move, if (r) does not imply that you can *r.
There is nothing in the rvalue ref proposals that really nails this
down. It is up to each class to define its behavior after a move.
At the very least it should support assignment and destruction in
order to work with generic code. Beyond that I'd say the class
should be free to implement whatever semantics / interface it feels
best.
In this particular case, I think it might be worth the extra
assignment to null r.px just to make the class invariant simpler.
However that statement falls well short of saying that the standard
should require such behavior, or even that this behavior isn't ok
with respect to putting this shared_ptr into std::containers or using
with std::algorithms. Furthermore I'm anxious to see what "best
practices" in this area are born out by field experience. And boost
is a great place to have such experiments! :-)
-Howard