aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorschneems <richard.schneeman@gmail.com>2016-06-06 11:26:22 -0500
committerschneems <richard.schneeman@gmail.com>2016-06-06 11:34:32 -0500
commit7bd41994480c17db71fdc07e3447ade929eaa386 (patch)
tree1a371428ed303eea71d9d1f991810359e545d60e
parent3fc5577363dc4049cd1d0bba8f3fa9296f1ced57 (diff)
downloadrails-7bd41994480c17db71fdc07e3447ade929eaa386.tar.gz
rails-7bd41994480c17db71fdc07e3447ade929eaa386.tar.bz2
rails-7bd41994480c17db71fdc07e3447ade929eaa386.zip
EventedFileUpdateChecker boots once per process
We need one file checker booted per process as talked about in #24990. Before we do a check to see if any updates have been registered by the listener we first check to make sure that the current process has booted a listener. We are intentionally not starting a listener when the checker is created. This way we can avoid #25259 in which puma warns of multiple threads created before fork. As written the listener for each process will be invoked by the `ActionDispatch::Executor` middleware when the `updated?` method is called. This is the first middleware on the stack and will be invoked before application code is read into memory. The downside of this approach is that the API is a little less obvious. I.e. that you have to call `updated?` to get the listener to start is not intuitive. We could make `boot!` not private if we want to make the API a little nicer. Alternatively we could boot when the checker is initialized however this reintroduces the puma threads warning, and also means that in cases of `rails server` or when using `preload!` that we have extra threads notifying of changes on a process that we don't care about. [close #24990] [close #25259]
-rw-r--r--activesupport/lib/active_support/evented_file_update_checker.rb15
-rw-r--r--activesupport/test/evented_file_update_checker_test.rb3
2 files changed, 12 insertions, 6 deletions
diff --git a/activesupport/lib/active_support/evented_file_update_checker.rb b/activesupport/lib/active_support/evented_file_update_checker.rb
index 21fdf7bb80..f549ce4040 100644
--- a/activesupport/lib/active_support/evented_file_update_checker.rb
+++ b/activesupport/lib/active_support/evented_file_update_checker.rb
@@ -13,11 +13,12 @@ module ActiveSupport
@dirs[@ph.xpath(dir)] = Array(exts).map { |ext| @ph.normalize_extension(ext) }
end
- @block = block
- @updated = Concurrent::AtomicBoolean.new(false)
- @lcsp = @ph.longest_common_subpath(@dirs.keys)
+ @block = block
+ @updated = Concurrent::AtomicBoolean.new(false)
+ @lcsp = @ph.longest_common_subpath(@dirs.keys)
+ @pid_hash = Concurrent::Hash.new
- if (dtw = directories_to_watch).any?
+ if (@dtw = directories_to_watch).any?
# Loading listen triggers warnings. These are originated by a legit
# usage of attr_* macros for private attributes, but adds a lot of noise
# to our test suite. Thus, we lazy load it and disable warnings locally.
@@ -28,11 +29,11 @@ module ActiveSupport
raise LoadError, "Could not load the 'listen' gem. Add `gem 'listen'` to the development group of your Gemfile", e.backtrace
end
end
- Listen.to(*dtw, &method(:changed)).start
end
end
def updated?
+ boot! unless @pid_hash[Process.pid]
@updated.true?
end
@@ -50,6 +51,10 @@ module ActiveSupport
end
private
+ def boot!
+ Listen.to(*@dtw, &method(:changed)).start
+ @pid_hash[Process.pid] = true
+ end
def changed(modified, added, removed)
unless updated?
diff --git a/activesupport/test/evented_file_update_checker_test.rb b/activesupport/test/evented_file_update_checker_test.rb
index bc3f77bd54..ce2e05da2c 100644
--- a/activesupport/test/evented_file_update_checker_test.rb
+++ b/activesupport/test/evented_file_update_checker_test.rb
@@ -11,7 +11,8 @@ class EventedFileUpdateCheckerTest < ActiveSupport::TestCase
end
def new_checker(files = [], dirs = {}, &block)
- ActiveSupport::EventedFileUpdateChecker.new(files, dirs, &block).tap do
+ ActiveSupport::EventedFileUpdateChecker.new(files, dirs, &block).tap do |c|
+ c.updated?
wait
end
end