From 31ee6b4daa1da9cd02e75b27924b2729345e999d Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Tue, 29 Dec 2009 12:59:01 -0800
Subject: quiet spurious wakeups for accept() in Thread* models

Under all MRI 1.8, a blocking Socket#accept Ruby method (needs
to[1]) translate to a non-blocking accept(2) system call that may
wake up threads/processes unnecessarily.  Unfortunately, we
failed to trap and ignore EAGAIN in those cases.

This issue did not affect Ruby 1.9 running under modern Linux
kernels where a _blocking_ accept(2) system call is not (easily,
at least) susceptible to spurious wakeups.  Non-Linux systems
running Ruby 1.9 may be affected.

[1] - using a blocking accept(2) on a shared socket with
      green threads is dangerous, as noted in
      commit ee7fe220ccbc991e1e7cbe982caf48e3303274c7
      (and commit 451ca6997b4f298b436605b7f0af75f369320425)
---
 lib/rainbows.rb              | 12 ++++++++++++
 lib/rainbows/thread_pool.rb  |  3 +--
 lib/rainbows/thread_spawn.rb |  5 ++---
 3 files changed, 15 insertions(+), 5 deletions(-)

(limited to 'lib')

diff --git a/lib/rainbows.rb b/lib/rainbows.rb
index d3a3e7d..9260649 100644
--- a/lib/rainbows.rb
+++ b/lib/rainbows.rb
@@ -60,6 +60,13 @@ module Rainbows
 
     # returns nil if accept fails
     if defined?(Fcntl::FD_CLOEXEC)
+      def sync_accept(sock)
+        rv = sock.accept
+        rv.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC)
+        rv
+      rescue Errno::EAGAIN, Errno::ECONNABORTED, Errno::EINTR
+      end
+
       def accept(sock)
         rv = sock.accept_nonblock
         rv.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC)
@@ -67,6 +74,11 @@ module Rainbows
       rescue Errno::EAGAIN, Errno::ECONNABORTED
       end
     else
+      def sync_accept(sock)
+        sock.accept
+      rescue Errno::EAGAIN, Errno::ECONNABORTED, Errno::EINTR
+      end
+
       def accept(sock)
         sock.accept_nonblock
       rescue Errno::EAGAIN, Errno::ECONNABORTED
diff --git a/lib/rainbows/thread_pool.rb b/lib/rainbows/thread_pool.rb
index 3ef719a..a68fcc6 100644
--- a/lib/rainbows/thread_pool.rb
+++ b/lib/rainbows/thread_pool.rb
@@ -44,8 +44,7 @@ module Rainbows
     def sync_worker
       s = LISTENERS.first
       begin
-        process_client(s.accept)
-      rescue Errno::EINTR, Errno::ECONNABORTED
+        c = Rainbows.sync_accept(s) and process_client(c)
       rescue => e
         Error.listen_loop(e)
       end while G.alive
diff --git a/lib/rainbows/thread_spawn.rb b/lib/rainbows/thread_spawn.rb
index eb3ca75..75cc150 100644
--- a/lib/rainbows/thread_spawn.rb
+++ b/lib/rainbows/thread_spawn.rb
@@ -34,8 +34,8 @@ module Rainbows
               # unlikely one.  Since this case is (or should be) uncommon,
               # just busy wait when we have to.
               sleep(0.01)
-            else
-              klass.new(l.accept) do |c|
+            elsif c = Rainbows.sync_accept(l)
+              klass.new(c) do |c|
                 begin
                   lock.synchronize { G.cur += 1 }
                   process_client(c)
@@ -44,7 +44,6 @@ module Rainbows
                 end
               end
             end
-          rescue Errno::EINTR, Errno::ECONNABORTED
           rescue => e
             Error.listen_loop(e)
           end while G.alive
-- 
cgit v1.2.3-24-ge0c7