Merge pull request #44499 from Shopify/ac-live-thread-copy

Copy over the IsolatedExecutionState in AC::Live

actionpack/CHANGELOG.md

@@ -1,3 +1,13 @@
+*   Fix `ActionContoller::Live` to copy the IsolatedExecutionState in the ephemeral thread.
+
+    Since it's inception `ActionContoller::Live` has been copying thread local variables
+    to keep things such as `CurrentAttributes` set from middlewares working in the controller action.
+
+    With the introduction of `IsolatedExecutionState` in 7.0, some of that global state was lost in
+    `ActionContoller::Live` controllers.
+
+    *Jean Boussier*
+
 *   Fix setting `trailing_slash: true` in route definition.
 
     ```ruby

actionpack/lib/action_controller/metal/live.rb

@@ -261,6 +261,7 @@ def process(name)
           # Since we're processing the view in a different thread, copy the
           # thread locals from the main thread to the child thread. :'(
           locals.each { |k, v| t2[k] = v }
+          ActiveSupport::IsolatedExecutionState.share_with(t1)
 
           begin
             super(name)

actionpack/test/controller/live_stream_test.rb

@@ -114,6 +114,10 @@ class LiveStreamTest < ActionController::TestCase
     class Exception < StandardError
     end
 
+    class CurrentState < ActiveSupport::CurrentAttributes
+      attribute :id
+    end
+
     class TestController < ActionController::Base
       include ActionController::Live
 
@@ -203,6 +207,10 @@ def thread_locals
         response.stream.close
       end
 
+      def isolated_state
+        render plain: CurrentState.id.inspect
+      end
+
       def with_stale
         render plain: "stale" if stale?(etag: "123", template: false)
       end
@@ -445,6 +453,15 @@ def test_thread_locals_get_copied
       get :thread_locals
     end
 
+    def test_isolated_state_get_copied
+      @controller.tc = self
+      CurrentState.id = "isolated_state"
+
+      get :isolated_state
+      assert_equal "isolated_state".inspect, response.body
+      assert_stream_closed
+    end
+
     def test_live_stream_default_header
       get :default_header
       assert response.headers["Content-Type"]

activesupport/lib/active_support/isolated_execution_state.rb

@@ -58,6 +58,13 @@ def context
         scope.current
       end
 
+      def share_with(other)
+        # Action Controller streaming spawns a new thread and copy thread locals.
+        # We do the same here for backward compatibility, but this is very much a hack
+        # and streaming should be rethought.
+        context.active_support_execution_state = other.active_support_execution_state.dup
+      end
+
       private
         def state
           context.active_support_execution_state ||= {}