Can no longer force stop an HTTP server task

[zarqman]

Since #153, it seems that task.stop can no longer forcefully stop an HTTP server task.

In testing, consider something like this:

### async-http enabled app
server = Async::HTTP::Server.new(...)
server_task = Async{ server.run }

# make a request to the server's endpoint, which in turn talks to a remote origin
# that remote origin uses http1.1 with the default/implied Connection: keep-alive

server_task.stop
# this hangs until the keep-alive connection to origin finally times out, which could be awhile

In practice, this is causing lengthy timeouts in the test suite here (jumped from around 1 minute to over an hour). The delays are coming during requests to http1.1 origins. Reverting task.defer_stop to begin in Async::HTTP::Protocol::HTTP1::Server#each resolves the issue. On the surface, the comparable change in ...HTTP2::Server doesn't seem to be as much of a problem.

While at least part of it is in our tests, and could potentially be worked around, I'm also concerned about stuck tasks (even if due to other bugs) and that there won't be a way to kill them in a timely fashion apart from kill -9.

[ioquatix]

Thanks for the report. This feature is kind of experimental, so this is helpful feedback. I'll investigate it today. In theory, #stop should work, it just shouldn't interrupt the current response until it's fully sent.

Sending #stop a 2nd time will bypass #defer_stop. However, I admit, this feels a bit arbitrary.

We also have Async::Task#terminate. In theory it should work (sending stop until the task is dead). However, I just tested it and it's not working as expected. Maybe as an alternative we could introduce stop! which bypasses defer_stop. But it also feels a bit arbitrary.

[ioquatix]

diff --git a/lib/async/http/protocol/http1/server.rb b/lib/async/http/protocol/http1/server.rb
index 901d25c..55800f2 100644
--- a/lib/async/http/protocol/http1/server.rb
+++ b/lib/async/http/protocol/http1/server.rb
@@ -99,14 +99,14 @@ module Async
                                                                
                                                                # Gracefully finish reading the request body if it was not already done so.
                                                                request&.each{}
-                                                               
-                                                               # This ensures we yield at least once every iteration of the loop and allow other fibers to execute.
-                                                               task.yield
                                                        rescue => error
                                                                raise
                                                        ensure
                                                                body&.close(error)
                                                        end
+                                                       
+                                                       # This ensures we yield at least once every iteration of the loop and allow other fibers to execute.
+                                                       task.yield
                                                end
                                        end
                                        

Do you mind checking to see if this fixes your issue?

[ioquatix]

(alternatively can you open a PR with a failing test case?)

[zarqman]

Thanks for the quick reply. I tested the potential patch for http1/server.rb and unfortunately it did not help (or seem to change anything).

I'll work on some kind of repro so we have something more concrete to work with.

[ioquatix]

Thanks that would be awesome, in the mean time, I'm going to work on #155 - this feature was a bit rushed to try and get something out for testing in falcon. But now I see there are a couple of tricky issues that remain to be solved.

[zarqman]

Here's a repro example. This appears to be an issue with graceful stop not actually stopping the server when it's idle and waiting for a keep-alive request, as somewhat discussed in socketry/falcon#71.

# examples/hanging_stop.rb

gem 'async', '2.10.1'
## toggle between old and new behavior here:
gem 'async-http', '0.63.0'
# gem 'async-http', '0.64.0'

require 'async'
require 'async/http/client'
require 'async/http/endpoint'
require 'async/http/server'
require 'async/http/version'

URL = 'http://127.0.0.1:8080'

puts "async: #{Async::VERSION}"
puts "async-http: #{Async::HTTP::VERSION}"

Async do |top_task|

  ## server start

  # timeout would be a normal keepalive duration
  #   these are often 55-75sec, but have seen up to 900sec in the wild
  endpoint = Async::HTTP::Endpoint.parse URL, timeout: 11, protocol: Async::HTTP::Protocol::HTTP11

  app = proc do |request|
    puts '[server] processing request'
    Protocol::HTTP::Response[200, {}, ['body']]
  end

  server = Async::HTTP::Server.new(app, endpoint)
  server_task = Async do
    puts '[server] starting'
    server.run
  end


  ## client request

  ep = Async::HTTP::Endpoint.parse URL, timeout: 7, protocol: Async::HTTP::Protocol::HTTP11

  client = Async::HTTP::Client.new ep, retries: 0
  request = Protocol::HTTP::Request['GET', '/']

  puts '[client] sending request'
  response = client.get('/')

  puts "[client] got responses, status=#{response.status}"

  body = response.finish
  # pp body: body
  puts client.pool

  # at this point, the request is done and the client remains connected for a possible 2nd request via keep-alive.
  # likewise, the server is waiting in `read_request_line` for a potential 2nd request.


  ## server shutdown

  puts 'stopping server'
  server_task.stop

ensure
  puts "waiting for top task to end @ #{Time.now}"
end
# should exit promptly and without an Async::TimeoutError
# when it delays, it's the same 11 seconds as the server's :timeout at the top

puts "exiting @ #{Time.now}"