fix: on altsvc processing, defer termination to when no more requests

a connection may be in the middle of processing multiple requests; once
an alt-svc header was received, while routing of pending data was done,
the current state of parser wasn't, but the connection has been
terminated and unlinked, leaving it in a state where errors may happen
if relying on attachment to a selector.

this fixes it by deferring termination to the point where the loop is
done with the current state of the connection; it does so by storing a
link to the altsvc connection.

lib/httpx/connection.rb

@@ -48,9 +48,9 @@ module HTTPX
 
     def initialize(uri, options)
       @current_session = @current_selector =
-        @parser = @sibling = @coalesced_connection =
-                    @family = @io = @ssl_session = @timeout =
-                                      @connected_at = @response_received_at = nil
+        @parser = @sibling = @coalesced_connection = @altsvc_connection =
+                               @family = @io = @ssl_session = @timeout =
+                                                 @connected_at = @response_received_at = nil
 
       @exhausted = @cloned = @main_sibling = false
 
@@ -86,10 +86,6 @@ module HTTPX
         @current_session.deselect_connection(self, @current_selector, @cloned)
       end
 
-      on(:altsvc) do |alt_origin, origin, alt_params|
-        build_altsvc_connection(alt_origin, origin, alt_params)
-      end
-
       self.addresses = @options.addresses if @options.addresses
     end
 
@@ -410,7 +406,11 @@ module HTTPX
           # * the number of pending requests
           # * whether the write buffer has bytes (i.e. for close handshake)
           if @pending.empty? && @inflight.zero? && @write_buffer.empty?
-            log(level: 3) { "NO MORE REQUESTS..." }
+            log(level: 3) { "NO MORE REQUESTS..." } if @parser && @parser.pending.any?
+
+            # terminate if an altsvc connection has been established
+            terminate if @altsvc_connection
+
             return
           end
 
@@ -455,7 +455,14 @@ module HTTPX
             break if @state == :closing || @state == :closed
 
             # exit #consume altogether if all outstanding requests have been dealt with
-            return if @pending.empty? && @inflight.zero?
+            if @pending.empty? && @inflight.zero? && @write_buffer.empty? # rubocop:disable Style/Next
+              log(level: 3) { "NO MORE REQUESTS..." } if @parser && @parser.pending.any?
+
+              # terminate if an altsvc connection has been established
+              terminate if @altsvc_connection
+
+              return
+            end
           end unless ((ints = interests).nil? || ints == :w || @state == :closing) && !epiped
 
           #
@@ -557,7 +564,7 @@ module HTTPX
     def set_parser_callbacks(parser)
       parser.on(:response) do |request, response|
         AltSvc.emit(request, response) do |alt_origin, origin, alt_params|
-          emit(:altsvc, alt_origin, origin, alt_params)
+          build_altsvc_connection(alt_origin, origin, alt_params)
         end
         @response_received_at = Utils.now
         @inflight -= 1
@@ -565,7 +572,7 @@ module HTTPX
         request.emit(:response, response)
       end
       parser.on(:altsvc) do |alt_origin, origin, alt_params|
-        emit(:altsvc, alt_origin, origin, alt_params)
+        build_altsvc_connection(alt_origin, origin, alt_params)
       end
 
       parser.on(:pong, &method(:send_pending))
@@ -791,6 +798,8 @@ module HTTPX
 
     # returns an HTTPX::Connection for the negotiated Alternative Service (or none).
     def build_altsvc_connection(alt_origin, origin, alt_params)
+      return if @altsvc_connection
+
       # do not allow security downgrades on altsvc negotiation
       return if @origin.scheme == "https" && alt_origin.scheme != "https"
 
@@ -808,10 +817,11 @@ module HTTPX
 
       connection.extend(AltSvc::ConnectionMixin) unless connection.is_a?(AltSvc::ConnectionMixin)
 
-      log(level: 1) { "#{origin} alt-svc: #{alt_origin}" }
+      @altsvc_connection = connection
+
+      log(level: 1) { "#{origin}: alt-svc connection##{connection.object_id} established to #{alt_origin}" }
 
       connection.merge(self)
-      terminate
     rescue UnsupportedSchemeError
       altsvc["noop"] = true
       nil

sig/connection.rbs

@@ -46,6 +46,7 @@ module HTTPX
     @exhausted: bool
     @cloned: bool
     @coalesced_connection: instance?
+    @altsvc_connection: instance?
     @sibling: instance?
     @main_sibling: bool
 

test/support/requests/altsvc.rb

@@ -8,12 +8,14 @@ module Requests
 
       HTTPX.plugin(SessionWithPool).wrap do |http|
         altsvc_uri = build_uri("/get", altsvc_origin)
-        response = http.get(altsvc_uri)
-        verify_status(response, 200)
-        verify_header(response.headers, "alt-svc", "h2=\"nghttp2:443\"")
-        response2 = http.get(altsvc_uri)
-        verify_status(response2, 200)
-        verify_no_header(response2.headers, "alt-svc")
+        res1, res2 = http.get(altsvc_uri, altsvc_uri)
+        verify_status(res1, 200)
+        verify_header(res1.headers, "alt-svc", "h2=\"nghttp2:443\"")
+        verify_status(res2, 200)
+        verify_header(res2.headers, "alt-svc", "h2=\"nghttp2:443\"")
+        res3 = http.get(altsvc_uri)
+        verify_status(res3, 200)
+        verify_no_header(res3.headers, "alt-svc")
         # introspection time
       end
     end