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.
guard · Sep 26, 2020 · 010f140 · 010f140
1 parent 34e3db4
commit 010f140
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 1 deletion.
diff --git 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
@@ -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