From eba1534939fe1cf005746f12446235bdd52014c1 Mon Sep 17 00:00:00 2001 From: Jonathan Hefner Date: Fri, 28 Aug 2020 16:23:54 -0500 Subject: [PATCH] Listen on fork in EventedFileUpdateChecker The Listen gem does not notify across process forks, and so forked processes must re-create their listeners. `EventedFileUpdateChecker` handled this in `updated?` by checking if `Process.pid` was different. Because file updates may have occurred after the start of the fork but before the call to `updated?`, `updated?` would always return true in this case. However, the above approach can result in unnecessary application reloading, particularly when using Spring. That, in turn, can cause unexpected and problematic behavior: #39431, #37591. This commit changes listener re-creation to occur immediately after fork, and changes `updated?` to return true only when necessary. --- .../evented_file_update_checker.rb | 18 +++++++----------- .../test/evented_file_update_checker_test.rb | 4 ---- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/activesupport/lib/active_support/evented_file_update_checker.rb b/activesupport/lib/active_support/evented_file_update_checker.rb index 18a1b4af5d..f9bc3be9be 100644 --- a/activesupport/lib/active_support/evented_file_update_checker.rb +++ b/activesupport/lib/active_support/evented_file_update_checker.rb @@ -4,6 +4,7 @@ require "set" require "pathname" require "concurrent/atomic/atomic_boolean" require "listen" +require "active_support/fork_tracker" module ActiveSupport # Allows you to "listen" to changes in a file system. @@ -40,20 +41,11 @@ module ActiveSupport end @block = block - @pid = Process.pid @core = Core.new(files, dirs) ObjectSpace.define_finalizer(self, @core.finalizer) end def updated? - @core.mutex.synchronize do - if @pid != Process.pid - @core.start - @pid = Process.pid - @core.updated.make_true - end - end - if @core.restart? @core.thread_safely(&:restart) @core.updated.make_true @@ -76,7 +68,7 @@ module ActiveSupport end class Core - attr_reader :updated, :mutex + attr_reader :updated def initialize(files, dirs) @files = files.map { |file| Pathname(file).expand_path }.to_set @@ -94,10 +86,14 @@ module ActiveSupport @mutex = Mutex.new start + @after_fork = ActiveSupport::ForkTracker.after_fork { start } end def finalizer - proc { stop } + proc do + stop + ActiveSupport::ForkTracker.unregister(@after_fork) + end end def thread_safely diff --git a/activesupport/test/evented_file_update_checker_test.rb b/activesupport/test/evented_file_update_checker_test.rb index 8132910818..963e7596a3 100644 --- a/activesupport/test/evented_file_update_checker_test.rb +++ b/activesupport/test/evented_file_update_checker_test.rb @@ -47,10 +47,6 @@ class EventedFileUpdateCheckerTest < ActiveSupport::TestCase touch_reader, touch_writer = IO.pipe pid = fork do - assert_predicate checker, :updated? - - # Clear previous check value. - checker.execute assert_not_predicate checker, :updated? # Fork is booted, ready for file to be touched