From 285d7cad6a4b93cdeea469e6cc4504a934907746 Mon Sep 17 00:00:00 2001 From: Alex Chinn Date: Mon, 15 Aug 2016 17:08:46 -0400 Subject: [PATCH] Fix deadlock that can occur when child live thread tries to load a constant after writing to the stream. --- .../lib/action_controller/metal/live.rb | 7 +++++- .../test/controller/live_stream_test.rb | 22 +++++++++++++++++++ actionpack/test/fixtures/load_me.rb | 2 ++ 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 actionpack/test/fixtures/load_me.rb diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index a18055c899..26a16104db 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -205,7 +205,12 @@ module ActionController private def each_chunk(&block) - while str = @buf.pop + loop do + str = nil + ActiveSupport::Dependencies.interlock.permit_concurrent_loads do + str = @buf.pop + end + break unless str yield str end end diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 3ea02f0a19..9ec37e7559 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -150,6 +150,20 @@ module ActionController response.stream.close end + def write_sleep_autoload + path = File.join(File.dirname(__FILE__), "../fixtures") + ActiveSupport::Dependencies.autoload_paths << path + + response.headers["Content-Type"] = "text/event-stream" + response.stream.write "before load" + sleep 0.01 + ::LoadMe + response.stream.close + latch.count_down + + ActiveSupport::Dependencies.autoload_paths.reject! {|p| p == path} + end + def thread_locals tc.assert_equal "aaron", Thread.current[:setting] @@ -281,6 +295,14 @@ module ActionController assert_equal "text/event-stream", @response.headers["Content-Type"] end + def test_delayed_autoload_after_write_within_interlock_hook + # Simulate InterlockHook + ActiveSupport::Dependencies.interlock.start_running + res = get :write_sleep_autoload + res.each {} + ActiveSupport::Dependencies.interlock.done_running + end + def test_async_stream rubinius_skip "https://github.com/rubinius/rubinius/issues/2934" diff --git a/actionpack/test/fixtures/load_me.rb b/actionpack/test/fixtures/load_me.rb new file mode 100644 index 0000000000..e516512a4e --- /dev/null +++ b/actionpack/test/fixtures/load_me.rb @@ -0,0 +1,2 @@ +class LoadMe +end