Permalink
Browse files

removes the mutex around `changed`

This method needs to ensure that if a change happens, it is going to be registered.
With this refactor suggested by @matthewd race conditions do not matter because if
no file is watched, nothing is done. And as long as some invocation sets the flag
to true, it will stay true.

The refactor leaves a race condition in which two simultaneous threads that watch
some of the files passed do the actual work in `any?`, whereas the mutex guaranteed
that was done at most once. But this is considered to be a better tradeoff.
  • Loading branch information...
fxn committed Nov 22, 2015
1 parent 4596c1a commit 0b9812bddea50f974d51175ae81bfd6d8407f946
Showing with 2 additions and 7 deletions.
  1. +2 −7 activesupport/lib/active_support/file_evented_update_checker.rb
@@ -1,7 +1,6 @@
require 'listen'
require 'set'
require 'pathname'
require 'thread'
require 'concurrent/atomic/atomic_boolean'
module ActiveSupport
@@ -22,8 +21,6 @@ def initialize(files, dirs = {}, &block)
if (dtw = directories_to_watch).any?
Listen.to(*dtw, &method(:changed)).start
end
@mutex = Mutex.new
end
def updated?
@@ -45,10 +42,8 @@ def execute_if_updated
private
def changed(modified, added, removed)
@mutex.synchronize do
unless updated?
@updated.value = (modified + added + removed).any? { |f| watching?(f) }
end
unless updated?
@updated.make_true if (modified + added + removed).any? { |f| watching?(f) }
end
end

0 comments on commit 0b9812b

Please sign in to comment.