headers
Phil Bouchard | 28 Aug 07:05

[smart_ptr] release()

As it turns out I think a release() member function should be added to the 
smart pointer.  Its purpose is to give away ownership of the object.  See 
the following pseudo-code:

template <typename T>
    class smart_ptr
    {
        ...
        smart_ptr(element_type *);

        value_type * get();

        element_type * release()
        {
            element_type * __p = pointer_ref();

            pointer_ref() = 0;

            if (-- counter_ref())
                return 0;
            else
                return __p;
        }
    };

It follows the same idea of its predecessor auto_ptr<> but will return a 
null pointer if the object is still shared thus not ready to be deleted. 
This way the allocator will be much easier to support either by calling this 
function before deallocate(), inside it or by using a specialized wrapper 
class to handle it:
(Continue reading)

Permalink | Reply |
headers
Sebastian Redl | 28 Aug 13:36
Favicon

Re: [smart_ptr] release()

Phil Bouchard wrote:
> As it turns out I think a release() member function should be added to the 
> smart pointer.
Which smart_ptr?

I think scoped_ptr should have a release(). It's technically impossible 
to add a useful one to the refcounted smart pointers.

Sebastian
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Permalink | Reply |
headers
Michael Marcin | 28 Aug 18:27

Re: [smart_ptr] release()

Sebastian Redl wrote:
> Phil Bouchard wrote:
>> As it turns out I think a release() member function should be added to 
>> the smart pointer.
> Which smart_ptr?
> 
> I think scoped_ptr should have a release(). It's technically impossible 
> to add a useful one to the refcounted smart pointers.
> 

It works for refcounted smart pointers iff p.unique() == true.

Thanks,

Micahel Marcin

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Permalink | Reply |
headers
Scott McMurray | 28 Aug 18:54

Re: [smart_ptr] release()

On Thu, Aug 28, 2008 at 12:27, Michael Marcin <mike.marcin <at> gmail.com> wrote:
> Sebastian Redl wrote:
>>
>> Phil Bouchard wrote:
>>>
>>> As it turns out I think a release() member function should be added to
>>> the smart pointer.
>>
>> Which smart_ptr?
>>
>> I think scoped_ptr should have a release(). It's technically impossible to
>> add a useful one to the refcounted smart pointers.
>>
>
> It works for refcounted smart pointers iff p.unique() == true.
>

Which means it's likely a race condition if you try to do it in a
multi-threaded program, because a weak_ptr could get lock()ed at any
point.  Custom deleters also make it quite awkward to handle properly.

Since it would require that only one pointer holds it, it seems like
the correct way to do it is to use unique_ptr instead, which enforces
that invariant.

~ Scott
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
(Continue reading)

Permalink | Reply |
headers
Felipe Magno de Almeida | 28 Aug 19:05

Re: [smart_ptr] release()

On Thu, Aug 28, 2008 at 1:54 PM, Scott McMurray <me22.ca+boost <at> gmail.com> wrote:
> On Thu, Aug 28, 2008 at 12:27, Michael Marcin <mike.marcin <at> gmail.com> wrote:
>> Sebastian Redl wrote:

[snip]

>>> I think scoped_ptr should have a release(). It's technically impossible to
>>> add a useful one to the refcounted smart pointers.
>>
>> It works for refcounted smart pointers iff p.unique() == true.
>
> Which means it's likely a race condition if you try to do it in a
> multi-threaded program, because a weak_ptr could get lock()ed at any
> point.

You can use the same technique that ~shared_ptr does.
Decrement it atomically and then check the old value.

> Custom deleters also make it quite awkward to handle properly.

Why?

> Since it would require that only one pointer holds it, it seems like
> the correct way to do it is to use unique_ptr instead, which enforces
> that invariant.

True. But it might be useful when you need to comply with
an interface.

> ~ Scott
(Continue reading)

Permalink | Reply |
headers
Scott McMurray | 28 Aug 23:52

Re: [smart_ptr] release()

On Thu, Aug 28, 2008 at 13:05, Felipe Magno de Almeida
<felipe.m.almeida <at> gmail.com> wrote:
> On Thu, Aug 28, 2008 at 1:54 PM, Scott McMurray <me22.ca+boost <at> gmail.com> wrote:
>> Custom deleters also make it quite awkward to handle properly.
>
> Why?
>
>> Since it would require that only one pointer holds it, it seems like
>> the correct way to do it is to use unique_ptr instead, which enforces
>> that invariant.
>
> True. But it might be useful when you need to comply with
> an interface.
>

