diff --git a/spec/detach_race_condition_spec.rb b/spec/detach_race_condition_spec.rb index 5d11be2..f1c39a2 100644 --- a/spec/detach_race_condition_spec.rb +++ b/spec/detach_race_condition_spec.rb @@ -1,10 +1,13 @@ require 'spec_helper' describe Cool.io::Loop do + # An IOWatcher that drains its pipe and then runs a user-supplied block, + # receiving itself as the argument. class Victim < Cool.io::IOWatcher - def initialize(io) - super + def initialize(io, &on_readable) + super(io) @io = io + @on_readable = on_readable end def on_readable @@ -12,38 +15,57 @@ def on_readable @io.read_nonblock(1024) rescue IO::WaitReadable, EOFError end + @on_readable.call(self) if @on_readable end end # https://github.com/socketry/cool.io/issues/87 - it "does not raise TypeError when a watcher is detached while an event is pending" do - loop = Cool.io::Loop.default - + # + # Several watchers have an event pending in the same loop iteration. When the + # first one dispatched detaches the others, the loop must skip their now-stale + # pending events instead of dispatching them to a detached watcher. Before the + # fix that raised "TypeError: wrong argument type nil (expected Coolio::Loop)" + # and could crash the VM. + # + # This is exercised deterministically within a single thread (a preceding + # callback detaching another watcher in the same loop cycle). The original + # reproduction detached from a separate thread while the loop was polling, + # which is an unsupported concurrent mutation of libev (not thread-safe) and + # crashed intermittently on macOS. + it "does not raise when a watcher with a pending event is detached during dispatch" do iterations = 200 expect { iterations.times do - r_victim, w_victim = IO.pipe - victim_watcher = Victim.new(r_victim) - victim_watcher.attach(loop) - - t1 = Thread.new do - sleep 0.01 - w_victim.write("dummy\n") - end - - t2 = Thread.new do - sleep 0.01 - victim_watcher.detach + coolio_loop = Cool.io::Loop.new + pipes = [] + watchers = [] + + 5.times do + r, w = IO.pipe + pipes << [r, w] + + watcher = Victim.new(r) do |fired| + # Detach every other watcher whose event is already queued for this + # same loop iteration. + watchers.each do |other| + other.detach if !other.equal?(fired) && other.attached? + end + end + watcher.attach(coolio_loop) + watchers << watcher + + w.write("dummy\n") # make the read end readable so an event is pending end - loop.run_once + coolio_loop.run_once - t1.join - t2.join + # Only the first dispatched watcher runs; it detaches the other four, + # whose pending events are then skipped. + expect(watchers.count(&:attached?)).to eq(1) - r_victim.close - w_victim.close + watchers.each { |watcher| watcher.detach if watcher.attached? } + pipes.each { |r, w| r.close; w.close } end }.not_to raise_error end