From 00ef607326d959e037454d0c809ff3c2d2280ecc Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Fri, 30 Aug 2024 13:25:26 +0200 Subject: [PATCH] url: fix connection reuse for HTTP/2 upgrades Normally, when a connection's filters have all connected, the multiplex status is determined. However, HTTP/2 Upgrade: requests will only do this when the first server response has been received. The current connection reuse mechanism does not accomodate that and when the time between connect and response is large enough, connection reuse may not happen as desired. See test case 2405 failures, such as in https://github.com/curl/curl/actions/runs/10629497461/job/29467166451 Add 'conn->bits.asks_multiplex' as indicator that a connection is still being evaluated for mulitplexing, so that new transfers may wait on this to be cleared. Closes #14739 --- lib/http.c | 2 ++ lib/http2.c | 1 + lib/url.c | 5 ++++- lib/urldata.h | 1 + 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/http.c b/lib/http.c index e748aa55b0..c5accd0f93 100644 --- a/lib/http.c +++ b/lib/http.c @@ -3439,6 +3439,7 @@ static CURLcode http_on_response(struct Curl_easy *data, /* Switching to HTTP/2, where we will get more responses */ infof(data, "Received 101, Switching to HTTP/2"); k->upgr101 = UPGR101_RECEIVED; + data->conn->bits.asks_multiplex = FALSE; /* We expect more response from HTTP/2 later */ k->header = TRUE; k->headerline = 0; /* restart the header line counter */ @@ -3485,6 +3486,7 @@ static CURLcode http_on_response(struct Curl_easy *data, if(k->upgr101 == UPGR101_H2) { /* A requested upgrade was denied, poke the multi handle to possibly allow a pending pipewait to continue */ + data->conn->bits.asks_multiplex = FALSE; Curl_multi_connchanged(data->multi); } diff --git a/lib/http2.c b/lib/http2.c index fdc8136acf..90ee636230 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -1736,6 +1736,7 @@ CURLcode Curl_http2_request_upgrade(struct dynbuf *req, free(base64); k->upgr101 = UPGR101_H2; + data->conn->bits.asks_multiplex = TRUE; return result; } diff --git a/lib/url.c b/lib/url.c index 88d3ecc9d3..4794bac8f8 100644 --- a/lib/url.c +++ b/lib/url.c @@ -907,7 +907,10 @@ static bool url_match_conn(struct connectdata *conn, void *userdata) * connections that do not use this feature */ return FALSE; - if(!Curl_conn_is_connected(conn, FIRSTSOCKET)) { + if(!Curl_conn_is_connected(conn, FIRSTSOCKET) || + conn->bits.asks_multiplex) { + /* Not yet connected, or not yet decided if it multiplexes. The later + * happens for HTTP/2 Upgrade: requests that need a response. */ if(match->may_multiplex) { match->seen_pending_conn = TRUE; /* Do not pick a connection that has not connected yet */ diff --git a/lib/urldata.h b/lib/urldata.h index 2a282beb66..48f8a3f70c 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -535,6 +535,7 @@ struct ConnectBits { #endif BIT(bound); /* set true if bind() has already been done on this socket/ connection */ + BIT(asks_multiplex); /* connection asks for multiplexing, but is not yet */ BIT(multiplex); /* connection is multiplexed */ BIT(tcp_fastopen); /* use TCP Fast Open */ BIT(tls_enable_alpn); /* TLS ALPN extension? */