Because you have to store the pointer and the deleter somewhere
anyways in order to clean it up properly after a hypothetical release.
 Since the shared_ptr is already doing that for you, why wouldn't you
just keep it around until you're done with it?

If you're receiving arbitrary shared_ptrs, then it's quite possible
that unique will never be true, so you can't be certain you can ever
release it in the first place.

If you're just doing it to conform to an interface, then you don't
know if the library is holding a copy, so you can't rely on being able
to release it.

If you have control over the whole stack, then you can use a unique_ptr instead.
(Continue reading)

Permalink | Reply |
headers
Phil Bouchard | 29 Aug 08:01

Re: [smart_ptr] release()


"Scott McMurray" <me22.ca+boost <at> gmail.com> 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:
(Continue reading)

Permalink | Reply |
headers
Scott McMurray | 29 Aug 18:14

Re: [smart_ptr] release()

On Fri, Aug 29, 2008 at 02:01, Phil Bouchard <philippe <at> fornux.com> wrote:
>
> It's not a good design idea having that allocator passed in to the smart
> pointer's constructor.  Here's why:
>
> [snip]
>
> This basically means that shared_ptr allocator implicit deletion idea only
> works with shared_ptr.  It breaks consistency with everybody else.
>

It's not an allocator, and it's often useful in strange ways:

    shared_ptr<FILE> f( fopen("file.txt"), fclose );

The fact that the deleter is specified on initialization is also
essential in that it allows you to create shared_ptrs to incomplete
types:

    struct S { auto_ptr<S> p; }; // illegal
    struct S { shared_ptr<S> p; }; // fine

So I don't really see your point, here.

~ Scott
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Permalink | Reply |
headers
Phil Bouchard | 30 Aug 05:21

Re: [smart_ptr] release()


"Scott McMurray" <me22.ca+boost <at> gmail.com> wrote in message 
news:fa28b9250808290914l19a6cf1fue658229148ec0f09 <at> mail.gmail.com...

[...]

> It's not an allocator, and it's often useful in strange ways:
>
>    shared_ptr<FILE> f( fopen("file.txt"), fclose );
>
> The fact that the deleter is specified on initialization is also
> essential in that it allows you to create shared_ptrs to incomplete
> types:
>
>    struct S { auto_ptr<S> p; }; // illegal
>    struct S { shared_ptr<S> p; }; // fine
>
> So I don't really see your point, here.

I think you're confusing the allocator and the deleter but I am exclusively 
referring to the allocator.  I just looked at the code of shared_ptr today 
and it is very similar to what I wrote yesterday:

