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

From: David B. Held (dheld_at_[hidden])
Date: 2003-01-31 14:15:49

"David Abrahams" <dave_at_[hidden]> wrote in message
news:u8yx1z2f5.fsf_at_boost-consulting.com...
> [...]
> If you want to subsume the current shared_ptr and auto_ptr you'd
> better handle it.

Ok, point taken.

> There are other ways to prevent indiscriminate mixing (an orthogonal
> "interoperability policy", perhaps?)

Actually, you can control mixing by determining which policies can be
constructed from which other policies.  I just didn't know if it was a
good idea to let users freely mix auto_ptr with shared_ptr.  But I
suppose if they get an auto_ptr from legacy code, and they want it to be
managed by shared_ptr, that is a legitimate use.

> [...]
> In principle you don't have to promise to delete the unmanaged
> resource if your constructor throws; it's just your desire to be
> usable with "new T" that makes it neccessary.

Ok, I think I get it.  Whereas, if a policy were to throw, and leak an
internally allocated count, then the c'tor does not even provide the
basic guarantee?

> In fact, I have been arguing for years that our smart pointers should
> never have had a public interface which adopts unmanaged resources
>on construction.  Instead, we should write:
>
>      std::auto_ptr<T> = std::auto_ptr_new<T>(arg1, arg2, arg3);
>
> Voila, a managed T straight out of the box.

It might be nice if you assigned it to a variable. ;>

> However, the desire to subsume existing smart pointers means you
>have to support the unsafe "unmanaged adoption interface."

Maybe we can eat our cake and have it too.

> [...]
> The move proposal says that standard components will require
> the move construct/assign of their UDT parameters, if any, to be
> nothrow.  In practice that would be needed by any generic
> components.  That's enough to make the move undo-able.

Ok, but that still leaves smart_ptr with a few tricks to pull off.  Let's
look at smart_ptr(std::auto_ptr p) for a minute:

smart_ptr(std::auto_ptr<U>& p)
    : storage(p), ownership(p), checking(), conversion()
{ checking::on_init(p); }

Now, the first that happens is that storage(p) attempts to take
ownership of p.  For default storage, this is a no-throw operation.
Let's say we're moving p into a counted pointer.  Now ownership
has to allocate a count, and might throw in the process.  If it does,
we either have to clean up p, or we have to stuff the toothpaste
back into the tube.

I see that shared_ptr solves this by violating RAII.  I don't think
Andrei included this case in the original Loki::SmartPtr, but I
think this new design will solve the problem.  We simply must
insist that for some policy configurations, storage(p) is no-throw.
I don't think that's a burden, because in the vast majority of cases
where it matters, the implementation will already be no-throw.
Given that, we have seen that there are policy configurations
where we need the ownership policy to tell us how to clean up
in the event of an exception.  This is my proposed solution:

template <class StoragePolicy>
ref_counted(std::auto_ptr<T>& p, StoragePolicy& storage)
try
    : count_(new unsigned(1))
{
    p.release();
}
catch (...)
{
    storage.release();
}

Unfortunately, it requires a function try block. :(  However, we
might be able to apply Dave's "Initializer ScopeGuard(TM)" to
this problem, by converting a function try block into an RAII
functoid.  Let's see what happens:

template <class StoragePolicy>
class InitializerScopeGuard
{
public:
    InitializerScopeGuard(StoragePolicy& storage)
        : execute_(true), storage_(storage)
    { }
    ~InitializerScopeGuard()
    { if (execute_) storage::release(); }
    void cancel()
    { execute_ = false; }
private:
    bool execute_;
    StoragePolicy& storage;
};

template <typename P>
class ref_counted_impl
{
public:
    template <typename U>
    ref_counted_impl(std::auto_ptr<U>& p, InitializerScopeGuard guard)
        : count_(new unsigned(1))
    {
        guard.cancel();
        p.release();
    }
    // ...
private:
    unsigned* count_;
};

template <typename P>
class ref_counted : ref_counted_impl<P>
{
public:
    template <typename U, class StoragePolicy>
    ref_counted(std::auto_ptr<U>& p, StoragePolicy& storage)
        : ref_counted_impl(p, InitializerScopeGuard(storage))
    { }
    // ...
};

smart_ptr(std::auto_ptr<U>& p)
    : storage(p), ownership(p, static_cast<StoragePolicy&>(*this)),
    checking(), conversion()
{ checking::on_init(p); }

Now, we insist that for this configuration, storage(p) is no-throw.

1) Thus, it will successfully acquire p, but both storage and auto_ptr
will own p.

2) For this c'tor, we insist that ownership follow the protocol
established for ref_counted.  That means the following:

a) If ownership(...) throws, it must call storage_policy::release(),
so that storage does not attempt to clean up the resource.

b) If ownership(...) does not throw, it must call auto_ptr::release(),
so that possession is fully transferred.

3) We must insist that checking and conversion be no-throw unless
p is null.

Note that we are using the default storage policy from before.  It
requires no modification under this scenario.

Now, let's consider intrusive_counted again.  After thinking this
over, it seems reasonable to me to impose the following condition:
Policy c'tors for an intrusive_ptr configuration must be no-throw,
except when p is null.  This doesn't seem like an
unreasonable requirement, because acquiring the resource almost
certainly shouldn't involve copying, and the ownership policy will
not need to allocate anything (at least for the default intrusive
policy). In general, checking will only throw on_init for null pointers
anyway.  But if checking is smart enough to detect an invalid
pointer, we will have to rethink the strategy.

This design does break orthogonality just a little bit by making
one of ref_counted's c'tors aware of storage_policy.  However,
I don't see any way around that.

Dave