[boost] Re: SmartPtr (Loki) - auto_ptr/move c'tor issue

From: David B. Held (dheld_at_[hidden])
Date: 2003-01-30 15:11:43

"David Abrahams" <dave_at_[hidden]> wrote in message
news:u1y2ujw8o.fsf_at_boost-consulting.com...
> [...]
> That's a very imprecise description, and exactly what I mean by
> "not sure what you really wanted."  Even though you think you have
> an answer now, I want to encourage you to write down very carefully
> what you think should happen in case an exception occurs at each
> possible point.  It might even tell you why your current design is
> "obviously" the right one.

All right, on the off chance that you or someone else spots a flaw in
the logic, here goes:

smart_ptr(P p)
    : storage(p), ownership(p), checking(), conversion()
{ checking::on_init(p) }

1) storage(p) throws

storage takes ownership of p immediately, so it must also clean up
p in the event of an exception.  None of the other policies are c'ted
yet, so we have no less than the basic guarantee, with the premise
that smart_ptr(P) takes ownership of p immediately.

2) ownership(p) throws

storage(p) must have succeeded, so it is a fully c'ted subobject.
ownership is responsible for cleaning up its own resources, but
nothing else.  ~storage() will get called, cleaning up p.

3) checking() throws

Unlikely, but the reasoning is the same as 2).  checking is
responsible for cleaning up its own resources, but the fully c'ted
storage will clean up p, because it "owns" it.  Since ownership
is fully c'ted, it...oops!  I just realized that ownership will leak its
count. :(  This was a good exercise after all!  Let's fix ref_counted:

        ~ref_counted()
        {
            delete pCount_;
        }

        bool release(P const&)
        {
            if (!--*pCount_) return true;
            pCount_ = 0;
            return false;
        }

Ok, now ownership will properly clean up after itself.

4) conversion() throws

Even less likely, but the same reasoning as 2).  conversion will
almost certainly have no resources of its own, but the fully c'ted
storage will clean up p, and ownership will clean up itself.

5) on_init(p) throws

Since 1), 2), 3), and 4) are safe, and on_init(p) acquires no new
resources, it inherits the same level of safety.

> [...]
> > But in order to solve the problem with too many c'tors for the
> > move configuration,
>
> "Lots of constructors" is one of the main reasons to factor it into
> a resource-managing base class and an interface-providing
> derived facade.

Hmm...in this case, I only have one too many c'tors.  Unfortunately,
that's one too many. :(

> [...]
> I love it when people put trademarked names on trivial things I've
> been doing since at least '96.  Well, alright, maybe I don't ;-)

You just didn't have the marketing saavy to come up with a catchy
name for it. ;>

> [...]
> It'll be interesting to see how well optimizers do with smart_ptr
> destructors, i.e.  will they be able to deduce that pointee_ has
> already been nulled out and eliminate the test and call to
> checked_delete,

I don't see how they could.

> and will they be able to then detect that the smart_ptr is being
> destroyed anyway and optimize away the setting of pointee_ to
> 0?

But that changes whether checked_delete actually does anything,
no??

> All theoretically possible.  We may need to give checked_delete
> a throw() specification on some platforms to get there.

You're gonna have to clue me in, because I don't see how a
compiler could make any of those deductions.

> [...]
> We'll have to see about efficiency.  My approach was biased
> towards paying in the constructor where things are probably
> expensive anyway and leaving the destructor as simple as
> possible.

I agree that a trivial d'tor is always nice.  And that particular
c'tor is probably going to be more expensive than the other c'tors
anyway.  I just figure that zeroing a pointer is pretty cheap, and
when the d'tor doesn't have to actually delete anything, it's just one
pointer test, which doesn't seem like a terrible price to pay for fairly
simple exception safety.

> [...]
> This might express that notion better, though I don't remember
> enough about the policies to say whether it's correct (also, you
> might want to rename one of the 'release' functions, since they
> seem to do different things):
>
>     smart_ptr::~smart_ptr()
>     {
>         ownership_policy::release(storage_policy::release());
>     }
> [...]

Well, the way I see it, release() is relative to the policy.  First,
ownership releases a reference, which might release
ownership's claim to the resource.  If ownership can't release
shared possession of the resource, then storage must release
the resource to ownership.

So even though storage has real physical possession of the
resource, ownership holds the logical shared possession of the
resource among smart pointers.  While the smart pointer is live,
this "power sharing" is viable.  In the end, however, only one of
them can determine the fate of the resource, so they each try to
release the resource to the other.

Of course, order is significant, which is why I wrote it this way:

smart_ptr::~smart_ptr()
{
    if (!ownership_policy::release(get_impl(*this)))
    {
        storage_policy::release();
    }
}

We could write it this way:

smart_ptr::~smart_ptr()
{
    storage_policy::release(
        !ownership_policy::release(get_impl(*this))
    );
}

After inlining, I think these are equivalent, but the first form
more clearly shows the ownership_policy trying to release
its claim first.

Dave