Re: [boost] [smart_ptr] release()

From: Phil Bouchard (philippe_at_[hidden])
Date: 2008-08-29 02:01:38

"Scott McMurray" <me22.ca+boost_at_[hidden]> wrote in message 
news:fa28b9250808281452t49546ee0k4f320d23ebee3de5_at_mail.gmail.com...

[...]

> Or if you really want, there's already sufficient infrastructure to do
> cleanup externally to the shared_ptr.  Just use a nop deleter and
> something like this:
> T *release_assuming_nop_deleter(shared_ptr<T> &sp) {
>    T *p = sp.get();
>    weak_ptr<T> wp = sp;
>    sp.reset();
>    if (shared_ptr<T> nsp = wp.lock()) {
>        sp = nsp;
>        return 0;
>    } else {
>        return p;
>    }
> }
>
> I think that it shouldn't be in the code, though, or it's just asking
> for people to do delete sp.release(); (though likely less directly
> than that) without realizing that it's wrong.  At the very least, if
> it absolutely had to be added, release() would need to return a
> pair<T*, boost::function<void(void*)> > (or similar).

It's not a good design idea having that allocator passed in to the smart 
pointer's constructor.  Here's why:

1)
Allocators are designed from bottom up for containers because nodes are 
allocated in important quantities.  Who cares about 1 or 2 shared_ptrs 
referring to a block from a local pool?  What happens if you forget passing 
in the allocator for pointer9?  This will encourage messy code.

People should know what they're doing if they start using allocators in the 
first place.

2)
What happens if the allocator has member variables?  The member variables 
will be copied over on all memory blocks pointed too?

3)
If the class or function using the smart pointer is the one responsible for 
allocating a block of memory then it should remain it's own responsibility 
deallocating it.

4)
This design doesn't work at all with shifted_ptr because the allocator 
functor will be calling the member function to destroy and deallocate its 
own memory block.  You better not call any more member function from that 
deallocation function!

// memory block part 1
template <typename T>
    class shifted : public owned_base
    {
        T elem_;
        ...
    };

 // memory block part 2
template<typename T, class A>
    class shifted_a : public shifted<T>
    {
        A a_;
        ...
    };

 // smart pointer
template <typename T>
    class shifted_ptr
    {
        ...
        void reset()
        {
            if (! -- counter_ref())
                // somehow calls deallocate()
                allocator_ref().operator () (* this);

            pointer_ref() = 0;
        }
    };

// allocator
template <typename T>
    class shifted_allocator
    {
        ...
        void deallocate(pointer & p)
        {
            delete p.release();
            // do not touch "this" anymore starting here!!
        }
    };

This basically means that shared_ptr allocator implicit deletion idea only 
works with shared_ptr.  It breaks consistency with everybody else.

-Phil