[Boost-commit] svn:boost r76990 - in trunk: boost/asio/detail boost/asio/detail/impl libs/asio/doc

Subject: [Boost-commit] svn:boost r76990 - in trunk: boost/asio/detail boost/asio/detail/impl libs/asio/doc
From: chris_at_[hidden]
Date: 2012-02-12 07:53:07

Author: chris_kohlhoff
Date: 2012-02-12 07:53:06 EST (Sun, 12 Feb 2012)
New Revision: 76990
URL: http://svn.boost.org/trac/boost/changeset/76990

Log:
Reverted earlier change to allow some speculative operations to be performed
without holding the lock, as it introduced a race condition in some
multithreaded scenarios.

Text files modified: 
   trunk/boost/asio/detail/epoll_reactor.hpp       |     2 --                                      
   trunk/boost/asio/detail/impl/epoll_reactor.ipp  |    35 ++---------------------------------     
   trunk/boost/asio/detail/impl/kqueue_reactor.ipp |    32 ++------------------------------        
   trunk/boost/asio/detail/kqueue_reactor.hpp      |     2 --                                      
   trunk/libs/asio/doc/history.qbk                 |     3 +++                                     
   5 files changed, 7 insertions(+), 67 deletions(-)

Modified: trunk/boost/asio/detail/epoll_reactor.hpp
==============================================================================
--- trunk/boost/asio/detail/epoll_reactor.hpp	(original)
+++ trunk/boost/asio/detail/epoll_reactor.hpp	2012-02-12 07:53:06 EST (Sun, 12 Feb 2012)
@@ -56,8 +56,6 @@
     descriptor_state* next_;
     descriptor_state* prev_;
 
-    bool op_queue_is_empty_[max_ops];
-
     mutex mutex_;
     epoll_reactor* reactor_;
     int descriptor_;

Modified: trunk/boost/asio/detail/impl/epoll_reactor.ipp
==============================================================================
--- trunk/boost/asio/detail/impl/epoll_reactor.ipp	(original)
+++ trunk/boost/asio/detail/impl/epoll_reactor.ipp	2012-02-12 07:53:06 EST (Sun, 12 Feb 2012)
@@ -156,10 +156,6 @@
     descriptor_data->reactor_ = this;
     descriptor_data->descriptor_ = descriptor;
     descriptor_data->shutdown_ = false;
-
-    for (int i = 0; i < max_ops; ++i)
-      descriptor_data->op_queue_is_empty_[i] =
-        descriptor_data->op_queue_[i].empty();
   }
 
   epoll_event ev = { 0, { 0 } };
@@ -185,10 +181,6 @@
     descriptor_data->descriptor_ = descriptor;
     descriptor_data->shutdown_ = false;
     descriptor_data->op_queue_[op_type].push(op);
-
-    for (int i = 0; i < max_ops; ++i)
-      descriptor_data->op_queue_is_empty_[i] =
-        descriptor_data->op_queue_[i].empty();
   }
 
   epoll_event ev = { 0, { 0 } };
@@ -220,22 +212,6 @@
     return;
   }
 
-  bool perform_speculative = allow_speculative;
-  if (perform_speculative)
-  {
-    if (descriptor_data->op_queue_is_empty_[op_type]
-        && (op_type != read_op
-          || descriptor_data->op_queue_is_empty_[except_op]))
-    {
-      if (op->perform())
-      {
-        io_service_.post_immediate_completion(op);
-        return;
-      }
-      perform_speculative = false;
-    }
-  }
-
   mutex::scoped_lock descriptor_lock(descriptor_data->mutex_);
 
   if (descriptor_data->shutdown_)
@@ -244,17 +220,11 @@
     return;
   }
 
