From 761fbafb8dc5a82f6dca9883f9f7af68c85b3649 Mon Sep 17 00:00:00 2001 From: Will Jordan Date: Wed, 7 Oct 2020 18:51:59 -0700 Subject: [PATCH] Test adding connection to Reactor after shutdown [changelog skip] (#2418) * Test adding connection to Reactor after shutdown Modifies `TestPumaServer#shutdown_requests` to pause `Reactor#add` until after shutdown begins, to ensure requests are handled correctly for this edge case. Adds unit-test coverage for the fix introduced in #2377 and updated in #2279. * Fix Queue#close implementation for Ruby 2.2 Allow `ClosedQueueError` to be raised when `Queue#<<` is called. * Pass `@block` directly instead of `@block.method(:call)` --- lib/puma/queue_close.rb | 1 + lib/puma/reactor.rb | 2 +- test/test_puma_server.rb | 18 +++++++++++++++++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/puma/queue_close.rb b/lib/puma/queue_close.rb index a9244b6722..2c354a2219 100644 --- a/lib/puma/queue_close.rb +++ b/lib/puma/queue_close.rb @@ -18,6 +18,7 @@ def push(object) raise ClosedQueueError if @closed super end + alias << push end Queue.prepend QueueClose end diff --git a/lib/puma/reactor.rb b/lib/puma/reactor.rb index a656d1f0a9..520cfc0e53 100644 --- a/lib/puma/reactor.rb +++ b/lib/puma/reactor.rb @@ -84,7 +84,7 @@ def select_loop retry end # Wakeup all remaining objects on shutdown. - @timeouts.each(&@block.method(:call)) + @timeouts.each(&@block) @selector.close end diff --git a/test/test_puma_server.rb b/test/test_puma_server.rb index 86d6b28b98..3588dc939e 100644 --- a/test/test_puma_server.rb +++ b/test/test_puma_server.rb @@ -972,6 +972,22 @@ def shutdown_requests(s1_complete: true, s1_response: nil, post: false, s2_respo [204, {}, []] } + pool = @server.instance_variable_get(:@thread_pool) + + # Trigger potential race condition by pausing Reactor#add until shutdown begins. + if options.fetch(:queue_requests, true) + reactor = @server.instance_variable_get(:@reactor) + reactor.instance_variable_set(:@pool, pool) + reactor.extend(Module.new do + def add(client) + if client.env['REQUEST_PATH'] == '/s2' + Thread.pass until @pool.instance_variable_get(:@shutdown) + end + super + end + end) + end + s1 = nil s2 = send_http post ? "POST /s2 HTTP/1.1\r\nHost: test.com\r\nContent-Type: text/plain\r\nContent-Length: 5\r\n\r\nhi!" : @@ -982,7 +998,7 @@ def shutdown_requests(s1_complete: true, s1_response: nil, post: false, s2_respo app_finished.signal if s1_complete end @server.stop - Thread.pass until @server.instance_variable_get(:@thread_pool).instance_variable_get(:@shutdown) + Thread.pass until pool.instance_variable_get(:@shutdown) assert_match(s1_response, s1.gets) if s1_response