Skip to content

Commit

Permalink
Merge pull request #41609
Browse files Browse the repository at this point in the history
  • Loading branch information
rafaelfranca committed Jun 23, 2021
1 parent fab5a81 commit 5b4466d
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 1 deletion.
4 changes: 4 additions & 0 deletions 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*

* Add support for 'private, no-store' Cache-Control headers.

Previously, 'no-store' was exclusive; no other directives could be specified.
Expand Down
11 changes: 10 additions & 1 deletion actionpack/lib/action_controller/metal/live.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -214,6 +219,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
Expand Down
3 changes: 3 additions & 0 deletions actionpack/lib/action_controller/test_case.rb
Expand Up @@ -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.
Expand Down
17 changes: 17 additions & 0 deletions actionpack/test/controller/live_stream_test.rb
Expand Up @@ -246,6 +246,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

Expand Down Expand Up @@ -329,7 +336,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

Expand All @@ -350,6 +365,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
Expand Down

0 comments on commit 5b4466d

Please sign in to comment.