template<class P, class D, class A>
    class sp_counted_impl_pda: public sp_counted_base
    {
        ...
        virtual void destroy() // nothrow
        {
            typedef typename A::template rebind< this_type >::other A2;
(Continue reading)

Permalink | Reply |
headers
Scott McMurray | 30 Aug 06:40

Re: [smart_ptr] release()

On Fri, Aug 29, 2008 at 23:21, Phil Bouchard <philippe <at> fornux.com> wrote:
>
> I think you're confusing the allocator and the deleter but I am exclusively
> referring to the allocator.
>

Fair enough.  I was confused by the fact that nothing in the thread
had been discussing the allocator, and there was no paragraph with any
kind of smooth transition.

Though nothing prevents an allocator from either using entirely global
state or from just including a pointer to the actual storage manager,
which could both be conceivably be useful, and don't have the problem
you mention...
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Permalink | Reply |
headers
Peter Dimov | 30 Aug 13:44

Re: [smart_ptr] release()

Phil Bouchard:

> template<class P, class D, class A>
>    class sp_counted_impl_pda: public sp_counted_base
>    {
>        ...
>        virtual void destroy() // nothrow
>        {
>            typedef typename A::template rebind< this_type >::other A2;
>
>            A2 a2( a_ );
>
>            this->~this_type();
>            a2.deallocate( this, 1 );
>            // ugh!
>        }
>    };
>
> This is ill-formed code.

I don't think you know what "ill-formed" means.

> The way shared_ptr works right now, the pool is going to be copied 
> entirely on to the sp_counted_impl_pda object.

shared_ptr requires that the A argument conforms to the Allocator 
requirements in the standard (Table 40 in N2691):

a1 == a2 bool returns true iff storage allocated from each can be 
deallocated via the other. operator== shall be reflexive, symmetric, and
(Continue reading)

Permalink | Reply |
headers
Phil Bouchard | 31 Aug 02:49

Re: [smart_ptr] release()


"Peter Dimov" <pdimov <at> pdimov.com> wrote in message 
news:006401c90a95$cfd70570$6507a80a <at> pdimov2...

[...]

> I don't think you know what "ill-formed" means.

I meant dangerous if not classified undefined when you delete the class 
indirectly (wrote too fast).  Now I just realized destroy() should be used:

        a2.destroy( this );
        a2.deallocate( this, 1 );

>> The way shared_ptr works right now, the pool is going to be copied 
>> entirely on to the sp_counted_impl_pda object.
>
> shared_ptr requires that the A argument conforms to the Allocator 
> requirements in the standard (Table 40 in N2691):
>
> a1 == a2 bool returns true iff storage allocated from each can be 
> deallocated via the other. operator== shall be reflexive, symmetric, and 
> transitive.
>
> X a1(a); post: a1 == a
>
> X a(b); post: Y(a) == b, a == X(b)
>
> Your example is not an Allocator.
(Continue reading)

Permalink | Reply |
headers
Peter Dimov | 31 Aug 03:58

Re: [smart_ptr] release()

Phil Bouchard:

> Interesting but unfortunate we cannot mix pools within allocators because 
> it makes allocators even less flexible (useful).

You can mix pools, but all copies must share the same pool. A conforming 
allocator should not contain the pool itself, but a pointer to the pool. Two 
allocators should compare equal when they point to the same pool.

It doesn't make much sense for each shared_ptr to use its own private pool 
anyway. shared_ptr only allocates once. 

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Permalink | Reply |
headers
Phil Bouchard | 31 Aug 20:56

Re: [smart_ptr] release()


"Peter Dimov" <pdimov <at> pdimov.com> wrote in message 
news:00e401c90b0d$2994d570$6507a80a <at> pdimov2...
> Phil Bouchard:
>
>> Interesting but unfortunate we cannot mix pools within allocators because 
>> it makes allocators even less flexible (useful).
>
> You can mix pools, but all copies must share the same pool. A conforming 
> allocator should not contain the pool itself, but a pointer to the pool. 
> Two allocators should compare equal when they point to the same pool.
>
> It doesn't make much sense for each shared_ptr to use its own private pool 
> anyway. shared_ptr only allocates once.

If we take a better allocator example having multiple pools, having searches 
starting with the fastest one (stack).  Here's the pseudo-code:

template <typename T>
    class quick_allocator
    {
        auto_pool * pa_;
        boost::thread_specific_ptr< boost::pool<> > pb_;

        ...
        void * allocate(std::size_t s)
        {
            void * p = 0;

            if (p = pa_->alloc(s)) // 1) stack array of fixed size
(Continue reading)

Permalink | Reply |
headers
Phil Bouchard | 9 Sep 11:20

Re: [smart_ptr] release()


"Phil Bouchard" <philippe <at> fornux.com> wrote in message 
news:g983c4$ecb$1 <at> ger.gmane.org...

[...]

> 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.

[...]

This statement was subjective.  I could not see the usefulness of supporting 
allocators inside shared_ptr if it couldn't be used by containers.  It turns 
out both are necessary:
- release() for generic coding and
- implicit deletion obviously for nested destruction of nodes (stack nodes 
for example)

At the same time release() makes the smart pointer reversible and should 
return the exact same pointer type used by the constructor and assignment 
operator parameters.

-Phil
(Continue reading)

Permalink | Reply |
headers
Sebastian Redl | 28 Aug 23:21
Favicon

Re: [smart_ptr] release()

Michael Marcin wrote:
> Sebastian Redl wrote:
>> I think scoped_ptr should have a release(). It's technically 
>> impossible to add a useful one to the refcounted smart pointers.
>>
>
> It works for refcounted smart pointers iff p.unique() == true.
Well, the "useful" part of my post is a matter of opinion, of course, 
but I can't think of a case where I would want a release() that might 
fail, ergo it's not useful.

Sebastian
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Permalink | Reply |

Gmane