Boost logo

Boost-Commit :

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:


Boost-Commit list run by bdawes at acm.org, david.abrahams at rcn.com, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk