Re: [boost] SmartPtr - Exception safety reprise

From: David Abrahams (dave_at_[hidden])
Date: 2003-02-01 13:44:45

"David B. Held" <dheld_at_[hidden]> writes:

> I started a new thread because the old one was getting hard to
> follow on my newsreader from the excessive nesting.  I realized
> there is yet another problem with my scheme (which should come
> as no suprise by now).  This time, the problem is the copy c'tor.
> Here's a recap:
>
>     storage::storage(storage const& rhs)
>     : pointee_(rhs.pointee_)
>     { }
>
>     storage::~storage()
>     { checked_delete(pointee_); }
>
>     void storage::release()
>     { pointee_ = default_value(); }
>
>     ref_counted::ref_counted(ref_counted const& rhs)
>     : count_(rhs.count_)
>     { }
>
>     ref_counted::~ref_counted()
>     { delete count_; }
>
>     bool ref_counted::release(P const& p)
>     {
>         if (!--*count_) return true;
>         count_ = 0; return false;
>     }
>
>     smart_ptr::smart_ptr(smart_ptr const& rhs)
>     : storage(rhs), ownership(rhs), checking(rhs), conversion(rhs)
>     { checking::on_init(p); }
>
>     smart_ptr::~smart_ptr()
>     {
>         if (!ownership::release(get_impl(*this)))
>         {
>             storage::release();
>         }
>     }
>
> The problem is that while ref_counted indeed cleans up its count in
> the face of later exceptions, it may do so prematurely.  For instance,
> if checking(rhs) were to throw, ~smart_ptr() does not get a chance
> to call ownership::release(), which prevents ownership from deleting
> the count.  Similarly, if, for some reason, ownership(rhs) were to
> throw, storage might prematurely free the resource.  The two most
> viable solutions I can think of are:
>
> A) add scope guards to make sure the right thing is done no matter
> who throws
>
> B) require that copying be no-throw

  C) Make sure the first base class of smart_ptr is the one that
     manages destruction of its resources:

  template <class Storage, class Ownership>
  struct ptr_manager
     : Storage, Ownership
  {
     ptr_manager(ptr_manager const& p) : Storage(p), Ownership(p) {}

     ~ptr_manager()
     {
         if (!this->Ownership::release(get_impl(*this)))
         {
             this->Storage::release();
         }
      }
  };

   smart_ptr::smart_ptr(smart_ptr const& rhs)
      : ptr_manager(rhs), checking(rhs), conversion(rhs)
   { checking::on_init(p); }

This is just more of the same RAII principles we've been discussing at
work.

I think that whatever orthogonal concepts are being represented by
ownership and storage here (and I'm really not sure I see a clear
separation), they must be sub-concepts of what is normally referred to
as ownership.  You can see that from the fact that you need a
sub-object to group them together to handle resource lifetime
correctly.  I would explore/discuss the implications of not separating
ownership and storage to see whether it's buying you more than it's
costing.

BTW, I don't want to beat this to death, but addressing the issue of
most compilers not doing the EBO for anything other than the first
base class has a huge bearing on all of this, because solving it means
you have to change the base class organization.  For me at least, one
of the primary reasons to use a policy-based smart pointer would be to
get the optimizations it can offer for really time-critical code.  In
fact, at the moment that's the *only* reason I can imagine wanting
something other than boost::shared_ptr<>.  If shared_ptr doesn't have
optimal footprint on most compilers I would be reluctant to use it
where efficiency matters most, and thus it would become useless to me.
I don't know about other people.

-- 
                       David Abrahams
   dave_at_[hidden] * http://www.boost-consulting.com
Boost support, enhancements, training, and commercial distribution