-  for (int i = 0; i < max_ops; ++i)
-    descriptor_data->op_queue_is_empty_[i] =
-      descriptor_data->op_queue_[i].empty();
-
-  if (descriptor_data->op_queue_is_empty_[op_type])
+  if (descriptor_data->op_queue_[op_type].empty())
   {
     if (allow_speculative)
     {
-      if (perform_speculative
-          && (op_type != read_op
-            || descriptor_data->op_queue_is_empty_[except_op]))
+      if (op_type != read_op || descriptor_data->op_queue_[except_op].empty())
       {
         if (op->perform())
         {
@@ -275,7 +245,6 @@
   }
 
   descriptor_data->op_queue_[op_type].push(op);
-  descriptor_data->op_queue_is_empty_[op_type] = false;
   io_service_.work_started();
 }
 

Modified: trunk/boost/asio/detail/impl/kqueue_reactor.ipp
==============================================================================
--- trunk/boost/asio/detail/impl/kqueue_reactor.ipp	(original)
+++ trunk/boost/asio/detail/impl/kqueue_reactor.ipp	2012-02-12 07:53:06 EST (Sun, 12 Feb 2012)
@@ -133,10 +133,6 @@
   descriptor_data->descriptor_ = descriptor;
   descriptor_data->shutdown_ = false;
 
-  for (int i = 0; i < max_ops; ++i)
-    descriptor_data->op_queue_is_empty_[i] =
-      descriptor_data->op_queue_[i].empty();
-
   return 0;
 }
 
@@ -152,10 +148,6 @@
   descriptor_data->shutdown_ = false;
   descriptor_data->op_queue_[op_type].push(op);
 
-  for (int i = 0; i < max_ops; ++i)
-    descriptor_data->op_queue_is_empty_[i] =
-      descriptor_data->op_queue_[i].empty();
-
   struct kevent event;
   switch (op_type)
   {
@@ -196,21 +188,6 @@
     return;
   }
 
-  if (allow_speculative)
-  {
-    if (descriptor_data->op_queue_is_empty_[op_type]
-        && (op_type != read_op
-          || descriptor_data->op_queue_is_empty_[except_op]))
-    {
-      if (op->perform())
-      {
-        io_service_.post_immediate_completion(op);
-        return;
-      }
-      allow_speculative = false;
-    }
-  }
-
   mutex::scoped_lock descriptor_lock(descriptor_data->mutex_);
 
   if (descriptor_data->shutdown_)
@@ -219,16 +196,12 @@
     return;
   }
 
-  for (int i = 0; i < max_ops; ++i)
-    descriptor_data->op_queue_is_empty_[i] =
-      descriptor_data->op_queue_[i].empty();
-
-  bool first = descriptor_data->op_queue_is_empty_[op_type];
+  bool first = descriptor_data->op_queue_[op_type].empty();
   if (first)
   {
     if (allow_speculative)
     {
-      if (op_type != read_op || descriptor_data->op_queue_is_empty_[except_op])
+      if (op_type != read_op || descriptor_data->op_queue_[except_op].empty())
       {
         if (op->perform())
         {
@@ -241,7 +214,6 @@
   }
 
   descriptor_data->op_queue_[op_type].push(op);
-  descriptor_data->op_queue_is_empty_[op_type] = false;
   io_service_.work_started();
 
   if (first)

Modified: trunk/boost/asio/detail/kqueue_reactor.hpp
==============================================================================
--- trunk/boost/asio/detail/kqueue_reactor.hpp	(original)
+++ trunk/boost/asio/detail/kqueue_reactor.hpp	2012-02-12 07:53:06 EST (Sun, 12 Feb 2012)
@@ -66,8 +66,6 @@
     descriptor_state* next_;
     descriptor_state* prev_;
 
-    bool op_queue_is_empty_[max_ops];
-
     mutex mutex_;
     int descriptor_;
     op_queue<reactor_op> op_queue_[max_ops];

Modified: trunk/libs/asio/doc/history.qbk
==============================================================================
--- trunk/libs/asio/doc/history.qbk	(original)
+++ trunk/libs/asio/doc/history.qbk	2012-02-12 07:53:06 EST (Sun, 12 Feb 2012)
@@ -29,6 +29,9 @@
 * Fixed a non-paged pool "leak" on Windows when an `io_service` is repeatedly
   run without anything to do
   ([@https://svn.boost.org/trac/boost/ticket/6321 #6321]).
+* Reverted earlier change to allow some speculative operations to be performed
+  without holding the lock, as it introduced a race condition in some
+  multithreaded scenarios.
 
 [heading Asio 1.6.1 / Boost 1.48]