diff --git a/lib/puma/minissl.rb b/lib/puma/minissl.rb index b33c5aaf..aa725af7 100644 --- a/lib/puma/minissl.rb +++ b/lib/puma/minissl.rb @@ -54,22 +54,21 @@ module Puma output = engine_read_all return output if output - begin - data = @socket.read_nonblock(size, exception: false) - if data == :wait_readable || data == :wait_writable - if @socket.to_io.respond_to?(data) - @socket.to_io.__send__(data) - elsif data == :wait_readable - IO.select([@socket.to_io]) - else - IO.select(nil, [@socket.to_io]) - end - elsif !data - return nil - else - break - end - end while true + data = @socket.read_nonblock(size, exception: false) + if data == :wait_readable || data == :wait_writable + # It would make more sense to let @socket.read_nonblock raise + # EAGAIN if necessary but it seems like it'll misbehave on Windows. + # I don't have a Windows machine to debug this so I can't explain + # exactly whats happening in that OS. Please let me know if you + # find out! + # + # In the meantime, we can emulate the correct behavior by + # capturing :wait_readable & :wait_writable and raising EAGAIN + # ourselves. + raise IO::EAGAINWaitReadable + elsif data.nil? + return nil + end @engine.inject(data) output = engine_read_all diff --git a/test/test_puma_server_ssl.rb b/test/test_puma_server_ssl.rb index 134075bf..090fc107 100644 --- a/test/test_puma_server_ssl.rb +++ b/test/test_puma_server_ssl.rb @@ -34,8 +34,8 @@ class TestPumaServerSSL < Minitest::Test def setup return if DISABLE_SSL - port = UniquePort.call - host = "127.0.0.1" + @port = UniquePort.call + @host = "127.0.0.1" app = lambda { |env| [200, {}, [env['rack.url_scheme']]] } @@ -53,10 +53,10 @@ class TestPumaServerSSL < Minitest::Test @events = SSLEventsHelper.new STDOUT, STDERR @server = Puma::Server.new app, @events - @ssl_listener = @server.add_ssl_listener host, port, ctx + @ssl_listener = @server.add_ssl_listener @host, @port, ctx @server.run - @http = Net::HTTP.new host, port + @http = Net::HTTP.new @host, @port @http.use_ssl = true @http.verify_mode = OpenSSL::SSL::VERIFY_NONE end @@ -80,6 +80,25 @@ class TestPumaServerSSL < Minitest::Test assert_equal "https", body end + def test_request_wont_block_thread + # Open a connection and give enough data to trigger a read, then wait + ctx = OpenSSL::SSL::SSLContext.new + ctx.verify_mode = OpenSSL::SSL::VERIFY_NONE + socket = OpenSSL::SSL::SSLSocket.new TCPSocket.new(@host, @port), ctx + socket.write "x" + sleep 0.1 + + # Capture the amount of threads being used after connecting and being idle + thread_pool = @server.instance_variable_get(:@thread_pool) + busy_threads = thread_pool.spawned - thread_pool.waiting + + socket.close + + # The thread pool should be empty since the request would block on read + # and our request should have been moved to the reactor. + assert busy_threads.zero?, "Our connection is monopolizing a thread" + end + def test_very_large_return giant = "x" * 2056610