Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fix a synchronisation bug

The run method will almost always be called inside a thread. This commit fixes
a synchronisation problem between the run and stop methods by explicitly opening
a pipe on the beginning of the run method.

The source of the problem is the fact that when someone stops the watcher,
the @path would become nil and calling the private method pipe would create a new pipe.

A sitiuation where this could be a problem is when the run method is waiting on changes
(blocking on select), note here that @running does not matter anymore, the code is blocking
on select!

Stoping the watcher and firing an event at the same time would unblock the run method
and getting the results in the old code would have created a new pipe and blocked on readline.
  • Loading branch information...
commit fb08ed268339fcf93551a64031008519e9ec6505 1 parent a331537
Maher Sallam authored

Showing 1 changed file with 13 additions and 11 deletions. Show diff stats Hide diff stats

  1. 24  lib/rb-fsevent/fsevent.rb
24  lib/rb-fsevent/fsevent.rb
@@ -32,12 +32,14 @@ def watch(watch_paths, options=nil, &block)
32 32
   end
33 33
 
34 34
   def run
  35
+    @pipe    = open_pipe
35 36
     @running = true
  37
+
36 38
     # please note the use of IO::select() here, as it is used specifically to
37 39
     # preserve correct signal handling behavior in ruby 1.8.
38  
-    while @running && IO::select([pipe], nil, nil, nil)
39  
-      if line = pipe.readline
40  
-        modified_dir_paths = line.split(":").select { |dir| dir != "\n" }
  40
+    while @running && IO::select([@pipe], nil, nil, nil)
  41
+      if line = @pipe.readline
  42
+        modified_dir_paths = line.split(':').select { |dir| dir != "\n" }
41 43
         callback.call(modified_dir_paths)
42 44
       end
43 45
     end
@@ -47,18 +49,18 @@ def run
47 49
   end
48 50
 
49 51
   def stop
50  
-    if pipe
51  
-      Process.kill("KILL", pipe.pid)
52  
-      pipe.close
  52
+    unless @pipe.nil?
  53
+      Process.kill('KILL', @pipe.pid)
  54
+      @pipe.close
53 55
     end
54 56
   rescue IOError
55 57
   ensure
56  
-    @pipe = @running = nil
  58
+    @running = false
57 59
   end
58 60
 
59 61
   if RUBY_VERSION < '1.9'
60  
-    def pipe
61  
-      @pipe ||= IO.popen("#{self.class.watcher_path} #{options_string} #{shellescaped_paths}")
  62
+    def open_pipe
  63
+      IO.popen("#{self.class.watcher_path} #{options_string} #{shellescaped_paths}")
62 64
     end
63 65
 
64 66
     private
@@ -89,8 +91,8 @@ def shellescape(str)
89 91
       return str
90 92
     end
91 93
   else
92  
-    def pipe
93  
-      @pipe ||= IO.popen([self.class.watcher_path] + @options + @paths)
  94
+    def open_pipe
  95
+      IO.popen([self.class.watcher_path] + @options + @paths)
94 96
     end
95 97
   end
96 98
 

0 notes on commit fb08ed2

Please sign in to comment.
Something went wrong with that request. Please try again.