From ebd915043e2b09ce4314137faf961126816fb70b Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Thu, 8 Feb 2018 13:27:23 -0500 Subject: [PATCH 1/2] Use infinite sized queue in rack test context to avoid deadlock situation when data is never consumed from the Action::Dispatch::Response::Buffer. --- actionpack/lib/action_controller/metal/live.rb | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 2f4c8fb83cd07..6c4c09438b879 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -141,7 +141,21 @@ def initialize(response) @cv = new_cond @aborted = false @ignore_disconnect = false - super(response, SizedQueue.new(10)) + + # https://github.com/rails/rails/issues/31813: + # + # In a browser context, the browser consumes data as soon as it's + # generated, freeing up space in the SizedQueue. However, in a rack + # test context, this is not the case, as data is never consumed. This + # leads to a deadlock upon trying to insert the 11th element in the + # queue, since the queue is waiting for space to become available + # before inserting the element: + # + # http://ruby-doc.org/stdlib-2.0.0/libdoc/thread/rdoc/SizedQueue.html#method-i-push + # + # Thus, use an infinite sized Queue in rack test contexts to work around + # the issue where data is never consumed from the queue. + super(response, Rails.env.test? ? Queue.new : SizedQueue.new(10)) end def write(string) From cfd64d8079eb00a6fb4451cb7674bd927c3c91e1 Mon Sep 17 00:00:00 2001 From: AndrewMcBurney Date: Fri, 9 Feb 2018 09:52:36 -0500 Subject: [PATCH 2/2] Updated test coupled to queue length in `actionpack/test/controller/live_stream_test.rb`, and removed superfluous comment in `actionpack/lib/action_controller/metal/live.rb`. --- .../lib/action_controller/metal/live.rb | 14 ------- .../test/controller/live_stream_test.rb | 40 ++++++++++--------- 2 files changed, 21 insertions(+), 33 deletions(-) diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 6c4c09438b879..6d89b9d49abc7 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -141,20 +141,6 @@ def initialize(response) @cv = new_cond @aborted = false @ignore_disconnect = false - - # https://github.com/rails/rails/issues/31813: - # - # In a browser context, the browser consumes data as soon as it's - # generated, freeing up space in the SizedQueue. However, in a rack - # test context, this is not the case, as data is never consumed. This - # leads to a deadlock upon trying to insert the 11th element in the - # queue, since the queue is waiting for space to become available - # before inserting the element: - # - # http://ruby-doc.org/stdlib-2.0.0/libdoc/thread/rdoc/SizedQueue.html#method-i-push - # - # Thus, use an infinite sized Queue in rack test contexts to work around - # the issue where data is never consumed from the queue. super(response, Rails.env.test? ? Queue.new : SizedQueue.new(10)) end diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 431fe90b23a24..6811fd3a36393 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -330,25 +330,27 @@ def test_async_stream end def test_abort_with_full_buffer - @controller.latch = Concurrent::CountDownLatch.new - @controller.error_latch = Concurrent::CountDownLatch.new - - capture_log_output do |output| - get :overfill_buffer_and_die, format: "plain" - - t = Thread.new(response) { |resp| - resp.await_commit - _, _, body = resp.to_a - body.each do - @controller.latch.wait - body.close - break - end - } - - t.join - @controller.error_latch.wait - assert_match "Error while streaming", output.rewind && output.read + Rails.stub(:env, ActiveSupport::StringInquirer.new("production")) do + @controller.latch = Concurrent::CountDownLatch.new + @controller.error_latch = Concurrent::CountDownLatch.new + + capture_log_output do |output| + get :overfill_buffer_and_die, format: "plain" + + t = Thread.new(response) { |resp| + resp.await_commit + _, _, body = resp.to_a + body.each do + @controller.latch.wait + body.close + break + end + } + + t.join + @controller.error_latch.wait + assert_match "Error while streaming", output.rewind && output.read + end end end