Re: [boost] shared_xxx issues

From: Greg Colvin (gcolvin_at_[hidden])
Date: 2001-09-02 00:05:11

From: David Abrahams <david.abrahams_at_[hidden]>
> From: "Greg Colvin" <gcolvin_at_[hidden]>
> To: <boost_at_[hidden]>
> Sent: Saturday, September 01, 2001 1:23 PM
> Subject: Re: [boost] shared_xxx issues
> 
> > From: David Abrahams <david.abrahams_at_[hidden]>
> > > First Issue:
> > >
> > > I've just found myself wanting a really flexible smart pointer, along
> the
> > > lines of the one in Loki by Andrei Alexandrescu. For a long time I
> thought
> > > that I would rarely want more than shared_ptr, but I can see that this
> is
> > > going to come up again and again for me. Unfortunately, I couldn't use
> > > Andrei's design because very few compilers will apply the empty base
> > > optimization in cases of multiple inheritance. So, even if shared_ptr is
> our
> > > choice for submission to the committee I would really like to see a
> > > policy-based smart pointer in boost.
> >
> > Which policies is it that you want?
> 
> In this particular case, I wanted an embedded reference count, a specialized
> allocator, and an embedded size which could be passed to the allocator upon
> destruction.

Wouldn't Peter's design handle the last two items?

> > > 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.
> >
> > Of course the problem with an assert() is that it only detects
> > this one easy case, a harmless one, and misses the hard and truly
> > dangerous cases, and thus might lead to a false sense of security.
> 
> I think you have the right policy, but in the wrong place. That policy
> applies well to documented external behavior, like the conditions under
> which an exception will be thrown. Assertions should not be documented per
> se, since they are just checks for preconditions, and not all preconditions
> can be checked. All preconditions, however, should be documented.
> 
> Admittedly, the chances that a shared_ptr will be reset to point at the same
> object are slim, but if it /does/ happen, there's a good chance that the
> same code could reset it to point at some other object which is already
> managed (to see this, ask yourself, "why would a programmer ever
> intentionally reset() a smart pointer to the same value?"  I can think of
> reasons, but it's a bit of a stretch). This is is the only chance we get to
> catch a very common source of bugs. Currently, we have a precondition like
> this one for reset (is it documented?): "the argument must not point to an
> object whose lifetime is managed by another smart pointer...unless it's the
> same one that you are calling reset on". I propose to take out the exception
> for this one case so that we stand a chance of detecting real bugs.

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.

> > It is possible of course to maintain a list or set of pointers
> > to all shared objects and assert that p is not in the set of shared
> > objects, as my cyclic_ptr experiment did.
> 
> But, as you also said, it doesn't work for multiple-inheritance.

I'd forgetten that, but you're right.  There is no conforming way
to check this precondition.  But it would be possible

> > As a general rule any use of the raw pointer functions of shared_.*
> > should be hidden away from ordinary use behind a create/recreate
> > interface.  I would be happy to deprecate all these functions
> > if suitable replacements could be found.
> 
> What is recreate?

If you have a function like

   shared_ptr<stuff> create_stuff(T arg) {
      return shared_ptr<stuff> (new stuff(arg));
   }

then a function like

   void recreate_stuff(stuff_ptr& p, T arg) {
      p.reset(new stuff(arg));
   }

might be more efficient than

   my_stuff_ptr = create_stuff(arg);