From 010f1406f93d97c2fab06308786e4548c17fd588 Mon Sep 17 00:00:00 2001 From: Jonathan Hefner Date: Sat, 26 Sep 2020 15:00:52 -0500 Subject: [PATCH] Prevent race condition in Notifier#run / #stop 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. --- lib/rb-inotify/notifier.rb | 3 ++- spec/notifier_spec.rb | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/rb-inotify/notifier.rb b/lib/rb-inotify/notifier.rb index 89be6f8..e653d49 100644 --- a/lib/rb-inotify/notifier.rb +++ b/lib/rb-inotify/notifier.rb @@ -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' @@ -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. diff --git a/spec/notifier_spec.rb b/spec/notifier_spec.rb index cd1abfd..8370151 100644 --- a/spec/notifier_spec.rb +++ b/spec/notifier_spec.rb @@ -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