Re: [boost] Re: shared_ptr: unsafe, or am I crazy?

From: Peter Dimov (pdimov_at_[hidden])
Date: 2002-01-18 15:27:11

From: "George A. Heintzelman" <georgeh_at_[hidden]>
> > From: "Thomas Maeder" <maeder_at_[hidden]>
> > > Am 2002.01.18 14:40 schrieb(en) Peter Dimov:
> > > > The original example was:
> > > >
> > > > shared_ptr<Derived> d(new Derived);
> > > > shared_ptr<Base> b(d);
> > > > d.reset();
> > > > b.reset();
> > > >
> > > > Which line invokes the undefined behavior? My answer is "none of
them"
> > > > and therefore I have implemented a version of shared_ptr that
handles the
> > > > example fine.
> > >
> > > My answer is: The second one.
> >
> > Possible, but inconvenient. There is no good reason to introduce
undefined
> > behavior here. Consider:
> >
> > shared_ptr<Derived> d(new Derived);
> > shared_ptr<Base> b(d);
> > b.reset();
> > d.reset();
> >
> > The second line is the same but it now works (with the current
> > implementation.)
>
> shared_ptr<T> x(0);
> if (&*x) {
>   x->foo();
> }
>
> This also works on current C++ implementations (all the ones I know
> about anyway). Nevertheless, this behavior is still undefined, and for
> good reason.
>
> So why not for the previous case?

Probably because

Base * p = new Derived;

is not undefined behavior.

If you read the current shared_ptr specification carefully, it implies that
the undefined behavior occurs on b.reset(), which is the last line, not on
the second (the implicit conversion.) This makes it the programmer's
responsibility to avoid this situation (IOW shared_ptr doesn't help in that
avoidance. ;-) ) So shared_ptr, as currently specified, allows the second
example but not the first.

Dave's original point was that shared_ptr should somehow prevent this, for
example by not allowing implicit conversions from Derived* to Base* when
Base is not polymorphic. This will make both examples invalid.

My opinion is that a shared_ptr that silently does the right thing is even
better; the correctness of the code snippets can be determined line by line,
in isolation, and the interaction between them and the order of execution
don't matter. (I've oversimplified a bit.)

IOW:

shared_ptr<Derived> d(new Derived); // OK since delete get() works

shared_ptr<Base> b(d); // OK, Derived* -> Base* implicit conversion

d.reset(); // OK, reset() has no precondition

b.reset(); // Ditto, order doesn't matter

> As has been pointed out elsewhere, this is not necessarily a problem
> for the destructor only, it is a more general problem when upcasting
> classes which overload base class functions non-virtually. The
> destructor is certainly the most common case, of course.
>
> IMHO this is NAD; this is one of the things C++ programmers are
> supposed to know about and understand when designing class hierarchies.

Agreed.

> I don't like adding the extra layer of indirection for *all* shared_ptr
> destructor calls, in order to 'solve' this, for the destructor case
> only. In a policy-based smart pointer, I would support making it an
> available policy but not the default one.

I'd be surprised if you could measure the added cost. Besides, the extra
layer of indirection has other good uses. It makes it possible to pass
shared_ptr<>s across exe/dll boundaries (that use different heaps.)

--
Peter Dimov
Multi Media Ltd.