Skip to content

Commit

Permalink
Prevent race condition in Notifier#run / #stop
Browse files Browse the repository at this point in the history
There is a potential race condition when a notifier is run in a new
thread and then immediately stopped in the original thread.  If
`Notifier#stop` sets `@stop = true` before `Notifier#run` sets
`@stop = false`, then `Notifier#run` will loop until `Notifier#stop` is
called again.  Furthermore, if `Notifier#run` acquires the `@running`
mutex before `Notifier#stop` does (but after `Notifier#stop` sets
`@stop = true`), then `Notifier#stop` will get stuck waiting for the
mutex while `Notifier#run` infinitely loops.

This commit modifies `Notifier#run` to assume `@stop == false` when it
begins, and to reset `@stop = false` only after its loop terminates,
thus eliminating the race.
  • Loading branch information
jonathanhefner authored and ioquatix committed Sep 26, 2020
1 parent 34e3db4 commit 010f140
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/rb-inotify/notifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ def fd
# @raise [SystemCallError] if inotify failed to initialize for some reason
def initialize
@running = Mutex.new
@stop = false
@pipe = IO.pipe
# JRuby shutdown sometimes runs IO finalizers before all threads finish.
if RUBY_ENGINE == 'jruby'
Expand Down Expand Up @@ -231,12 +232,12 @@ def watch(path, *flags, &callback)
def run
@running.synchronize do
Thread.current[:INOTIFY_RUN_THREAD] = true
@stop = false

process until @stop
end
ensure
Thread.current[:INOTIFY_RUN_THREAD] = false
@stop = false
end

# Stop watching for filesystem events.
Expand Down
14 changes: 14 additions & 0 deletions spec/notifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,20 @@
dir.join("one.txt").write("hello world")
run_thread.join(1) or raise "timeout"
end

it "can be stopped before it starts processing" do
barrier = Concurrent::Event.new

run_thread = Thread.new do
barrier.wait
@notifier.run
end

@notifier.stop
barrier.set

run_thread.join(1) or raise "timeout"
end
end

describe :fd do
Expand Down

0 comments on commit 010f140

Please sign in to comment.