diff options
author | Eric Wong <normalperson@yhbt.net> | 2009-12-29 12:59:01 -0800 |
---|---|---|
committer | Eric Wong <normalperson@yhbt.net> | 2009-12-29 12:59:01 -0800 |
commit | 31ee6b4daa1da9cd02e75b27924b2729345e999d (patch) | |
tree | c00cc159a7e330763ae72f307dab3664d0f3d5b9 | |
parent | d5375f5c24abfae0173007f47bc9e83139d556b5 (diff) | |
download | rainbows-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.rb | 12 | ||||
-rw-r--r-- | lib/rainbows/thread_pool.rb | 3 | ||||
-rw-r--r-- | lib/rainbows/thread_spawn.rb | 5 | ||||
-rwxr-xr-x | t/t0012-spurious-wakeups-quiet.sh | 41 |
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 |