about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2009-12-29 12:59:01 -0800
committerEric Wong <normalperson@yhbt.net>2009-12-29 12:59:01 -0800
commit31ee6b4daa1da9cd02e75b27924b2729345e999d (patch)
treec00cc159a7e330763ae72f307dab3664d0f3d5b9
parentd5375f5c24abfae0173007f47bc9e83139d556b5 (diff)
downloadrainbows-31ee6b4daa1da9cd02e75b27924b2729345e999d.tar.gz
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)
-rw-r--r--lib/rainbows.rb12
-rw-r--r--lib/rainbows/thread_pool.rb3
-rw-r--r--lib/rainbows/thread_spawn.rb5
-rwxr-xr-xt/t0012-spurious-wakeups-quiet.sh41
4 files changed, 56 insertions, 5 deletions
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
diff --git a/t/t0012-spurious-wakeups-quiet.sh b/t/t0012-spurious-wakeups-quiet.sh
new file mode 100755
index 0000000..23557b7
--- /dev/null
+++ b/t/t0012-spurious-wakeups-quiet.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+nr=${nr-4}
+. ./test-lib.sh
+
+# ApacheBench (ab) is commonly installed in the sbin paths in Debian-based
+# systems...
+AB="$(which ab 2>/dev/null || :)"
+if test -z "$AB"
+then
+        AB=$(PATH=/usr/local/sbin:/usr/sbin:$PATH which ab 2>/dev/null || :)
+fi
+
+if test -z "$AB"
+then
+        t_info "skipping $T since 'ab' could not be found"
+        exit 0
+fi
+
+t_plan 4 "quiet spurious wakeups for $model"
+
+t_begin "setup and start" && {
+        rainbows_setup $model
+        echo "preload_app true" >> $unicorn_config
+        echo "worker_processes $nr" >> $unicorn_config
+        rainbows -D env.ru -c $unicorn_config -E none
+        rainbows_wait_start
+}
+
+t_begin "spam the server with requests" && {
+        $AB -c1 -n100 http://$listen/
+}
+
+t_begin "killing succeeds" && {
+        kill -QUIT $rainbows_pid
+}
+
+t_begin "check stderr" && {
+        check_stderr
+}
+
+t_done