[Boost-commit] svn:boost r39098 - trunk/boost/asio/detail

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: