Boost shared pointers, PThreads and void* casts
I haven't been able to find any clear explanation about this, so here it goes: I'm coding a multithreaded program using pthreads. The general behaviour is this: -A function starts. -The function creates a shared pointer "OBJ". -The function creates a Thread, passing "OBJ" as the parameter (having to cast it as void*). -The thread starts, getting a (void*) param which is casted back to "OBJ" (shared pointer type). -The function ends, and therefore its "OBJ" goes out of scope. -The thread" ends, so its "OBJ" goes out of scope too. (of course, the thread could at times end before the function instead) I'm afraid that the shared pointer does not behave well due to those nasty void* casts... maybe trying to delete twice the object. What's the proper way of coding this? Weak pointers maybe? Thanks a lot for any help. -- Saludos, Bruno González _______________________________________________ Msn/Jabber: stenyak AT gmail.com ICQ: 153709484 http://www.stenyak.com
boost-users-bounces@lists.boost.org <> wrote:
I haven't been able to find any clear explanation about this, so here it goes:
I'm coding a multithreaded program using pthreads. The general behaviour is this:
-A function starts. -The function creates a shared pointer "OBJ". -The function creates a Thread, passing "OBJ" as the parameter (having to cast it as void*). -The thread starts, getting a (void*) param which is casted back to "OBJ" (shared pointer type). -The function ends, and therefore its "OBJ" goes out of scope. -The thread" ends, so its "OBJ" goes out of scope too. (of course, the thread could at times end before the function instead)
I'm afraid that the shared pointer does not behave well due to those nasty void* casts... maybe trying to delete twice the object. What's the proper way of coding this? Weak pointers maybe?
Thanks a lot for any help.
The first problem I see is that a boost::shared_ptr is not the same size as a void*. The believe the cast would slice off the pointer to the reference count. I would try something like the following (untested): //Notice that we are passing the shared pointer by value. I //consider this important because we are about to abuse it. void Spawn_thread (boost::shared_ptr <whatever> value) { pthread_t thread; pthread_attr_t attr; //Notice that I'm passing a raw pointer to the shared pointer. if (!pthread_create (&thread, &attr, &Thread_main, &value)) { //Wait for the new thread to copy its parameter into a //local variable Wait_for_signal(); } else { //Error handling } } void *Thread_main (void *arg) { //Dereference the argument (to get back the shared pointer), copy //it into a local shared pointer and allow the main thread to proceed boost::shared_ptr <whatever> parameter = *reinterpret_cast <boost::shared_ptr <whatever> *> (arg); //Remove any temptation to use a value that is about to become undefined. arg = NULL; //Now that we have copied the shared pointer, we can allow Wait_for_signal() to return. Signal_main_thread(); //Your code goes here. Under NO circumstances are you to call //pthread_exit(). You must allow Thread_main() to return //normally if you want to avoid memory leaks. } Wait_for_signal and Signal_main_thread may need some explanation. I'm not sure of the best way to implement them with pthreads. In Windows, I would just use an event, but I remember reading that pthreads doesn't have one. You could use a semaphore or a simple spin lock to implement them. I hate to say it, but you'll have to get creative if there is even the slightest chance of multiple threads calling Spawn_thread at the same time.
On Tue, Dec 18, 2007 at 10:28:37AM -0500, Andrew Holden wrote:
boost-users-bounces@lists.boost.org <> wrote:
I haven't been able to find any clear explanation about this, so here it goes:
I'm coding a multithreaded program using pthreads. The general behaviour is this:
-A function starts. -The function creates a shared pointer "OBJ". -The function creates a Thread, passing "OBJ" as the parameter (having to cast it as void*). -The thread starts, getting a (void*) param which is casted back to "OBJ" (shared pointer type). -The function ends, and therefore its "OBJ" goes out of scope. -The thread" ends, so its "OBJ" goes out of scope too. (of course, the thread could at times end before the function instead)
I'm afraid that the shared pointer does not behave well due to those nasty void* casts... maybe trying to delete twice the object. What's the proper way of coding this? Weak pointers maybe?
Thanks a lot for any help.
The first problem I see is that a boost::shared_ptr is not the same size as a void*. The believe the cast would slice off the pointer to the reference count. I would try something like the following (untested):
He might just allocate the shared_ptr on the heap (the dynamic memory allocation will be anyway cheaper than thread creation): void *thread(void *ptr) { shared_ptr<X> *_p = static_cast<shared_ptr<X>*>(ptr); shared_ptr<X> p = *_p; delete _p; ... } void creator() { ... pthread_create(&thrid, 0, thread, new shared_ptr<X>(orig_ptr)); ... } No need for any additional synchronization.
Zeljko Vrba [zvrba@ifi.uio.no] wrote:
He might just allocate the shared_ptr on the heap (the dynamic memory allocation will be anyway cheaper than thread creation):
void *thread(void *ptr) { shared_ptr<X> *_p = static_cast<shared_ptr<X>*>(ptr); shared_ptr<X> p = *_p; delete _p; ... }
void creator() { ... pthread_create(&thrid, 0, thread, new shared_ptr<X>(orig_ptr)); ... }
No need for any additional synchronization.
Good point. And certainly much simpler than my solution.
Shared pointer question: func(...) { shared_pointer<foo> p; p = list<shared_pointer<foo> >.pop_front(); p = list<shared_pointer<foo> >.pop_front(); } Will this cause a memory leak? Do I need to do a delete in between, or does the act of reassignment cause the prior object to be dereferenced? Thanks, Elli Barasch
Elli Barasch wrote:
Shared pointer question: [snip odd example]
Will this cause a memory leak? Do I need to do a delete in between, or does the act of reassignment cause the prior object to be dereferenced?
A second assignment to a given shared_ptr should not cause a leak. You should avoid an explicit delete. If p is a shared_ptr pointing to a heap object, then assigning a new value to p will decrement the previously-referenced object's refcount, which may cause the heap object to be deleted. This document: http://boost.org/libs/smart_ptr/shared_ptr.htm#Members says this under 'assignment': shared_ptr & operator=(shared_ptr const & r); // never throws template<class Y> shared_ptr & operator=(shared_ptr<Y> const & r); // never throws template<class Y> shared_ptr & operator=(std::auto_ptr<Y> & r); Effects: Equivalent to shared_ptr(r).swap(*this). Returns: *this. Notes: The use count updates caused by the temporary object construction and destruction are not considered observable side effects, and the implementation is free to meet the effects (and the implied guarantees) via different means, without creating a temporary. But the important point is that the assignment must behave as though a shared_ptr holding p's prior value has been deleted.
That's the answer I was looking for. Sorry for my mangled example, but you had the idea... Nat Goodspeed wrote:
Elli Barasch wrote:
Shared pointer question:
[snip odd example]
Will this cause a memory leak? Do I need to do a delete in between, or does the act of reassignment cause the prior object to be dereferenced?
A second assignment to a given shared_ptr should not cause a leak. You should avoid an explicit delete.
If p is a shared_ptr pointing to a heap object, then assigning a new value to p will decrement the previously-referenced object's refcount, which may cause the heap object to be deleted.
This document: http://boost.org/libs/smart_ptr/shared_ptr.htm#Members says this under 'assignment':
shared_ptr & operator=(shared_ptr const & r); // never throws template<class Y> shared_ptr & operator=(shared_ptr<Y> const & r); // never throws template<class Y> shared_ptr & operator=(std::auto_ptr<Y> & r);
Effects: Equivalent to shared_ptr(r).swap(*this).
Returns: *this.
Notes: The use count updates caused by the temporary object construction and destruction are not considered observable side effects, and the implementation is free to meet the effects (and the implied guarantees) via different means, without creating a temporary.
But the important point is that the assignment must behave as though a shared_ptr holding p's prior value has been deleted. _______________________________________________ Boost-users mailing list Boost-users@lists.boost.org http://lists.boost.org/mailman/listinfo.cgi/boost-users
Thank you two for the help. On 12/18/07, Zeljko Vrba <zvrba@ifi.uio.no> wrote:
On Tue, Dec 18, 2007 at 10:28:37AM -0500, Andrew Holden wrote: He might just allocate the shared_ptr on the heap (the dynamic memory allocation will be anyway cheaper than thread creation):
void *thread(void *ptr) { shared_ptr<X> *_p = static_cast<shared_ptr<X>*>(ptr); shared_ptr<X> p = *_p; delete _p; ... }
void creator() { ... pthread_create(&thrid, 0, thread, new shared_ptr<X>(orig_ptr)); ... }
Ok. This is how my code structure looks: ---------------------------- typedef shared_ptr<MyClass> pMyClass; void* MyClass::actionThread(void *objectPointer) { pMyClass * _obj = static_cast<pMyClass*>(objectPointer); pMyClass obj = *_obj; obj->action(false); delete _obj; } void MyClass::action(bool threaded) { if (threaded) { pthread_t tid; pthread_create(&tid, 0, actionThread, new pMyClass(this)) } else { //TODO: actually perform the action } } ---------------------------- When calling an instance->action(false) it works correctly. When calling instance->action(true) it sometimes segfaults. So i'm not sure i interpreted your code sample correctly. Do i need to use enable_shared_from_this? Or do i have to avoid using "this" (orig_ptr in your code) somehow maybe? Thanks in advance! -- Saludos, Bruno González _______________________________________________ Msn/Jabber: stenyak AT gmail.com ICQ: 153709484 http://www.stenyak.com
On Wed, Dec 19, 2007 at 01:21:10PM +0100, STenyaK (Bruno Gonzalez) wrote:
Ok. This is how my code structure looks: ---------------------------- typedef shared_ptr<MyClass> pMyClass; void* MyClass::actionThread(void *objectPointer) { pMyClass * _obj = static_cast<pMyClass*>(objectPointer); pMyClass obj = *_obj; obj->action(false); delete _obj; }
It is _crucial_ to call delete BEFORE any other action in the code, or else you may suffer memory leak (consider e.g. that action throws an exception). Otherwise, your interpretation seems correct.
Do i need to use enable_shared_from_this? Or do i have to avoid using "this" (orig_ptr in your code) somehow maybe?
Uf, I don't know. Read this: http://www.boost.org/libs/smart_ptr/sp_techniques.html it mentiones several instances of the "shared_ptr from this" problem.
STenyaK (Bruno Gonzalez) [stenyak@gmail.com] wrote:
Ok. This is how my code structure looks: ---------------------------- typedef shared_ptr<MyClass> pMyClass; void* MyClass::actionThread(void *objectPointer) { pMyClass * _obj = static_cast<pMyClass*>(objectPointer); pMyClass obj = *_obj; obj->action(false); delete _obj; } void MyClass::action(bool threaded) { if (threaded) { pthread_t tid; pthread_create(&tid, 0, actionThread, new pMyClass(this)) } else { //TODO: actually perform the action } } ---------------------------- When calling an instance->action(false) it works correctly. When calling instance->action(true) it sometimes segfaults. So i'm not sure i interpreted your code sample correctly.
Do i need to use enable_shared_from_this? Or do i have to avoid using "this" (orig_ptr in your code) somehow maybe?
Thanks in advance!
I would stongly recommend shared_from_this. You code, as written, will guaranteee that actionThread will destroy the MyClass as soon asactionThread finishes, even if the main thread still expects it to exist. This would result in a double-free if the main thread has its own shared pointer to the object. Actually, the fact that this is a member function opens another possibility. First, derive MyClass from enable_shared_from_this <MyClass>, then you can implement the functions like this: ---------------------------- typedef shared_ptr<MyClass> pMyClass; void* MyClass::actionThread(void *objectPointer) { MyClass * _obj = static_cast<MyClass*>(objectPointer); pMyClass obj = _obj->shared_from_this(); obj->action(false); } void MyClass::action(bool threaded) { if (threaded) { pthread_t tid; pthread_create(&tid, 0, actionThread, this) } else { //TODO: actually perform the action } } ---------------------------- This works because shared_from_this is specifically designed to allow member functions of a class to recover the shared pointer to the object.
On 12/19/07, Andrew Holden <aholden@charteroaksystems.com> wrote:
STenyaK (Bruno Gonzalez) [stenyak@gmail.com] wrote: typedef shared_ptr<MyClass> pMyClass; void* MyClass::actionThread(void *objectPointer) { MyClass * _obj = static_cast<MyClass*>(objectPointer); pMyClass obj = _obj->shared_from_this(); obj->action(false); }
void MyClass::action(bool threaded) { if (threaded) { pthread_t tid; pthread_create(&tid, 0, actionThread, this) } else { //TODO: actually perform the action } } ----------------------------
This works because shared_from_this is specifically designed to allow member functions of a class to recover the shared pointer to the object.
Just in case... actionThread is a static method of the class, since otherwise C++ won't allow me to get a pointer of it (and i dislike global functions). Does this fact have any implications in the last proposed code sample? Thanks again to everybody. -- Saludos, Bruno González _______________________________________________ Msn/Jabber: stenyak AT gmail.com ICQ: 153709484 http://www.stenyak.com
STenyaK (Bruno Gonzalez) [stenyak@gmail.com] wrote:
On 12/19/07, Andrew Holden <aholden@charteroaksystems.com> wrote:
typedef shared_ptr<MyClass> pMyClass; void* MyClass::actionThread(void *objectPointer) { MyClass * _obj = static_cast<MyClass*>(objectPointer); pMyClass obj = _obj->shared_from_this(); obj->action(false); }
void MyClass::action(bool threaded) { if (threaded) { pthread_t tid; pthread_create(&tid, 0, actionThread, this) } else { //TODO: actually perform the action } } ----------------------------
This works because shared_from_this is specifically designed to allow member functions of a class to recover the shared pointer to the object.
Just in case... actionThread is a static method of the class, since otherwise C++ won't allow me to get a pointer of it (and i dislike global functions). Does this fact have any implications in the last proposed code sample?
Not really. actionThread is still a member function of MyClass, which means it has full access to MyClass's members (as long as it has access to a MyClass object somehow), including private and protected members, and protected members from base classes. _obj is a valid pointer to a MyClass, so there is nothing wrong with actionThread calling _obj's shared_from_this function.
On 12/19/07, Andrew Holden <aholden@charteroaksystems.com> wrote:
STenyaK (Bruno Gonzalez) [stenyak@gmail.com] wrote:
On 12/19/07, Andrew Holden <aholden@charteroaksystems.com> wrote:
typedef shared_ptr<MyClass> pMyClass; void* MyClass::actionThread(void *objectPointer) { MyClass * _obj = static_cast<MyClass*>(objectPointer); pMyClass obj = _obj->shared_from_this(); obj->action(false); }
void MyClass::action(bool threaded) { if (threaded) { pthread_t tid; pthread_create(&tid, 0, actionThread, this) sleep(1); } else { //TODO: actually perform the action } } ----------------------------
I'm still getting crashes. By adding that sleep sentence i seem to prevent them (or most of them). So i guess i have to add a wait right there, in order to make sure that _obj still exists when trying to get its shared_from_this? -- Saludos, Bruno González _______________________________________________ Msn/Jabber: stenyak AT gmail.com ICQ: 153709484 http://www.stenyak.com
Andrew Holden [aholden@charteroaksystems.com] wrote:
STenyaK (Bruno Gonzalez) [stenyak@gmail.com] wrote:
Ok. This is how my code structure looks: ---------------------------- typedef shared_ptr<MyClass> pMyClass; void* MyClass::actionThread(void *objectPointer) { pMyClass * _obj = static_cast<pMyClass*>(objectPointer); pMyClass obj = *_obj; obj->action(false); delete _obj; } void MyClass::action(bool threaded) { if (threaded) { pthread_t tid; pthread_create(&tid, 0, actionThread, new pMyClass(this)) } else { //TODO: actually perform the action } } ----------------------------
---------------------------- typedef shared_ptr<MyClass> pMyClass; void* MyClass::actionThread(void *objectPointer) { MyClass * _obj = static_cast<MyClass*>(objectPointer); pMyClass obj = _obj->shared_from_this(); obj->action(false); }
void MyClass::action(bool threaded) { if (threaded) { pthread_t tid; pthread_create(&tid, 0, actionThread, this) } else { //TODO: actually perform the action } } ----------------------------
I just realized you should ignore this implementation because it contains a race condition (main thread may delete the object before actionThread can call shared_from_this. Here is a tweak for your version that uses shared_from_this: ---------------------------- typedef shared_ptr<MyClass> pMyClass; void* MyClass::actionThread(void *objectPointer) { pMyClass * _obj = static_cast<pMyClass*>(objectPointer); pMyClass obj = *_obj; delete _obj; obj->action(false); } void MyClass::action(bool threaded) { if (threaded) { pthread_t tid; //Change next line: pthread_create(&tid, 0, actionThread, new pMyClass(shared_from_this())) } else { //TODO: actually perform the action } } ----------------------------
participants (5)
-
Andrew Holden -
Elli Barasch -
Nat Goodspeed -
STenyaK (Bruno Gonzalez) -
Zeljko Vrba