From: heister Date: Fri, 8 Nov 2013 14:21:32 +0000 (+0000) Subject: fix abandoning threads. Instead of keeping ThreadDescriptor alive using a shared_ptr... X-Git-Url: https://gitweb.dealii.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b3cc6d032c43780b1899fa4223769ee1bfa372b2;p=dealii-svn.git fix abandoning threads. Instead of keeping ThreadDescriptor alive using a shared_ptr to self, we just detach() from the c++ thread when destructed. git-svn-id: https://svn.dealii.org/trunk@31590 0785d39b-7218-0410-832d-ea1e28bc413d --- diff --git a/deal.II/include/deal.II/base/thread_management.h b/deal.II/include/deal.II/base/thread_management.h index a0d4418cfe..854aae1494 100644 --- a/deal.II/include/deal.II/base/thread_management.h +++ b/deal.II/include/deal.II/base/thread_management.h @@ -1279,50 +1279,14 @@ namespace Threads * While we have only one of these * objects per thread, several * Threads::Thread objects may refer to - * this descriptor. - * - * Note, however, that since this - * object holds the location - * where we store the return - * value of the thread, the - * object has to live at least as - * long as the thread - * executes. This means that even - * if all Threads::Thread objects - * that referred to this - * descriptor (through a - * std::shared_ptr) have gone out - * of scope, we must still hold - * on to the object. We do this - * by having the descriptor keep - * a pointer to itself and reset - * it to zero once it is done -- - * effectively keeping the use - * pointer above zero as long as - * work is going on. - * - * To enable the current class to - * obtain a shared_ptr from - * itself, we derive from the - * class - * std::enable_shared_from_this. + * this descriptor. If all Thread + * objects go out of scope the + * ThreadDescriptor will detach from + * the thread before being destructed. */ template - struct ThreadDescriptor : public std_cxx1x::enable_shared_from_this > + struct ThreadDescriptor { - /** - * A pointer to the current - * object, kept nonzero as - * long as the thread is - * executing to avoid - * destroying the current - * object while we are still - * expecting to write - * something into the return - * value location. - */ - std_cxx1x::shared_ptr > self; - /** * An object that represents the * thread started. @@ -1333,8 +1297,14 @@ namespace Threads * An object that will hold the value * returned by the function called on * the thread. + * + * The return value is stored + * in a shared_ptr because we might + * abandon the the ThreadDescriptor. + * This makes sure the object stays + * alive until the thread exits. */ - return_value ret_val; + std_cxx1x::shared_ptr > ret_val; /** * A bool variable that is initially @@ -1405,41 +1375,24 @@ namespace Threads thread_is_active (false) {} + ~ThreadDescriptor () + { + if (!thread_is_active) + return; + thread.detach(); + thread_is_active = false; + } /** * Start the thread and let * it put its return value * into the ret_val object. - * - * Note that we cannot - * already do this in the - * constructor of this class: - * we will call - * enable_shared_from_this::shared_from_this - * which requires that there - * is already a shared_ptr - * that owns this object; - * however, at the time of - * creation, there can of - * course be no shared_ptr - * that owns the object -- it - * is, after all, just being - * created and can not - * already have been handed - * off to a shared_ptr; as a - * consequence, we first - * create the object, give it - * to a shared_ptr, and then - * call shared_from_this. */ void start (const std_cxx1x::function &function) { - // set the self pointer - // and then start the - // thread - self = this->shared_from_this (); thread_is_active = true; - thread = std_cxx1x::thread (thread_entry_point, function, this); + ret_val.reset(new return_value()); + thread = std_cxx1x::thread (thread_entry_point, function, ret_val); } @@ -1472,7 +1425,7 @@ namespace Threads */ static void thread_entry_point (const std_cxx1x::function function, - ThreadDescriptor *descriptor) + std_cxx1x::shared_ptr > ret_val) { // call the function // in question. since an @@ -1489,7 +1442,7 @@ namespace Threads internal::register_thread (); try { - call (function, descriptor->ret_val); + call (function, *ret_val); } catch (const std::exception &exc) { @@ -1500,24 +1453,6 @@ namespace Threads internal::handle_unknown_exception (); } internal::deregister_thread (); - - // once we are done, - // we can release our - // hold on the thread - // descriptor - // object. if all - // Threads::Thread - // objects that - // pointed here - // should have gone - // out of scope - // already, then this - // will trigger - // destruction of the - // descriptor object - Assert (descriptor->self.use_count () > 0, - ExcInternalError()); - descriptor->self.reset (); } }; @@ -1542,7 +1477,7 @@ namespace Threads * returned by the function called on * the thread. */ - return_value ret_val; + std_cxx1x::shared_ptr > ret_val; /** * Start the thread and @@ -1551,6 +1486,7 @@ namespace Threads */ void start (const std_cxx1x::function &function) { + ret_val.reset(new return_value()); call (function, ret_val); } @@ -1615,15 +1551,7 @@ namespace Threads thread_descriptor (new internal::ThreadDescriptor()) { // in a second step, start - // the thread. this needs - // to be done in two steps - // because inside the start - // function we call - // enable_shared_from_this::shared_from_this - // which requires that - // there is already a - // shared_ptr that owns the - // object + // the thread. thread_descriptor->start (function); } @@ -1671,7 +1599,7 @@ namespace Threads RT return_value () { join (); - return thread_descriptor->ret_val.get(); + return thread_descriptor->ret_val->get(); } /** @@ -1712,29 +1640,6 @@ namespace Threads * sure that that object lives * as long as there is at least * one subscriber to it. - * - * Note that since the - * descriptor holds the - * location where the return - * value is stored, even if all - * Threads::Thread objects that - * point to the descriptor go - * out of scope, we must make - * sure that the descriptor is - * not yet destroyed. Rather, - * the descriptor must be - * destroyed when the last - * Threads::Thread object that - * points to it goes out of - * scope or whenever the thread - * is finished, whatever - * happens later. We do this by - * having the descriptor keep a - * pointer to itself and reset - * it to zero once it is done - * -- effectively keeping the use - * pointer above zero as long - * as work is going on. */ std_cxx1x::shared_ptr > thread_descriptor; };