Eric Wong
2009-Nov-27 08:22 UTC
[Rev-talk] [PATCH] async_watcher: do not trigger on spurious wakeups
Also pushed out to git://yhbt.net/rev.git I actually found this bug while chasing down some weird behavior with IO#read under 1.8 (which does not affect 1.9). I was unable to reproduce what was happening inside Rainbows![1] in an isolated test case, but I found another failure case for this test. [1] - which does not share AsyncWatcher across fork, but does handle a lot of signals.>From e2ab2b774ce6431519a42ac5b831708f14069801 Mon Sep 17 00:00:00 2001From: Eric Wong <normalperson at yhbt.net> Date: Thu, 26 Nov 2009 18:11:17 -0800 Subject: [PATCH] async_watcher: do not trigger on spurious wakeups Also included is a convoluted test case that is able to reliably reproduce the failure on both a UP and SMP Linux 2.6 machines without this change. --- lib/rev/async_watcher.rb | 7 ++++- spec/async_watcher_spec.rb | 57 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletions(-) create mode 100644 spec/async_watcher_spec.rb diff --git a/lib/rev/async_watcher.rb b/lib/rev/async_watcher.rb index d038742..df12c34 100644 --- a/lib/rev/async_watcher.rb +++ b/lib/rev/async_watcher.rb @@ -31,7 +31,12 @@ module Rev def on_readable # Read a byte from the pipe. This clears readability, unless # another signal is pending - @reader.read 1 + begin + @reader.read_nonblock 1 + rescue Errno::EAGAIN + # in case there are spurious wakeups from forked processs + return + end on_signal end end diff --git a/spec/async_watcher_spec.rb b/spec/async_watcher_spec.rb new file mode 100644 index 0000000..ecce516 --- /dev/null +++ b/spec/async_watcher_spec.rb @@ -0,0 +1,57 @@ +require File.dirname(__FILE__) + ''/../lib/rev'' +require ''tempfile'' +require ''fcntl'' + +describe Rev::AsyncWatcher do + + it "does not signal on spurious wakeups" do + aw = Rev::AsyncWatcher.new + tmp = Tempfile.new(''rev_async_watcher_test'') + nr_fork = 2 # must be at least two for spurious wakeups + + # We have aetter chance of failing if this overflows the pipe buffer + # which POSIX requires >= 512 bytes, Linux 2.6 uses 4096 bytes + nr_signal = 4096 * 4 + + append = File.open(tmp.path, "ab") + append.sync = true + rd, wr = ::IO.pipe + + aw.on_signal { append.syswrite("#$$\n") } + children = nr_fork.times.map do + fork do + trap(:TERM) { exit!(0) } + rloop = Rev::Loop.default + aw.attach(rloop) + wr.write ''.'' # signal to master that we''re ready + rloop.run + exit!(1) # should not get here + end + end + + # ensure children are ready + nr_fork.times { rd.sysread(1).should == ''.'' } + + # send our signals + nr_signal.times { aw.signal } + + # wait for the pipe buffer to be consumed by the children + sleep 1 while tmp.stat.ctime >= (Time.now - 4) + + children.each do |pid| + Process.kill(:TERM, pid) + _, status = Process.waitpid2(pid) + status.exitstatus.should == 0 + end + + # we should''ve written a line for every signal we sent + lines = tmp.readlines + lines.size.should == nr_signal + + # theoretically a bad kernel scheduler could give us fewer... + lines.sort.uniq.size.should == nr_fork + + tmp.close! + end + +end -- Eric Wong