Re: [boost] shared_xxx issues

From: Peter Dimov (pdimov_at_[hidden])
Date: 2001-09-02 05:41:41

From: "Greg Colvin" <gcolvin_at_[hidden]>
> From: David Abrahams <david.abrahams_at_[hidden]>

[...]

> > > > Second Issue:
> > > >
> > > > I found the following code in shared_array (and I assume the
analogous code
> > > > is in shared_ptr):
> > > >
> > > >         void reset(T* p=0)
> > > >         {
> > > >             if ( px == p ) return;  // fix: self-assignment safe
> > > >
> > > > What this does is to make a silent success of resetting a shared_ptr
with
> > > > the pointer it owns. The problem with that is that resetting a
shared_ptr to
> > > > any /other/ pointer which is managed by a shared_ptr is a fatal (and
> > > > hard-to-detect) error, which happens even to experienced
programmers. I
> > > > don't think the trade-off is a good one. Making this special case
(which we
> > > > can detect) silently work when all other cases will fail without
complaint
> > > > is a mistake. I'd rather see an assert() here.

[...]

> You've almost convinced me.  What is bugging me is a desire to
> stay consistent with auto_ptr:
>
>    void reset(X* p=0) throw();
>       Effects:
>          If get() != p then delete get().
>       Postconditions:
>          *this holds the pointer p.

I think that Dave is right. I don't know why this made it into
auto_ptr::reset, but the above shared_ptr::reset does not conform to its
definition:

"shared_ptr reset

void reset( T* p=0 );
First, if use_count() == 1, deletes the object pointed to by the stored
pointer. Otherwise, use_count() for any remaining copies is decremented by
1.

Then replaces the contents of this, as if by storing a copy of p, which must
have been allocated via a C++ new expression or be 0. Afterwards,
use_count() is 1 (even if p==0; see ~shared_ptr). Note that in C++  delete
on a pointer with a value of 0 is harmless.

The only exception which may be thrown is std::bad_alloc.  If an exception
is thrown,  delete p is called."

The 'fix' makes the reset() a no-op, and this is not allowed by the
specification. Note the effect 'decrements use_count for remaining copies'
and the postcondition 'use_count() == 1'. (BTW an assert is not allowed
either by this definition. The undefined behavior would happen at
destruction, not at reset.)

That's why in my implementation (after staring at the current
shared_ptr::reset() code for a while) I simply used

void reset(T * p = 0)
{
    shared_ptr(p).swap(*this);
}

--
Peter Dimov
Multi Media Ltd.