c++ - Is it a good idea to shut down a class's thread member in the class's destructor? -
what's best way shut down boost thread managed c++ class when it's time object of class destroyed? have class creates , starts thread on construction , provides public wake() method wakes thread when it's time work. wake() method uses boost mutex , boost condition variable signal thread; thread procedure waits on condition variable, work , goes waiting.
at moment, shut thread down in class's destructor, using boolean member variable "running" flag; clear flag , call notify_one() on condition variable. thread procedure wakes up, notices "running" false, , returns. here's code:
class worker { public: worker(); ~worker(); void wake(); private: worker(worker const& rhs); // prevent copying worker& operator=(worker const& rhs); // prevent assignment void threadproc(); bool m_running; boost::mutex m_mutex; boost::condition_variable m_condition; boost::scoped_ptr<boost::thread> m_pthread; }; worker::worker() : m_running(true) , m_mutex() , m_condition() , m_pthread() { m_pthread.reset(new boost::thread(boost::bind(&worker::threadproc, this))); } worker::~worker() { m_running = false; m_condition.notify_one(); m_pthread->join(); } void worker::wake() { boost::lock_guard<boost::mutex> lock(m_mutex); m_condition.notify_one(); } void worker::threadproc() { (;;) { boost::unique_lock<boost::mutex> lock(m_mutex); m_condition.wait(lock); if (! m_running) break; // work here } } is idea shut down thread in class's destructor this, or should provide public method lets user before object destroyed, when there's more potential error handling and/or forcibly destroying thread if thread procedure fails return cleanly or in time?
cleaning object's mess in destructor appealing require less attention detail user (abstraction, hurrah!) seems me should things in destructor if can guarantee take full responsibility cleaning things , thoroughly, , there's small chance code outside class might 1 day need know whether or not thread shut down cleanly.
also, mechanism i'm using - writing member variable in object on stack of 1 thread , reading variable in thread - safe , sane?
it idea release resources class creates when class destroyed, if 1 of resources thread. if resource created explicitly via user call, such worker::start(), there should explicit way release it, such worker::stop(). idea either perform cleanup in destructor in event user not call worker::stop() and/or provide user scoped helper class implements raii-idiom, invoking worker::start() in constructor , worker::stop() in destructor. however, if resource allocation done implicitly, such in worker constructor, release of resource should implicit, leaving destructor prime candidate responsibility.
destruction
lets examine worker::~worker(). general rule not throw exceptions in destructors. if worker object on stack unwinding exception, , worker::~worker() throws exception, std::terminate() invoked, killing application. while worker::~worker() not explicitly throwing exception, important consider of functions invoking may throw:
m_condition.notify_one()not throw.m_pthread->join()throwboost::thread_interrupted.
if std::terminate() desired behavior, no change required. however, if std::terminate() not desired, catch boost::thread_interrupted , suppress it.
worker::~worker() { m_running = false; m_condition.notify_one(); try { m_pthread->join(); } catch ( const boost::thread_interrupted& ) { /* suppressed */ } } concurrency
managing threading can tricky. important define exact desired behavior of functions worker::wake(), understand behavior of types facilitate threading , synchronization. example, boost::condition_variable::notify_one() has no effect if no threads blocked in boost::condition_variable::wait(). lets examine possible concurrent paths worker::wake().
below crude attempt diagram concurrency 2 scenarios:
- order-of-operation occurs top-to-bottom. (i.e. operations @ top occur before operations @ bottom.
- concurrent operations written on same line.
<,>used highlight when 1 thread waking or unblocking thread. examplea > bindicates threadaunblocking threadb.
scenario: worker::wake() invoked while worker::threadproc() blocked on m_condition.
other thread | worker::threadproc -----------------------------------+------------------------------------------ | lock( m_mutex ) | `-- m_mutex.lock() | m_condition::wait( lock ) | |-- m_mutex.unlock() | |-- waits on notification worker::wake() | | |-- lock( m_mutex ) | | | `-- m_mutex.lock() | | |-- m_condition::notify_one() > |-- wakes notification `-- ~lock() | `-- m_mutex.lock() // blocks `-- m_mutex.unlock() > `-- // acquires lock | // work here | ~lock() // end of loop's scope | `-- m_mutex.unlock()
result: worker::wake() returns quickly, , worker::threadproc runs.
scenario: worker::wake() invoked while worker::threadproc() not blocked on m_condition.
other thread | worker::threadproc -----------------------------------+------------------------------------------ | lock( m_mutex ) | `-- m_mutex.lock() | m_condition::wait( lock ) | |-- m_mutex.unlock() worker::wake() > |-- wakes | `-- m_mutex.lock() worker::wake() | // work here |-- lock( m_mutex ) | // still doing work... | |-- m_mutex.lock() // block | // hope not block on system call | | | // , more work... | | | ~lock() // end of loop's scope | |-- // still blocked < `-- m_mutex.unlock() | `-- // acquires lock | lock( m_mutex ) // next 'for' iteration. |-- m_condition::notify_one() | `-- m_mutex.lock() // blocked `-- ~lock() | |-- // still blocked `-- m_mutex.unlock() > `-- // acquires lock | m_condition::wait( lock ) | |-- m_mutex.unlock() | `-- waits on notification | `-- still waiting...
result: worker::wake() blocked worker::threadproc did work, no-op, sent notification m_condition when no 1 waiting on it.
this not particularly dangerous worker::wake(), can cause problems in worker::~worker(). if worker::~worker() runs while worker::threadproc doing work, worker::~worker() may block indefinitely when joining thread, thread may not waiting on m_condition @ point in notified, , worker::threadproc checks m_running after done waiting on m_condition.
working towards solution
in example, lets define following requirements:
worker::~worker()not causestd::terminate()invoked.worker::wake()not block whileworker::threadprocdoing work.- if
worker::wake()called whileworker::threadprocnot doing work, notifyworker::threadprocwork. - if
worker::wake()called whileworker::threadprocdoing work, notifyworker::threadprocperform iteration of work. - multiple calls
worker::wake()whileworker::threadprocdoing work result inworker::threadprocperforming single additional iteration of work.
code:
#include <boost/thread.hpp> class worker { public: worker(); ~worker(); void wake(); private: worker(worker const& rhs); // prevent copying worker& operator=(worker const& rhs); // prevent assignment void threadproc(); enum state { has_work, no_work, shutdown }; state m_state; boost::mutex m_mutex; boost::condition_variable m_condition; boost::thread m_thread; }; worker::worker() : m_state(no_work) , m_mutex() , m_condition() , m_thread() { m_thread = boost::thread(&worker::threadproc, this); } worker::~worker() { // create scope mutex locked when changing state , // notifying condition. result in deadlock if lock // still held function when trying join thread. { boost::lock_guard<boost::mutex> lock(m_mutex); m_state = shutdown; m_condition.notify_one(); } try { m_thread.join(); } catch ( const boost::thread_interrupted& ) { /* suppress */ }; } void worker::wake() { boost::lock_guard<boost::mutex> lock(m_mutex); m_state = has_work; m_condition.notify_one(); } void worker::threadproc() { (;;) { // create scope lock mutex when checking state. // not continue hold mutex wile doing busy work. { boost::unique_lock<boost::mutex> lock(m_mutex); // while there no work (implies not shutting down), wait on // condition. while (no_work == m_state) { m_condition.wait(lock); // wake either wake() or ~worker() signaling condition // variable. @ point, m_state either has_work or // shutdown. } // on shutdown, break out of loop. if (shutdown == m_state) break; // set state indicate no work queued. m_state = no_work; } // work here } } note: personal preference, opted not allocated boost::thread on heap, , result, not need manage via boost::scoped_ptr. boost::thread has default constructor refer not-a-thread, , move-assignable.
Comments
Post a Comment