Gracefully deal with dangling symlinks

lib/spring/application.rb

@@ -66,7 +66,17 @@ def initialized?
 
     def start_watcher
       @watcher = Spring.watcher
-      @watcher.on_stale { state! :watcher_stale }
+
+      @watcher.on_stale do
+        state! :watcher_stale
+      end
+
+      if @watcher.respond_to? :on_debug
+        @watcher.on_debug do |message|
+          spring_env.log "[watcher:#{app_env}] #{message}"
+        end
+      end
+
       @watcher.start
     end
 

lib/spring/test/watcher_test.rb

@@ -162,6 +162,33 @@ def assert_not_stale
         watcher.add './foobar'
         assert watcher.files.empty?
       end
+
+      test "add symlink" do
+        File.write("#{dir}/bar", "bar")
+        File.symlink("#{dir}/bar", "#{dir}/foo")
+        watcher.add './foo'
+        assert_equal ["#{dir}/bar"], watcher.files.to_a
+      end
+
+      test "add dangling symlink" do
+        File.symlink("#{dir}/bar", "#{dir}/foo")
+        watcher.add './foo'
+        assert watcher.files.empty?
+      end
+
+      test "add directory with dangling symlink" do
+        subdir = "#{@dir}/subdir"
+        FileUtils.mkdir(subdir)
+        File.symlink("dangling", "#{subdir}/foo")
+
+        watcher.add subdir
+        assert_not_stale
+
+        # Adding a new file should mark as stale despite the dangling symlink.
+        File.write("#{subdir}/new-file", "new")
+        watcher.check_stale
+        assert_stale
+      end
     end
   end
 end

lib/spring/watcher/abstract.rb

@@ -23,9 +23,21 @@ def initialize(root, latency)
         @directories = Set.new
         @stale       = false
         @listeners   = []
+
+        @on_debug    = nil
+      end
+
+      def on_debug(&block)
+        @on_debug = block
+      end
+
+      def debug
+        @on_debug.call(yield) if @on_debug
       end
 
       def add(*items)
+        debug { "watcher: add: #{items.inspect}" }
+
         items = items.flatten.map do |item|
           item = Pathname.new(item)
 
@@ -36,14 +48,30 @@ def add(*items)
           end
         end
 
-        items = items.select(&:exist?)
+        items = items.select do |item|
+          if item.symlink?
+            item.readlink.exist?.tap do |exists|
+              if !exists
+                debug { "add: ignoring dangling symlink: #{item.inspect} -> #{item.readlink.inspect}" }
+              end
+            end
+          else
+            item.exist?
+          end
+        end
 
         synchronize {
           items.each do |item|
             if item.directory?
               directories << item.realpath.to_s
             else
-              files << item.realpath.to_s
+              begin
+                files << item.realpath.to_s
+              rescue Errno::ENOENT
+                # Race condition. Ignore symlinks whose target was removed
+                # since the check above, or are deeply chained.
+                debug { "add: ignoring now-dangling symlink: #{item.inspect} -> #{item.readlink.inspect}" }
+              end
             end
           end
 
@@ -56,16 +84,19 @@ def stale?
       end
 
       def on_stale(&block)
+        debug { "added listener: #{block.inspect}" }
         @listeners << block
       end
 
       def mark_stale
         return if stale?
         @stale = true
+        debug { "marked stale, calling listeners: listeners=#{@listeners.inspect}" }
         @listeners.each(&:call)
       end
 
       def restart
+        debug { "restarting" }
         stop
         start
       end

lib/spring/watcher/polling.rb

@@ -12,7 +12,13 @@ def initialize(root, latency)
       end
 
       def check_stale
-        synchronize { mark_stale if mtime < compute_mtime }
+        synchronize do
+          computed = compute_mtime
+          if mtime < computed
+            debug { "check_stale: mtime=#{mtime.inspect} < computed=#{computed.inspect}" }
+            mark_stale
+          end
+        end
       end
 
       def add(*)
@@ -21,36 +27,61 @@ def add(*)
       end
 
       def start
+        debug { "start: poller=#{@poller.inspect}" }
         unless @poller
           @poller = Thread.new {
             Thread.current.abort_on_exception = true
 
-            loop do
-              Kernel.sleep latency
-              check_stale
+            begin
+              loop do
+                Kernel.sleep latency
+                check_stale
+              end
+            rescue Exception => e
+              debug do
+                "poller: aborted: #{e.class}: #{e}\n  #{e.backtrace.join("\n  ")}"
+              end
+              raise
             end
           }
         end
       end
 
       def stop
+        debug { "stopping poller: #{@poller.inspect}" }
         if @poller
           @poller.kill
           @poller = nil
         end
       end
 
       def subjects_changed
-        @mtime = compute_mtime
+        computed = compute_mtime
+        debug { "subjects_changed: mtime #{@mtime} -> #{computed}" }
+        @mtime = computed
       end
 
       private
 
       def compute_mtime
-        expanded_files.map { |f| File.mtime(f).to_f }.max || 0
-      rescue Errno::ENOENT
-        # if a file does no longer exist, the watcher is always stale.
-        Float::MAX
+        expanded_files.map do |f|
+          # Get the mtime of symlink targets. Ignore dangling symlinks.
+          if File.symlink?(f)
+            begin
+              File.mtime(f)
+            rescue Errno::ENOENT
+              0
+            end
+          # If a file no longer exists, treat it as changed.
+          else
+            begin
+              File.mtime(f)
+            rescue Errno::ENOENT
+              debug { "compute_mtime: no longer exists: #{f}" }
+              Float::MAX
+            end
+          end.to_f
+        end.max || 0
       end
 
       def expanded_files