Skip to content

Commit

Permalink
Make detail::thread children wait on the thread before destruct. Fixes
Browse files Browse the repository at this point in the history
…#3

Signed-off-by: Amr Ali <amr@databracket.com>
  • Loading branch information
amrali committed Oct 31, 2012
1 parent 6ec6538 commit ea0fdc3
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 15 deletions.
28 changes: 18 additions & 10 deletions cloudless/detail/thread.cpp
Expand Up @@ -38,9 +38,7 @@ namespace detail
// thread

thread::thread() :
_M_barrier(2),
_M_stop(false),
_M_detached(false)
_M_stop(false)
{}

thread::~thread()
Expand All @@ -49,7 +47,9 @@ namespace detail
// otherwise nasty things shall ensue.
stop();

_M_wait();
// Wait for threads to terminate before allowing this instance
// to be destroyed.
wait();
}

bool
Expand Down Expand Up @@ -118,13 +118,16 @@ namespace detail
}

void
thread::_M_wait()
thread::wait()
{
// Here we make sure that the thread will not deadlock on itself
// by waiting on a barrier that will never be reached by the parent
// thread, in the case that it was joined.
// Here we make sure that the object holding information
// about the thread will not destruct unless the thread
// fully terminates.
if (_M_detached)
_M_barrier.wait();
{
_M_waiter.lock();
_M_waiter.unlock();
}
}

void
Expand All @@ -133,6 +136,10 @@ namespace detail
// This mutex is only used as a memory fence
boost::mutex::scoped_lock lck(_M_mutex);

// Lock _M_waiter so that objects cannot destruct
// until this thread finishes.
_M_waiter.lock();

try {
prologue(); // Executed once before body()
while (!_M_stop) { body(); }
Expand All @@ -142,7 +149,8 @@ namespace detail
catch (...) { /* We're sorry for your loss. */ }
} catch (...) { /* Not much to be done ... */ }

_M_wait();
// Now it is safe to allow objects to destruct fully.
_M_waiter.unlock();
}

} // namespace detail
Expand Down
13 changes: 9 additions & 4 deletions cloudless/detail/thread.hpp
Expand Up @@ -29,11 +29,11 @@
#define CLOUDLESS_DETAIL_THREAD_HPP

#include <boost/thread/thread.hpp>
#include <boost/thread/barrier.hpp>
#include <boost/thread/mutex.hpp>

#include <cloudless/detail/export.hpp>
#include <cloudless/detail/exception.hpp>
#include <cloudless/detail/ipod.hpp>
#include <cloudless/detail/shared_ptr.hpp>
#include <cloudless/detail/shared_array.hpp>
#include <cloudless/detail/noncopyable.hpp>
Expand Down Expand Up @@ -129,6 +129,12 @@ namespace detail

protected:

/**
* A function used to wait for the thread to terminate before
* any objects fully destruct.
*/
void wait();

/**
* A virtual function to be optionally overrided by a child of this class.
* This function will be executed only once before entering the body() function.
Expand Down Expand Up @@ -180,12 +186,11 @@ namespace detail
private:
shared_ptr<boost::thread> _Mp_thread;
boost::mutex _M_mutex;
boost::barrier _M_barrier;
boost::mutex _M_waiter;
ipod<bool> _M_detached;
volatile bool _M_stop;
volatile bool _M_detached;

private:
void _M_wait();
void _M_run();

};
Expand Down
5 changes: 5 additions & 0 deletions cloudless/device.cpp
Expand Up @@ -38,6 +38,11 @@ namespace cloudless
detail::thread(), _M_edges(edges)
{}

device::~device()
{
wait();
}

edges&
device::get_edges() const
{
Expand Down
2 changes: 1 addition & 1 deletion cloudless/device.hpp
Expand Up @@ -44,7 +44,7 @@ namespace cloudless
{
public:
device(const edges& edges);
virtual ~device() {}
virtual ~device();

protected:

Expand Down

0 comments on commit ea0fdc3

Please sign in to comment.