diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 68cbe847d0b20..be8ec824d5708 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix ActionController::Live controller test deadlocks by removing the body buffer size limit for tests. + + *Dylan Thacker-Smith* + * Drop support for the `SERVER_ADDR` header Following up https://github.com/rack/rack/pull/1573 and https://github.com/rails/rails/pull/42349 diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 5c684ba173cf3..ca7d3d6382b2e 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -127,6 +127,11 @@ class ClientDisconnected < RuntimeError class Buffer < ActionDispatch::Response::Buffer #:nodoc: include MonitorMixin + class << self + attr_accessor :queue_size + end + @queue_size = 10 + # Ignore that the client has disconnected. # # If this value is `true`, calling `write` after the client @@ -136,7 +141,7 @@ class Buffer < ActionDispatch::Response::Buffer #:nodoc: attr_accessor :ignore_disconnect def initialize(response) - super(response, SizedQueue.new(10)) + super(response, build_queue(self.class.queue_size)) @error_callback = lambda { true } @cv = new_cond @aborted = false @@ -219,6 +224,10 @@ def each_chunk(&block) yield str end end + + def build_queue(queue_size) + queue_size ? SizedQueue.new(queue_size) : Queue.new + end end class Response < ActionDispatch::Response #:nodoc: all diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 7c9221e2cef96..c4ff30277eb2a 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -24,6 +24,9 @@ module Live def new_controller_thread # :nodoc: yield end + + # Avoid a deadlock from the queue filling up + Buffer.queue_size = nil end # ActionController::TestCase will be deprecated and moved to a gem in the future. diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 6d008144ae5f7..2f7606255c283 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -264,6 +264,13 @@ def overfill_buffer_and_die end end + def overfill_default_buffer + ("a".."z").each do |char| + response.stream.write(char) + end + response.stream.close + end + def ignore_client_disconnect response.stream.ignore_disconnect = true @@ -368,7 +375,15 @@ def test_async_stream assert t.join(3), "timeout expired before the thread terminated" end + def test_infinite_test_buffer + get :overfill_default_buffer + assert_equal ("a".."z").to_a.join, response.stream.body + end + def test_abort_with_full_buffer + old_queue_size = ActionController::Live::Buffer.queue_size + ActionController::Live::Buffer.queue_size = 10 + @controller.latch = Concurrent::CountDownLatch.new @controller.error_latch = Concurrent::CountDownLatch.new @@ -389,6 +404,8 @@ def test_abort_with_full_buffer @controller.error_latch.wait assert_match "Error while streaming", output.rewind && output.read end + ensure + ActionController::Live::Buffer.queue_size = old_queue_size end def test_ignore_client_disconnect