Directly support stateful executor hooks

Also, make sure to call the +complete+ hooks if +run+ fails.

3 files changed, 156 insertions, 4 deletions

diff --git a/activesupport/lib/active_support/execution_wrapper.rb b/activesupport/lib/active_support/execution_wrapper.rb
index 2bd1c01d35..00c5745a25 100644
--- a/activesupport/lib/active_support/execution_wrapper.rb
+++ b/activesupport/lib/active_support/execution_wrapper.rb
@@ -19,6 +19,32 @@ module ActiveSupport
       set_callback(:complete, *args, &block)
     end
 
+    # Register an object to be invoked during both the +run+ and
+    # +complete+ steps.
+    #
+    # +hook.complete+ will be passed the value returned from +hook.run+,
+    # and will only be invoked if +run+ has previously been called.
+    # (Mostly, this means it won't be invoked if an exception occurs in
+    # a preceding +to_run+ block; all ordinary +to_complete+ blocks are
+    # invoked in that situation.)
+    def self.register_hook(hook, outer: false)
+      if outer
+        run_args = [prepend: true]
+        complete_args = [:after]
+      else
+        run_args = complete_args = []
+      end
+
+      to_run(*run_args) do
+        hook_state[hook] = hook.run
+      end
+      to_complete(*complete_args) do
+        if hook_state.key?(hook)
+          hook.complete hook_state[hook]
+        end
+      end
+    end
+
     # Run this execution.
     #
     # Returns an instance, whose +complete!+ method *must* be invoked
@@ -29,7 +55,15 @@ module ActiveSupport
       if active?
         Null
       else
-        new.tap(&:run!)
+        new.tap do |instance|
+          success = nil
+          begin
+            instance.run!
+            success = true
+          ensure
+            instance.complete! unless success
+          end
+        end
       end
     end
 
@@ -37,11 +71,11 @@ module ActiveSupport
     def self.wrap
       return yield if active?
 
-      state = run!
+      instance = run!
       begin
         yield
       ensure
-        state.complete!
+        instance.complete!
       end
     end
 
@@ -74,5 +108,10 @@ module ActiveSupport
     ensure
       self.class.active.delete Thread.current
     end
+
+    private
+      def hook_state
+        @_hook_state ||= {}
+      end
   end
 end
diff --git a/activesupport/lib/active_support/reloader.rb b/activesupport/lib/active_support/reloader.rb
index 5d1f0e1e66..5623bdd349 100644
--- a/activesupport/lib/active_support/reloader.rb
+++ b/activesupport/lib/active_support/reloader.rb
@@ -43,7 +43,13 @@ module ActiveSupport
     # Initiate a manual reload
     def self.reload!
       executor.wrap do
-        new.tap(&:run!).complete!
+        new.tap do |instance|
+          begin
+            instance.run!
+          ensure
+            instance.complete!
+          end
+        end
       end
       prepare!
     end
diff --git a/activesupport/test/executor_test.rb b/activesupport/test/executor_test.rb
index 6db6db4fa8..d9b389461a 100644
--- a/activesupport/test/executor_test.rb
+++ b/activesupport/test/executor_test.rb
@@ -1,6 +1,9 @@
 require 'abstract_unit'
 
 class ExecutorTest < ActiveSupport::TestCase
+  class DummyError < RuntimeError
+  end
+
   def test_wrap_invokes_callbacks
     called = []
     executor.to_run { called << :run }
@@ -35,6 +38,20 @@ class ExecutorTest < ActiveSupport::TestCase
     assert_equal [:run, :body, :complete], called
   end
 
+  def test_exceptions_unwind
+    called = []
+    executor.to_run { called << :run_1 }
+    executor.to_run { raise DummyError }
+    executor.to_run { called << :run_2 }
+    executor.to_complete { called << :complete }
+
+    assert_raises(DummyError) do
+      executor.wrap { called << :body }
+    end
+
+    assert_equal [:run_1, :complete], called
+  end
+
   def test_avoids_double_wrapping
     called = []
     executor.to_run { called << :run }
@@ -51,6 +68,96 @@ class ExecutorTest < ActiveSupport::TestCase
     assert_equal [:run, :early, :body, :late, :complete], called
   end
 
+  def test_hooks_carry_state
+    supplied_state = :none
+
+    hook = Class.new do
+      define_method(:run) do
+        :some_state
+      end
+
+      define_method(:complete) do |state|
+        supplied_state = state
+      end
+    end.new
+
+    executor.register_hook(hook)
+
+    executor.wrap { }
+
+    assert_equal :some_state, supplied_state
+  end
+
+  def test_nil_state_is_sufficient
+    supplied_state = :none
+
+    hook = Class.new do
+      define_method(:run) do
+        nil
+      end
+
+      define_method(:complete) do |state|
+        supplied_state = state
+      end
+    end.new
+
+    executor.register_hook(hook)
+
+    executor.wrap { }
+
+    assert_equal nil, supplied_state
+  end
+
+  def test_exception_skips_uninvoked_hook
+    supplied_state = :none
+
+    hook = Class.new do
+      define_method(:run) do
+        :some_state
+      end
+
+      define_method(:complete) do |state|
+        supplied_state = state
+      end
+    end.new
+
+    executor.to_run do
+      raise DummyError
+    end
+    executor.register_hook(hook)
+
+    assert_raises(DummyError) do
+      executor.wrap { }
+    end
+
+    assert_equal :none, supplied_state
+  end
+
+  def test_exception_unwinds_invoked_hook
+    supplied_state = :none
+
+    hook = Class.new do
+      define_method(:run) do
+        :some_state
+      end
+
+      define_method(:complete) do |state|
+        supplied_state = state
+      end
+    end.new
+
+    executor.register_hook(hook)
+    executor.to_run do
+      raise DummyError
+    end
+
+    assert_raises(DummyError) do
+      executor.wrap { }
+    end
+
+    assert_equal :some_state, supplied_state
+  end
+
   def test_separate_classes_can_wrap
     other_executor = Class.new(ActiveSupport::Executor)