[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: template <typename T> class allocator { ... typedef smart_ptr<T> pointer; void deallocate(pointer & __p) { delete p.release(); } }; -Phil
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
On Thu, Aug 28, 2008 at 12:27, Michael Marcin <mike.marcin@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
On Thu, Aug 28, 2008 at 1:54 PM, Scott McMurray <me22.ca+boost@gmail.com> wrote:
On Thu, Aug 28, 2008 at 12:27, Michael Marcin <mike.marcin@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
Regards, -- Felipe Magno de Almeida
On Thu, Aug 28, 2008 at 13:05, Felipe Magno de Almeida <felipe.m.almeida@gmail.com> wrote:
On Thu, Aug 28, 2008 at 1:54 PM, Scott McMurray <me22.ca+boost@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. 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).
"Scott McMurray" <me22.ca+boost@gmail.com> wrote in message news:fa28b9250808281452t49546ee0k4f320d23ebee3de5@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
On Fri, Aug 29, 2008 at 02:01, Phil Bouchard <philippe@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
"Scott McMurray" <me22.ca+boost@gmail.com> wrote in message news:fa28b9250808290914l19a6cf1fue658229148ec0f09@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; A2 a2( a_ ); this->~this_type(); a2.deallocate( this, 1 ); // ugh! } }; This is ill-formed code. It's obvious shared_ptr is still in its theoretical stage as far as the allocator is concerned; because std::allocator is a bad example to start with. Let's take a real one like the following: // allocator template <typename T> class my_allocator { boost::fast_pool_allocator pool_; ... void deallocate(pointer & p) { pool_.deallocate(p.release()); // do not touch "this" anymore starting here!! } }; The way shared_ptr works right now, the pool is going to be copied entirely on to the sp_counted_impl_pda object. The problem here is that the pool is going to delete its own instance when deallocate() is called. The allocator variable sp_counted_impl_pda::a_ will need to be changed for a functor or a global function pointer before anything else. Moreover using and allocator explicitly using release() saves some unnecessary overhead for tested classes and library programmers. One might wonder why using a smart pointer for well tested classes? Well it turns out shifted_ptr is much more efficient this way because all node allocations will be part of the same set. -Phil
On Fri, Aug 29, 2008 at 23:21, Phil Bouchard <philippe@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...
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 transitive. X a1(a); post: a1 == a X a(b); post: Y(a) == b, a == X(b) Your example is not an Allocator.
"Peter Dimov" <pdimov@pdimov.com> wrote in message news:006401c90a95$cfd70570$6507a80a@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.
Interesting but unfortunate we cannot mix pools within allocators because it makes allocators even less flexible (useful). Secondly the pointer argument of deallocate() and destroy() should be passed in as references because: - you have better control - the allocator knows what need to be done according to the pointer type And shouldn't necessarily be the same as the one returned by allocate(). -Phil
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.
"Peter Dimov" <pdimov@pdimov.com> wrote in message news:00e401c90b0d$2994d570$6507a80a@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 return p; if (p = pb_->ordered_malloc(s)) // 2) thread local heap return p; if (p = ::operator new (s)) // 3) system heap return p; return p; } }; Two pointers here will have to be copied on each sp_counted_impl_pda object (sizeof(pb_) == sizeof(void *) with pthreads). It's not too bad when we use pointers but hopefully there will be some assertion preventing usage of pools not following the new standards inside shared_ptr, because my own internal sh_owned_base_nt.hpp pool never considered this and the mistake can easily be propagated. -Phil
"Phil Bouchard" <philippe@fornux.com> wrote in message news:g983c4$ecb$1@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
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
participants (6)
-
Felipe Magno de Almeida -
Michael Marcin -
Peter Dimov -
Phil Bouchard -
Scott McMurray -
Sebastian Redl