Re: [boost] boost::weak_ptr suggestions

From: Peter Dimov (pdimov_at_[hidden])
Date: 2002-04-01 09:31:05

From: "Raoul Gough" <RaoulGough_at_[hidden]>
> I have some suggestions about the existing boost::weak_ptr (I hope this is
> the right forum to post these issues).

It is. :-)

> Firstly, operator= and the copy constructor can currently make use of an
> invalid pointer value. This kind of thing has been discussed on
> comp.lang.c++.moderated, and I think the consensus was that reading an
> invalid pointer value is *undefined behaviour* even in the absence of
> dereferencing. Consider the following case:
>
> shared_ptr<X> sp1 (new X);
> weak_ptr<X> wp1 (sp1);
>
> sp1.reset (); // Invalidate wp1.px
> weak_ptr<X> wp2 (wp1) // Copies invalid pointer value
> wp2 = wp1; // Copies invalid pointer value
>
> The problem is that the raw pointer value (the px member variable) becomes
> invalid once its target object is deleted (see section 3.7.3.2 para 4). To
> be safely portable, it would be better for operator= and the copy
> constructor to use the other object's get() member function to ensure that
> the px value is only read if known to be valid. IIRC, the real-world
> argument against even looking at an invalid pointer value was that some
> architectures may use registers which perform address validation or even
> cause pre-fetching of memory pages.

You are right. I am aware of the potential problem; the current
implementation seems to work in practice, so I decided to leave it as is.
This is purely an implementation detail and the interface is not affected.

> Secondly, I believe it would be better for the get() method to throw or
> assert when called on an invalidated pointer, instead of transparently
> returning 0. In my opinion, there is a fundamental difference between the
> two states (null and invalid) which is not observable with the current
> interface. The addition of a member function like "bool is_valid() const;"
> would also allow the user code to decide how to deal with an invalid
> pointer, instead of merging the two distinct states into the one (null)
> state.

Right again. However, the primary methods of accessing a weak_ptr are (1)
constructing a shared_ptr (which does throw) and (2) make_shared. get() has
been retained for efficiency but is not recommended (in multithreaded
programs.)

> The big advantage of considering invalid.get() an error is that code which
> then works without error using weak_ptr would have *exactly* unchanged
> semantics using a plain pointer replacement. This allows (for example) a
> debug build/release build choice between weak_ptr<T> and T* for
performance
> reasons. If weak_ptr<T> silently returns null on invalid pointers, then
this
> guarantee cannot be made - what would be undefined use on a plain pointer
is
> not detected by the weak_ptr.

Interesting point. You can write your own get() that does what you want:

T * get(weak_ptr<T> const & p)
{
    return shared_ptr<T>(p).get();
}

but it's not as efficient as a throwing get(). Most people seem to prefer
the current get() semantics, though, where 0 is returned.