Minor timeout bug in Reactor

[wjordan]

There is a minor timeout bug in Reactor affecting unit-test stability, real-world impact is probably minimal.

To Reproduce

• Reduce first_data_timeout from 2 to 0.01 in test_timeout_in_data_phase (just to reproduce the issue quicker- the bug can still affect the existing test):

  puma/test/test_puma_server.rb

  Line 377 in 2ded685

  @server.first_data_timeout = 2

• Run the test repeatedly in a while loop and it will eventually hang:

  while :; do ruby -Ilib:test test/test_puma_server.rb --name /test_timeout_in_data_phase/; done

Analysis

In Reactor, the monitor of the Client object (c) is removed from @timeouts when the object becomes readable (has new data available to read):

puma/lib/puma/reactor.rb

Lines 214 to 221 in 19b2a21

	# We have to be sure to remove it from the timeout
	# list or we'll accidentally close the socket when
	# it's in use!
	if c.timeout_at
	@mutex.synchronize do
	@timeouts.delete mon
	end
	end

However, the object is only actually moved from the reactor to the thread-pool when c.try_to_finish is true (when the header / body is fully buffered):

puma/lib/puma/reactor.rb

Lines 224 to 227 in 19b2a21

	if c.try_to_finish
	@app_pool << c
	clear_monitor mon
	end

This leaves the edge-case where an object becomes readable but try_to_finish returns false, leaving the object in the reactor in a state where it will never time out.

[wjordan]

Fixed by #2279.

[dentarg]

Maybe this is what's happening for https://stackoverflow.com/questions/65290236/heroku-h18-error-in-a-rails-app-on-an-unhandled-route