From: chris_at_[hidden]
Date: 2007-09-01 02:42:09
Author: chris_kohlhoff
Date: 2007-09-01 02:41:15 EDT (Sat, 01 Sep 2007)
New Revision: 39098
URL: http://svn.boost.org/trac/boost/changeset/39098
Log:
Fix problem where a thread can go idle even if there are handlers that are
ready to be dispatched.
Remove need to have a mutex per idle thread.
Remove need to have a mutex per idle thread.
Text files modified:
trunk/boost/asio/detail/null_event.hpp | 9 ++-
trunk/boost/asio/detail/posix_event.hpp | 37 +++++---------
trunk/boost/asio/detail/posix_mutex.hpp | 3 +
trunk/boost/asio/detail/scoped_lock.hpp | 12 ++++
trunk/boost/asio/detail/task_io_service.hpp | 99 ++++++++++++++++++---------------------
trunk/boost/asio/detail/win_event.hpp | 17 +++++-
6 files changed, 96 insertions(+), 81 deletions(-)
Modified: trunk/boost/asio/detail/null_event.hpp
==============================================================================
--- trunk/boost/asio/detail/null_event.hpp (original)
+++ trunk/boost/asio/detail/null_event.hpp 2007-09-01 02:41:15 EDT (Sat, 01 Sep 2007)
@@ -44,17 +44,20 @@
}
// Signal the event.
- void signal()
+ template <typename Lock>
+ void signal(Lock&)
{
}
// Reset the event.
- void clear()
+ template <typename Lock>
+ void clear(Lock&)
{
}
// Wait for the event to become signalled.
- void wait()
+ template <typename Lock>
+ void wait(Lock&)
{
}
};
Modified: trunk/boost/asio/detail/posix_event.hpp
==============================================================================
--- trunk/boost/asio/detail/posix_event.hpp (original)
+++ trunk/boost/asio/detail/posix_event.hpp 2007-09-01 02:41:15 EDT (Sat, 01 Sep 2007)
@@ -25,6 +25,7 @@
#if defined(BOOST_HAS_PTHREADS)
#include <boost/asio/detail/push_options.hpp>
+#include <boost/assert.hpp>
#include <boost/throw_exception.hpp>
#include <pthread.h>
#include <boost/asio/detail/pop_options.hpp>
@@ -43,7 +44,7 @@
posix_event()
: signalled_(false)
{
- int error = ::pthread_mutex_init(&mutex_, 0);
+ int error = ::pthread_cond_init(&cond_, 0);
if (error != 0)
{
boost::system::system_error e(
@@ -51,53 +52,43 @@
"event");
boost::throw_exception(e);
}
-
- error = ::pthread_cond_init(&cond_, 0);
- if (error != 0)
- {
- ::pthread_mutex_destroy(&mutex_);
- boost::system::system_error e(
- boost::system::error_code(error, boost::system::native_ecat),
- "event");
- boost::throw_exception(e);
- }
}
// Destructor.
~posix_event()
{
::pthread_cond_destroy(&cond_);
- ::pthread_mutex_destroy(&mutex_);
}
// Signal the event.
- void signal()
+ template <typename Lock>
+ void signal(Lock& lock)
{
- ::pthread_mutex_lock(&mutex_); // Ignore EINVAL and EDEADLK.
+ BOOST_ASSERT(lock.locked());
+ (void)lock;
signalled_ = true;
::pthread_cond_signal(&cond_); // Ignore EINVAL.
- ::pthread_mutex_unlock(&mutex_); // Ignore EINVAL and EPERM.
}
// Reset the event.
- void clear()
+ template <typename Lock>
+ void clear(Lock& lock)
{
- ::pthread_mutex_lock(&mutex_); // Ignore EINVAL and EDEADLK.
+ BOOST_ASSERT(lock.locked());
+ (void)lock;
signalled_ = false;
- ::pthread_mutex_unlock(&mutex_); // Ignore EINVAL and EPERM.
}
// Wait for the event to become signalled.
- void wait()
+ template <typename Lock>
+ void wait(Lock& lock)
{
- ::pthread_mutex_lock(&mutex_); // Ignore EINVAL and EDEADLK.
+ BOOST_ASSERT(lock.locked());
while (!signalled_)
- ::pthread_cond_wait(&cond_, &mutex_); // Ignore EINVAL.
- ::pthread_mutex_unlock(&mutex_); // Ignore EINVAL and EPERM.
+ ::pthread_cond_wait(&cond_, &lock.mutex().mutex_); // Ignore EINVAL.
}
private:
- ::pthread_mutex_t mutex_;
::pthread_cond_t cond_;
bool signalled_;
};
Modified: trunk/boost/asio/detail/posix_mutex.hpp
==============================================================================
--- trunk/boost/asio/detail/posix_mutex.hpp (original)
+++ trunk/boost/asio/detail/posix_mutex.hpp 2007-09-01 02:41:15 EDT (Sat, 01 Sep 2007)
@@ -36,6 +36,8 @@
namespace asio {
namespace detail {
+class posix_event;
+
class posix_mutex
: private noncopyable
{
@@ -88,6 +90,7 @@
}
private:
+ friend class posix_event;
::pthread_mutex_t mutex_;
};
Modified: trunk/boost/asio/detail/scoped_lock.hpp
==============================================================================
--- trunk/boost/asio/detail/scoped_lock.hpp (original)
+++ trunk/boost/asio/detail/scoped_lock.hpp 2007-09-01 02:41:15 EDT (Sat, 01 Sep 2007)
@@ -64,6 +64,18 @@
}
}
+ // Test whether the lock is held.
+ bool locked() const
+ {
+ return locked_;
+ }
+
+ // Get the underlying mutex.
+ Mutex& mutex()
+ {
+ return mutex_;
+ }
+
private:
// The underlying mutex.
Mutex& mutex_;
Modified: trunk/boost/asio/detail/task_io_service.hpp
==============================================================================
--- trunk/boost/asio/detail/task_io_service.hpp (original)
+++ trunk/boost/asio/detail/task_io_service.hpp 2007-09-01 02:41:15 EDT (Sat, 01 Sep 2007)
@@ -40,6 +40,7 @@
: boost::asio::detail::service_base<task_io_service<Task> >(io_service),
mutex_(),
task_(use_service<Task>(io_service)),
+ task_interrupted_(true),
outstanding_work_(0),
handler_queue_(&task_handler_),
handler_queue_end_(&task_handler_),
@@ -80,8 +81,7 @@
typename call_stack<task_io_service>::context ctx(this);
idle_thread_info this_idle_thread;
- this_idle_thread.prev = &this_idle_thread;
- this_idle_thread.next = &this_idle_thread;
+ this_idle_thread.next = 0;
boost::asio::detail::mutex::scoped_lock lock(mutex_);
@@ -98,8 +98,7 @@
typename call_stack<task_io_service>::context ctx(this);
idle_thread_info this_idle_thread;
- this_idle_thread.prev = &this_idle_thread;
- this_idle_thread.next = &this_idle_thread;
+ this_idle_thread.next = 0;
boost::asio::detail::mutex::scoped_lock lock(mutex_);
@@ -134,7 +133,7 @@
void stop()
{
boost::asio::detail::mutex::scoped_lock lock(mutex_);
- stop_all_threads();
+ stop_all_threads(lock);
}
// Reset in preparation for a subsequent run invocation.
@@ -156,7 +155,7 @@
{
boost::asio::detail::mutex::scoped_lock lock(mutex_);
if (--outstanding_work_ == 0)
- stop_all_threads();
+ stop_all_threads(lock);
}
// Request invocation of the given handler.
@@ -201,9 +200,14 @@
++outstanding_work_;
// Wake up a thread to execute the handler.
- if (!interrupt_one_idle_thread())
- if (task_handler_.next_ == 0 && handler_queue_end_ != &task_handler_)
+ if (!interrupt_one_idle_thread(lock))
+ {
+ if (!task_interrupted_)
+ {
+ task_interrupted_ = true;
task_.interrupt();
+ }
+ }
}
private:
@@ -214,7 +218,7 @@
{
if (outstanding_work_ == 0 && !stopped_)
{
- stop_all_threads();
+ stop_all_threads(lock);
ec = boost::system::error_code();
return 0;
}
@@ -230,11 +234,14 @@
handler_queue_ = h->next_;
if (handler_queue_ == 0)
handler_queue_end_ = 0;
- bool more_handlers = (handler_queue_ != 0);
- lock.unlock();
+ h->next_ = 0;
if (h == &task_handler_)
{
+ bool more_handlers = (handler_queue_ != 0);
+ task_interrupted_ = more_handlers || polling;
+ lock.unlock();
+
// If the task has already run and we're polling then we're done.
if (task_has_run && polling)
{
@@ -252,6 +259,7 @@
}
else
{
+ lock.unlock();
handler_cleanup c(lock, *this);
// Invoke the handler. May throw an exception.
@@ -264,31 +272,10 @@
else if (this_idle_thread)
{
// Nothing to run right now, so just wait for work to do.
- if (first_idle_thread_)
- {
- this_idle_thread->next = first_idle_thread_;
- this_idle_thread->prev = first_idle_thread_->prev;
- first_idle_thread_->prev->next = this_idle_thread;
- first_idle_thread_->prev = this_idle_thread;
- }
+ this_idle_thread->next = first_idle_thread_;
first_idle_thread_ = this_idle_thread;
- this_idle_thread->wakeup_event.clear();
- lock.unlock();
- this_idle_thread->wakeup_event.wait();
- lock.lock();
- if (this_idle_thread->next == this_idle_thread)
- {
- first_idle_thread_ = 0;
- }
- else
- {
- if (first_idle_thread_ == this_idle_thread)
- first_idle_thread_ = this_idle_thread->next;
- this_idle_thread->next->prev = this_idle_thread->prev;
- this_idle_thread->prev->next = this_idle_thread->next;
- this_idle_thread->next = this_idle_thread;
- this_idle_thread->prev = this_idle_thread;
- }
+ this_idle_thread->wakeup_event.clear(lock);
+ this_idle_thread->wakeup_event.wait(lock);
}
else
{
@@ -302,39 +289,44 @@
}
// Stop the task and all idle threads.
- void stop_all_threads()
+ void stop_all_threads(
+ boost::asio::detail::mutex::scoped_lock& lock)
{
stopped_ = true;
- interrupt_all_idle_threads();
- if (task_handler_.next_ == 0 && handler_queue_end_ != &task_handler_)
+ interrupt_all_idle_threads(lock);
+ if (!task_interrupted_)
+ {
+ task_interrupted_ = true;
task_.interrupt();
+ }
}
// Interrupt a single idle thread. Returns true if a thread was interrupted,
// false if no running thread could be found to interrupt.
- bool interrupt_one_idle_thread()
+ bool interrupt_one_idle_thread(
+ boost::asio::detail::mutex::scoped_lock& lock)
{
if (first_idle_thread_)
{
- first_idle_thread_->wakeup_event.signal();
- first_idle_thread_ = first_idle_thread_->next;
+ idle_thread_info* idle_thread = first_idle_thread_;
+ first_idle_thread_ = idle_thread->next;
+ idle_thread->next = 0;
+ idle_thread->wakeup_event.signal(lock);
return true;
}
return false;
}
// Interrupt all idle threads.
- void interrupt_all_idle_threads()
+ void interrupt_all_idle_threads(
+ boost::asio::detail::mutex::scoped_lock& lock)
{
- if (first_idle_thread_)
+ while (first_idle_thread_)
{
- first_idle_thread_->wakeup_event.signal();
- idle_thread_info* current_idle_thread = first_idle_thread_->next;
- while (current_idle_thread != first_idle_thread_)
- {
- current_idle_thread->wakeup_event.signal();
- current_idle_thread = current_idle_thread->next;
- }
+ idle_thread_info* idle_thread = first_idle_thread_;
+ first_idle_thread_ = idle_thread->next;
+ idle_thread->next = 0;
+ idle_thread->wakeup_event.signal(lock);
}
}
@@ -440,6 +432,7 @@
{
// Reinsert the task at the end of the handler queue.
lock_.lock();
+ task_io_service_.task_interrupted_ = true;
task_io_service_.task_handler_.next_ = 0;
if (task_io_service_.handler_queue_end_)
{
@@ -478,7 +471,7 @@
{
lock_.lock();
if (--task_io_service_.outstanding_work_ == 0)
- task_io_service_.stop_all_threads();
+ task_io_service_.stop_all_threads(lock_);
}
private:
@@ -503,6 +496,9 @@
}
} task_handler_;
+ // Whether the task has been interrupted.
+ bool task_interrupted_;
+
// The count of unfinished work.
int outstanding_work_;
@@ -522,7 +518,6 @@
struct idle_thread_info
{
event wakeup_event;
- idle_thread_info* prev;
idle_thread_info* next;
};
Modified: trunk/boost/asio/detail/win_event.hpp
==============================================================================
--- trunk/boost/asio/detail/win_event.hpp (original)
+++ trunk/boost/asio/detail/win_event.hpp 2007-09-01 02:41:15 EDT (Sat, 01 Sep 2007)
@@ -28,6 +28,7 @@
#include <boost/asio/detail/socket_types.hpp>
#include <boost/asio/detail/push_options.hpp>
+#include <boost/assert.hpp>
#include <boost/throw_exception.hpp>
#include <boost/asio/detail/pop_options.hpp>
@@ -60,21 +61,31 @@
}
// Signal the event.
- void signal()
+ template <typename Lock>
+ void signal(Lock& lock)
{
+ BOOST_ASSERT(lock.locked());
+ (void)lock;
::SetEvent(event_);
}
// Reset the event.
- void clear()
+ template <typename Lock>
+ void clear(Lock& lock)
{
+ BOOST_ASSERT(lock.locked());
+ (void)lock;
::ResetEvent(event_);
}
// Wait for the event to become signalled.
- void wait()
+ template <typename Lock>
+ void wait(Lock& lock)
{
+ BOOST_ASSERT(lock.locked());
+ lock.unlock();
::WaitForSingleObject(event_, INFINITE);
+ lock.lock();
}
private: