From e093c67f1cc3fbedefc49fd2e3f9520afcfa769f Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Thu, 23 Jul 2026 14:17:13 +0200 Subject: [PATCH] connect: connection close tweaks - connclose/streamclose/connkeep() remove description string that was never used anywhere. Add trace statements where reasons for closing were not already traced and maybe not obvious. - multi_remove_handle: only lookup former connection in pool when transfer is set to connect only - test1554: adapt expectations now that pool is less often locked Closes #22379 --- lib/cfilters.c | 6 +- lib/conncache.c | 34 ----------- lib/conncache.h | 8 --- lib/connect.c | 50 ++++++++-------- lib/connect.h | 42 +++++--------- lib/easy.c | 8 +-- lib/ftp.c | 17 +++--- lib/gopher.c | 2 +- lib/http.c | 35 ++++++------ lib/http2.c | 10 ++-- lib/imap.c | 3 +- lib/ldap.c | 2 +- lib/mqtt.c | 2 +- lib/multi.c | 110 ++++++++++++++++-------------------- lib/pop3.c | 3 +- lib/sendf.c | 4 +- lib/smb.c | 24 +++++--- lib/smtp.c | 3 +- lib/tftp.c | 2 +- lib/transfer.c | 2 +- lib/vquic/cf-ngtcp2-proxy.c | 2 +- lib/vquic/cf-ngtcp2.c | 2 +- lib/vquic/cf-quiche.c | 2 +- lib/vssh/libssh.c | 2 +- lib/vssh/libssh2.c | 2 +- tests/data/test1554 | 6 -- 26 files changed, 157 insertions(+), 226 deletions(-) diff --git a/lib/cfilters.c b/lib/cfilters.c index 238698d332..bef39e6f5b 100644 --- a/lib/cfilters.c +++ b/lib/cfilters.c @@ -600,11 +600,9 @@ bool Curl_conn_is_multiplex(struct connectdata *conn, int sockindex) { struct Curl_cfilter *cf; - if(!CONN_SOCK_IDX_VALID(sockindex)) + if(!conn || !CONN_SOCK_IDX_VALID(sockindex)) return FALSE; - cf = conn ? conn->cfilter[sockindex] : NULL; - - for(; cf; cf = cf->next) { + for(cf = conn->cfilter[sockindex]; cf; cf = cf->next) { if(cf->cft->flags & CF_TYPE_MULTIPLEX) return TRUE; if(cf->cft->flags & (CF_TYPE_IP_CONNECT | CF_TYPE_SSL)) diff --git a/lib/conncache.c b/lib/conncache.c index 17e8d715b0..eb88d67dc8 100644 --- a/lib/conncache.c +++ b/lib/conncache.c @@ -811,40 +811,6 @@ struct connectdata *Curl_cpool_get_conn(struct Curl_easy *data, return fctx.conn; } -struct cpool_do_conn_ctx { - curl_off_t id; - Curl_cpool_conn_do_cb *cb; - void *cbdata; -}; - -static int cpool_do_conn(struct Curl_easy *data, - struct connectdata *conn, void *param) -{ - struct cpool_do_conn_ctx *dctx = param; - - if(conn->connection_id == dctx->id) { - dctx->cb(conn, data, dctx->cbdata); - return 1; - } - return 0; -} - -void Curl_cpool_do_by_id(struct Curl_easy *data, curl_off_t conn_id, - Curl_cpool_conn_do_cb *cb, void *cbdata) -{ - struct cpool *cpool = cpool_get_instance(data); - struct cpool_do_conn_ctx dctx; - - if(!cpool) - return; - dctx.id = conn_id; - dctx.cb = cb; - dctx.cbdata = cbdata; - CPOOL_LOCK(cpool, data); - cpool_foreach(data, cpool, &dctx, cpool_do_conn); - CPOOL_UNLOCK(cpool, data); -} - void Curl_cpool_do_locked(struct Curl_easy *data, struct connectdata *conn, Curl_cpool_conn_do_cb *cb, void *cbdata) diff --git a/lib/conncache.h b/lib/conncache.h index 71940d724c..78ccd8282f 100644 --- a/lib/conncache.h +++ b/lib/conncache.h @@ -143,14 +143,6 @@ typedef void Curl_cpool_conn_do_cb(struct connectdata *conn, struct Curl_easy *data, void *cbdata); -/** - * Invoke the callback on the pool's connection with the - * given connection id (if it exists). - */ -void Curl_cpool_do_by_id(struct Curl_easy *data, - curl_off_t conn_id, - Curl_cpool_conn_do_cb *cb, void *cbdata); - /** * Invoked the callback for the given data + connection under the * connection pool's lock. diff --git a/lib/connect.c b/lib/connect.c index ca81849ce7..c220475389 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -192,43 +192,41 @@ curl_socket_t Curl_getconnectinfo(struct Curl_easy *data, conn = Curl_cpool_get_conn(data, data->state.lastconnect_id); if(!conn) { data->state.lastconnect_id = -1; + if(connp) + *connp = NULL; return CURL_SOCKET_BAD; } if(connp) - /* only store this if the caller cares for it */ *connp = conn; return conn->sock[FIRSTSOCKET]; } + if(connp) + *connp = NULL; return CURL_SOCKET_BAD; } -/* - * Curl_conncontrol() marks streams or connection for closure. - */ -void Curl_conncontrol(struct connectdata *conn, - int ctrl /* see defines in header */ -#if defined(DEBUGBUILD) && defined(CURLVERBOSE) - , const char *reason -#endif - ) +void Curl_conncontrol(struct connectdata *conn, int ctrl) { - /* close if a connection, or a stream that is not multiplexed. */ - /* This function will be called both before and after this connection is - associated with a transfer. */ - bool closeit, is_multiplex; - DEBUGASSERT(conn); -#if defined(DEBUGBUILD) && defined(CURLVERBOSE) - (void)reason; /* useful for debugging */ -#endif - is_multiplex = Curl_conn_is_multiplex(conn, FIRSTSOCKET); - closeit = (ctrl == CONNCTRL_CONNECTION) || - ((ctrl == CONNCTRL_STREAM) && !is_multiplex); - if((ctrl == CONNCTRL_STREAM) && is_multiplex) - ; /* stream signal on multiplex conn never affects close state */ - else if((curl_bit)closeit != conn->bits.close) { - conn->bits.close = closeit; /* the only place in the source code that - should assign this bit */ + if(!conn) { + DEBUGASSERT(0); + return; + } + switch(ctrl) { + case CONNCTRL_CONN_KEEP: + conn->bits.close = FALSE; + break; + case CONNCTRL_CONN_CLOSE: + conn->bits.close = TRUE; + break; + case CONNCTRL_STREAM_CLOSE: + /* stream close when multiplexing does not affect connection */ + if(!Curl_conn_is_multiplex(conn, FIRSTSOCKET)) + conn->bits.close = TRUE; + break; + default: + DEBUGASSERT(0); + break; } } diff --git a/lib/connect.h b/lib/connect.h index 20eedaf10e..91cd924901 100644 --- a/lib/connect.h +++ b/lib/connect.h @@ -70,37 +70,23 @@ curl_socket_t Curl_getconnectinfo(struct Curl_easy *data, struct connectdata **connp); /* - * Curl_conncontrol() marks the end of a connection/stream. The 'ctrl' - * argument specifies if it is the end of a connection or a stream. - * - * For stream-based protocols (such as HTTP/2), a stream close will not cause - * a connection close. Other protocols will close the connection for both - * cases. - * - * It sets the bit.close bit to TRUE (with an explanation for debug builds), - * when the connection will close. + * Curl_conncontrol() manipulates the `conn->bits.close` bit on + * a connection: + * - CONNCTRL_CONN_KEEP: clear the bit + * - CONNCTRL_CONN_CLOSE: set the bit + * - CONNCTRL_STREAM_CLOSE: set the bit when the connection is not + * multiplexed + * The call does *NOT* cause any immediate connection close. */ +#define CONNCTRL_CONN_KEEP 0 +#define CONNCTRL_CONN_CLOSE 1 +#define CONNCTRL_STREAM_CLOSE 2 -#define CONNCTRL_KEEP 0 /* undo a marked closure */ -#define CONNCTRL_CONNECTION 1 -#define CONNCTRL_STREAM 2 +void Curl_conncontrol(struct connectdata *conn, int ctrl); -void Curl_conncontrol(struct connectdata *conn, - int ctrl -#if defined(DEBUGBUILD) && defined(CURLVERBOSE) - , const char *reason -#endif - ); - -#if defined(DEBUGBUILD) && defined(CURLVERBOSE) -#define streamclose(x, y) Curl_conncontrol(x, CONNCTRL_STREAM, y) -#define connclose(x, y) Curl_conncontrol(x, CONNCTRL_CONNECTION, y) -#define connkeep(x, y) Curl_conncontrol(x, CONNCTRL_KEEP, y) -#else /* !DEBUGBUILD || !CURLVERBOSE */ -#define streamclose(x, y) Curl_conncontrol(x, CONNCTRL_STREAM) -#define connclose(x, y) Curl_conncontrol(x, CONNCTRL_CONNECTION) -#define connkeep(x, y) Curl_conncontrol(x, CONNCTRL_KEEP) -#endif +#define streamclose(x) Curl_conncontrol((x), CONNCTRL_STREAM_CLOSE) +#define connclose(x) Curl_conncontrol((x), CONNCTRL_CONN_CLOSE) +#define connkeep(x) Curl_conncontrol((x), CONNCTRL_CONN_KEEP) /** * Setup the cfilters at `sockindex` in connection `conn`. diff --git a/lib/easy.c b/lib/easy.c index 7321e73878..88b4c194c2 100644 --- a/lib/easy.c +++ b/lib/easy.c @@ -758,13 +758,9 @@ static CURLcode easy_perform(struct Curl_easy *data, bool events) /* if the handle has a connection still attached (it is/was a connect-only handle) then disconnect before performing */ if(data->conn) { - struct connectdata *c; - curl_socket_t s; + struct connectdata *conn = data->conn; Curl_detach_connection(data); - s = Curl_getconnectinfo(data, &c); - if((s != CURL_SOCKET_BAD) && c) { - Curl_conn_terminate(data, c, TRUE); - } + Curl_conn_terminate(data, conn, TRUE); DEBUGASSERT(!data->conn); } diff --git a/lib/ftp.c b/lib/ftp.c index c5725c48ec..ff9fed98af 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -3610,7 +3610,7 @@ static CURLcode ftp_sendquote(struct Curl_easy *data, return CURLE_OK; } -static CURLcode ftp_done_status(struct connectdata *conn, +static CURLcode ftp_done_status(struct Curl_easy *data, struct ftp_conn *ftpc, CURLcode status, bool premature) { @@ -3641,7 +3641,8 @@ static CURLcode ftp_done_status(struct connectdata *conn, ftpc->ctl_valid = FALSE; ftpc->cwdfail = TRUE; /* set this TRUE to prevent us to remember the current path, as this connection is going */ - connclose(conn, "FTP ended with bad error code"); + CURL_TRC_FTP(data, "FTP ended with bad error code"); + connclose(data->conn); return status; /* use the already set error code */ } return CURLE_OK; @@ -3669,7 +3670,7 @@ static void ftp_done_path(struct Curl_easy *data, struct ftp_conn *ftpc, /* We can limp along anyway (and should try to since we may already be in * the error path) */ ftpc->ctl_valid = FALSE; /* mark control connection as bad */ - connclose(conn, "FTP: out of memory!"); /* mark for connection closure */ + connclose(conn); /* mark for connection closure */ curlx_safefree(ftpc->prevpath); /* no path remembering */ } else { /* remember working directory for connection reuse */ @@ -3712,7 +3713,7 @@ static CURLcode ftp_done_secondary_socket(struct Curl_easy *data, failf(data, "Failure sending ABOR command: %s", curl_easy_strerror(result)); ftpc->ctl_valid = FALSE; /* mark control connection as bad */ - connclose(conn, "ABOR command failed"); /* connection closure */ + connclose(conn); /* connection closure */ } } @@ -3744,7 +3745,7 @@ static CURLcode ftp_done_control_reply(struct Curl_easy *data, if(!nread && (result == CURLE_OPERATION_TIMEDOUT)) { failf(data, "control connection looks dead"); ftpc->ctl_valid = FALSE; /* mark control connection as bad */ - connclose(conn, "Timeout or similar in FTP DONE operation"); /* close */ + connclose(conn); /* close */ } if(result) @@ -3754,7 +3755,7 @@ static CURLcode ftp_done_control_reply(struct Curl_easy *data, /* we have sent ABOR and there is no reliable way to check if it was * successful or not; we have to close the connection now */ infof(data, "partial download completed, closing connection"); - connclose(conn, "Partial download with no ability to check"); + connclose(conn); return result; } @@ -3838,7 +3839,7 @@ static CURLcode ftp_done(struct Curl_easy *data, CURLcode status, if(!ftp || !ftpc) return CURLE_OK; - result = ftp_done_status(data->conn, ftpc, status, premature); + result = ftp_done_status(data, ftpc, status, premature); ftp_done_wildcard(data, ftpc); ftp_done_path(data, ftpc, result); @@ -4299,7 +4300,7 @@ static CURLcode ftp_quit(struct Curl_easy *data, failf(data, "Failure sending QUIT command: %s", curl_easy_strerror(result)); ftpc->ctl_valid = FALSE; /* mark control connection as bad */ - connclose(data->conn, "QUIT command failed"); /* mark for closure */ + connclose(data->conn); /* mark for closure */ ftp_state(data, ftpc, FTP_STOP); return result; } diff --git a/lib/gopher.c b/lib/gopher.c index 9781ffff63..57dfa263e0 100644 --- a/lib/gopher.c +++ b/lib/gopher.c @@ -51,7 +51,7 @@ static CURLcode gopher_connecting(struct Curl_easy *data, bool *done) result = Curl_conn_connect(data, FIRSTSOCKET, TRUE, done); if(result) - connclose(conn, "Failed TLS connection"); + connclose(conn); *done = TRUE; return result; } diff --git a/lib/http.c b/lib/http.c index d8eca68198..91dcdb9716 100644 --- a/lib/http.c +++ b/lib/http.c @@ -464,7 +464,7 @@ static CURLcode http_perhapsrewind(struct Curl_easy *data, ongoing_auth ? ongoing_auth : "", ongoing_auth ? " send, " : ""); /* We decided to abort the ongoing transfer */ - streamclose(conn, "Mid-auth HTTP and much data left to send"); + streamclose(conn); data->req.size = 0; /* do not download any more than 0 bytes */ data->req.http_bodyless = TRUE; } @@ -577,7 +577,7 @@ CURLcode Curl_http_auth_act(struct Curl_easy *data) if(data->state.authhost.picked == CURLAUTH_NTLM && (data->req.httpversion_sent > 11)) { infof(data, "Forcing HTTP/1.1 for NTLM"); - connclose(conn, "Force HTTP/1.1 connection"); + connclose(conn); data->state.http_neg.wanted = CURL_HTTP_V1x; data->state.http_neg.allowed = CURL_HTTP_V1x; } @@ -1678,7 +1678,7 @@ CURLcode Curl_http_done(struct Curl_easy *data, return an error here */ failf(data, "Empty reply from server"); /* Mark it as closed to avoid the "left intact" message */ - streamclose(conn, "Empty reply from server"); + streamclose(conn); return CURLE_GOT_NOTHING; } @@ -2711,7 +2711,7 @@ static CURLcode http_firstwrite(struct Curl_easy *data) /* The resume point is at the end of file, consider this fine even if it does not allow resume from here. */ infof(data, "The entire document is already downloaded"); - streamclose(conn, "already downloaded"); + streamclose(conn); /* Abort download */ CURL_REQ_CLEAR_RECV(data); k->done = TRUE; @@ -2739,7 +2739,7 @@ static CURLcode http_firstwrite(struct Curl_easy *data) infof(data, "Simulate an HTTP 304 response"); /* we abort the transfer before it is completed == we ruin the reuse ability. Close the connection */ - streamclose(conn, "Simulated 304 handling"); + streamclose(conn); return CURLE_OK; } } /* we have a time condition */ @@ -3272,7 +3272,7 @@ static CURLcode http_header_c(struct Curl_easy *data, failf(data, "Maximum file size exceeded"); return CURLE_FILESIZE_EXCEEDED; } - streamclose(conn, "overflow content-length"); + streamclose(conn); infof(data, "Overflow Content-Length: value"); return CURLE_OK; } @@ -3333,7 +3333,7 @@ static CURLcode http_header_c(struct Curl_easy *data, * the connection will close when this request has been * served. */ - connclose(conn, "Connection: close used"); + connclose(conn); return CURLE_OK; } if((k->httpversion == 10) && @@ -3344,7 +3344,7 @@ static CURLcode http_header_c(struct Curl_easy *data, * pleasure. Default action for 1.0 is to close. * * [RFC2068, section 19.7.1] */ - connkeep(conn, "Connection keep-alive"); + connkeep(conn); infof(data, "HTTP/1.0 connection set to keep alive"); return CURLE_OK; } @@ -3460,7 +3460,7 @@ static CURLcode http_header_p(struct Curl_easy *data, * connection will be kept alive for our pleasure. * Default action for 1.0 is to close. */ - connkeep(conn, "Proxy-Connection keep-alive"); /* do not close */ + connkeep(conn); /* do not close */ infof(data, "HTTP/1.0 proxy connection set to keep alive"); } else if((k->httpversion == 11) && conn->http_proxy.peer && @@ -3469,7 +3469,7 @@ static CURLcode http_header_p(struct Curl_easy *data, * We get an HTTP/1.1 response from a proxy and it says it will * close down after this transfer. */ - connclose(conn, "Proxy-Connection: asked to close after done"); + connclose(conn); infof(data, "HTTP/1.1 proxy connection set close"); } return CURLE_OK; @@ -3634,7 +3634,8 @@ static CURLcode http_header_t(struct Curl_easy *data, /* if this is not chunked, only close can signal the end of this * transfer as Content-Length is said not to be trusted for * transfer-encoding! */ - connclose(conn, "HTTP/1.1 transfer-encoding without chunks"); + CURL_TRC_M(data, "HTTP/1.1 transfer-encoding without chunks"); + connclose(conn); k->ignore_cl = TRUE; } return CURLE_OK; @@ -3780,7 +3781,7 @@ static CURLcode http_statusline(struct Curl_easy *data, we get one of those fancy headers that tell us the server keeps it open for us! */ infof(data, "HTTP 1.0, assume close after body"); - connclose(conn, "HTTP/1.0 close after body"); + connclose(conn); } k->http_bodyless = k->httpcode >= 100 && k->httpcode < 200; @@ -4078,7 +4079,7 @@ static CURLcode http_handle_send_error(struct Curl_easy *data) } else { infof(data, "Got HTTP failure 417 while sending data"); - streamclose(conn, "Stop sending data before everything sent"); + streamclose(conn); result = http_perhapsrewind(data, conn); if(result) return result; @@ -4096,7 +4097,7 @@ static CURLcode http_handle_send_error(struct Curl_easy *data) } else { infof(data, "HTTP error before end of send, stop sending"); - streamclose(conn, "Stop sending data before everything sent"); + streamclose(conn); result = Curl_req_abort_sending(data); if(result) return result; @@ -4172,7 +4173,7 @@ static CURLcode http_on_response(struct Curl_easy *data, according to RFC2616 section 4.4 point 5, we assume that the server will close the connection to signal the end of the document. */ infof(data, "no chunk, no close, no size. Assume close to signal end"); - streamclose(conn, "HTTP: No end-of-message indicator"); + streamclose(conn); } http_check_auth_closure(data, conn); @@ -4508,7 +4509,7 @@ static CURLcode http_parse_headers(struct Curl_easy *data, /* this is not the beginning of a protocol first header line. * Cannot be 0.9 if version was detected or connection was reused. */ k->header = FALSE; - streamclose(conn, "bad HTTP: No end-of-message indicator"); + streamclose(conn); if((k->httpversion >= 10) || conn->bits.reuse) { failf(data, "Invalid status line"); return CURLE_WEIRD_SERVER_REPLY; @@ -4545,7 +4546,7 @@ static CURLcode http_parse_headers(struct Curl_easy *data, /* the first read "header", the status line */ statusline st = checkprotoprefix(data, conn, hd, hlen); if(st == STATUS_BAD) { - streamclose(conn, "bad HTTP: No end-of-message indicator"); + streamclose(conn); /* this is not the beginning of a protocol first header line. * Cannot be 0.9 if version was detected or connection was reused. */ if((k->httpversion >= 10) || conn->bits.reuse) { diff --git a/lib/http2.c b/lib/http2.c index 8612efc302..a63fc4a59c 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -512,7 +512,8 @@ static CURLcode h2_process_pending_input(struct Curl_cfilter *cf, the connection may not be reused. This is set when a GOAWAY frame has been received or when the limit of stream identifiers has been reached. */ - connclose(cf->conn, "http/2: No new requests allowed"); + CURL_TRC_M(data, "http/2: No new requests allowed"); + connclose(cf->conn); } return CURLE_OK; @@ -1688,7 +1689,7 @@ static CURLcode http2_handle_stream_close(struct Curl_cfilter *cf, if(stream->error == NGHTTP2_REFUSED_STREAM) { infof(data, "HTTP/2 stream %d refused by server, try again on a new " "connection", stream->id); - connclose(cf->conn, "REFUSED_STREAM"); /* do not use this anymore */ + connclose(cf->conn); /* do not use this anymore */ data->state.refused_stream = TRUE; return CURLE_RECV_ERROR; /* trigger Curl_retry_request() later */ } @@ -1930,8 +1931,9 @@ static CURLcode h2_progress_ingress(struct Curl_cfilter *cf, } if(ctx->conn_closed && Curl_bufq_is_empty(&ctx->inbufq)) { - connclose(cf->conn, ctx->rcvd_goaway ? "server closed with GOAWAY" : - "server closed abruptly"); + CURL_TRC_CF(data, cf, "server closed %s", + ctx->rcvd_goaway ? "with GOAWAY" : "abruptly"); + connclose(cf->conn); } CURL_TRC_CF(data, cf, "[0] ingress: done"); diff --git a/lib/imap.c b/lib/imap.c index fb23cfef41..6a48f85152 100644 --- a/lib/imap.c +++ b/lib/imap.c @@ -2010,7 +2010,8 @@ static CURLcode imap_done(struct Curl_easy *data, CURLcode status, return CURLE_OK; if(status) { - connclose(conn, "IMAP done with bad status"); /* marked for closure */ + CURL_TRC_M(data, "IMAP done with bad status"); + connclose(conn); /* marked for closure */ result = status; /* use the already set error code */ } else if(!data->set.connect_only && diff --git a/lib/ldap.c b/lib/ldap.c index b4b107a572..63a40784a8 100644 --- a/lib/ldap.c +++ b/lib/ldap.c @@ -652,7 +652,7 @@ quit: /* no data to transfer */ Curl_xfer_setup_nop(data); - connclose(conn, "LDAP connection always disable reuse"); + connclose(conn); return result; } diff --git a/lib/mqtt.c b/lib/mqtt.c index ad39792987..a9cdf21498 100644 --- a/lib/mqtt.c +++ b/lib/mqtt.c @@ -975,7 +975,7 @@ static CURLcode mqtts_connecting(struct Curl_easy *data, bool *done) result = Curl_conn_connect(data, FIRSTSOCKET, TRUE, done); if(result) - connclose(conn, "Failed TLS connection"); + connclose(conn); return result; } diff --git a/lib/multi.c b/lib/multi.c index 77cb306d31..f68124436f 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -78,7 +78,7 @@ static CURLMcode add_next_timeout(const struct curltime *pnow, static void multi_timeout(struct Curl_multi *multi, struct curltime *expire_time, long *timeout_ms); -static void process_pending_handles(struct Curl_multi *multi); +static void multi_schedule_pending(struct Curl_multi *multi); static void multi_xfer_bufs_free(struct Curl_multi *multi); #ifdef DEBUGBUILD static void multi_xfer_tbl_dump(struct Curl_multi *multi); @@ -666,14 +666,14 @@ static void multi_done_locked(struct connectdata *conn, conn->connection_id, conn->destination, data->set.reuse_forbid, conn->bits.close, mdctx->premature, Curl_conn_is_multiplex(conn, FIRSTSOCKET)); - connclose(conn, "disconnecting"); + connclose(conn); Curl_conn_terminate(data, conn, (bool)mdctx->premature); } else if(!Curl_conn_get_max_concurrent(data, conn, FIRSTSOCKET)) { CURL_TRC_M(data, "multi_done, conn #%" FMT_OFF_T " to %s was shutdown" " by server, not reusing", conn->connection_id, conn->destination); - connclose(conn, "server shutdown"); + connclose(conn); Curl_conn_terminate(data, conn, (bool)mdctx->premature); } else { @@ -751,7 +751,7 @@ static CURLcode multi_done(struct Curl_easy *data, if(conn) Curl_conn_ev_data_done(data, premature); - process_pending_handles(data->multi); /* connection / multiplex */ + multi_schedule_pending(data->multi); /* connection / multiplex */ if(!result) result = Curl_req_done(&data->req, data, premature); @@ -771,16 +771,6 @@ static CURLcode multi_done(struct Curl_easy *data, return result; } -static void close_connect_only(struct connectdata *conn, - struct Curl_easy *data, - void *userdata) -{ - (void)userdata; - (void)data; - if(conn->bits.connect_only) - connclose(conn, "Removing connect-only easy handle"); -} - CURLMcode Curl_multi_remove_handle(struct Curl_multi *multi, struct Curl_easy *data) { @@ -808,20 +798,14 @@ CURLMcode Curl_multi_remove_handle(struct Curl_multi *multi, premature = (data->mstate < MSTATE_COMPLETED); - /* If the 'state' is not INIT or COMPLETED, we might need to do something - nice to put the easy_handle in a good known state when this returns. */ - if(data->conn && - data->mstate > MSTATE_DO && - data->mstate < MSTATE_COMPLETED) { - /* Set connection owner so that the DONE function closes it. We can - safely do this here since connection is killed. */ - streamclose(data->conn, "Removed with partial response"); - } - if(data->conn) { + /* If the 'state' is not INIT or COMPLETED, we might need to do something + nice to put the easy_handle in a good known state when this returns. */ + if(premature && (data->mstate > MSTATE_DO)) + streamclose(data->conn); + /* multi_done() clears the association between the easy handle and the connection. - Note that this ignores the return code because there is nothing really useful to do with it anyway! */ (void)multi_done(data, data->result, premature); @@ -835,6 +819,7 @@ CURLMcode Curl_multi_remove_handle(struct Curl_multi *multi, /* If in `msgsent`, it was deducted from `multi->xfers_alive` already. */ if(!Curl_uint32_bset_contains(&multi->msgsent, data->mid)) --multi->xfers_alive; + if(data->state.really_alive) { data->state.really_alive = FALSE; --multi->xfers_really_alive; @@ -852,26 +837,29 @@ CURLMcode Curl_multi_remove_handle(struct Curl_multi *multi, /* Tell event handling that this transfer is definitely going away */ Curl_multi_ev_xfer_done(multi, data); - if(data->set.connect_only && !data->multi_easy) { - /* This removes a handle that was part the multi interface that used - CONNECT_ONLY, that connection is now left alive but since this handle - has bits.close set nothing can use that transfer anymore and it is - forbidden from reuse. This easy handle cannot find the connection - anymore once removed from the multi handle - - Better close the connection here, at once. */ - struct connectdata *c; - curl_socket_t s; - s = Curl_getconnectinfo(data, &c); - if((s != CURL_SOCKET_BAD) && c) { - Curl_conn_terminate(data, c, TRUE); + if(data->set.connect_only) { + if(data->multi_easy) { + if(data->state.lastconnect_id != -1) { + /* Mark any connect-only connection for closure */ + struct connectdata *conn; + (void)Curl_getconnectinfo(data, &conn); + if(conn && conn->bits.connect_only) + connclose(conn); + } } - } + else { + /* This removes a handle that was part the multi interface that used + CONNECT_ONLY, that connection is now left alive but since this handle + has bits.close set nothing can use that connection anymore and it is + forbidden from reuse. This easy handle cannot find the connection + anymore once removed from the multi handle - if(data->state.lastconnect_id != -1) { - /* Mark any connect-only connection for closure */ - Curl_cpool_do_by_id(data, data->state.lastconnect_id, - close_connect_only, NULL); + Better close the connection here, at once. */ + struct connectdata *conn; + (void)Curl_getconnectinfo(data, &conn); + if(conn) + Curl_conn_terminate(data, conn, TRUE); + } } #ifdef USE_LIBPSL @@ -904,10 +892,8 @@ CURLMcode Curl_multi_remove_handle(struct Curl_multi *multi, data->mid = UINT32_MAX; data->master_mid = UINT32_MAX; - /* NOTE NOTE NOTE - We do not touch the easy handle here! */ - process_pending_handles(multi); - + /* A pending transfer *might* be able to run now. */ + multi_schedule_pending(multi); mresult = Curl_update_timer(multi); if(mresult) return mresult; @@ -1872,7 +1858,7 @@ static bool multi_handle_timeout(struct Curl_easy *data, if(data->conn) { /* Force connection closed if the connection has indeed been used */ if(data->mstate > MSTATE_DO) { - streamclose(data->conn, "Disconnect due to timeout"); + streamclose(data->conn); *stream_error = TRUE; } (void)multi_done(data, *result, TRUE); @@ -2077,7 +2063,7 @@ static CURLMcode multistate_performing(struct Curl_easy *data, if(!ret) { infof(data, "Downgrades to HTTP/1.1"); - streamclose(data->conn, "Disconnect HTTP/2 for HTTP/1"); + streamclose(data->conn); data->state.http_neg.wanted = CURL_HTTP_V1x; data->state.http_neg.allowed = CURL_HTTP_V1x; /* clear the error message bit too as we ignore the one we got */ @@ -2112,7 +2098,7 @@ static CURLMcode multistate_performing(struct Curl_easy *data, if(!(data->conn->scheme->flags & PROTOPT_DUAL) && result != CURLE_HTTP2_STREAM) - streamclose(data->conn, "Transfer returned error"); + streamclose(data->conn); multi_posttransfer(data); multi_done(data, result, TRUE); @@ -2323,7 +2309,7 @@ static CURLMcode multistate_ratelimiting(struct Curl_easy *data, if(result) { if(!(data->conn->scheme->flags & PROTOPT_DUAL) && result != CURLE_HTTP2_STREAM) - streamclose(data->conn, "Transfer returned error"); + streamclose(data->conn); multi_posttransfer(data); multi_done(data, result, TRUE); @@ -2357,7 +2343,7 @@ static CURLMcode multistate_connect(struct Curl_multi *multi, return mresult; } else - process_pending_handles(data->multi); + multi_schedule_pending(data->multi); if(!result) { /* after the connect has been sent off, go WAITCONNECT unless the @@ -2369,7 +2355,7 @@ static CURLMcode multistate_connect(struct Curl_multi *multi, if(!data->conn->bits.reuse && Curl_conn_is_multiplex(data->conn, FIRSTSOCKET)) { /* new connection, can multiplex, wake pending handles */ - process_pending_handles(data->multi); + multi_schedule_pending(data->multi); } multistate(data, MSTATE_PROTOCONNECT); } @@ -2398,7 +2384,7 @@ static CURLcode is_finished(struct Curl_multi *multi, connection detach and termination happens only here */ /* Check if we can move pending requests to send pipe */ - process_pending_handles(multi); /* connection */ + multi_schedule_pending(multi); /* connection */ if(data->conn) { if(stream_error) { @@ -2428,7 +2414,7 @@ static CURLcode is_finished(struct Curl_multi *multi, if(result) { /* aborted due to progress callback return code must close the connection */ - streamclose(data->conn, "Aborted by callback"); + streamclose(data->conn); /* if not yet in DONE state, go there, otherwise COMPLETED */ multistate(data, (data->mstate < MSTATE_DONE) ? @@ -2542,7 +2528,7 @@ static CURLMcode multistate_connecting(struct Curl_easy *data, if(!data->conn->bits.reuse && Curl_conn_is_multiplex(data->conn, FIRSTSOCKET)) { /* new connection, can multiplex, wake pending handles */ - process_pending_handles(data->multi); + multi_schedule_pending(data->multi); } multistate(data, MSTATE_PROTOCONNECT); return CURLM_CALL_MULTI_PERFORM; @@ -2678,7 +2664,7 @@ static CURLMcode multistate_did(struct Curl_multi *multi, DEBUGASSERT(data->conn); if(data->conn->bits.multiplex) /* Check if we can move pending requests to send pipe */ - process_pending_handles(multi); /* multiplexed */ + multi_schedule_pending(multi); /* multiplexed */ /* Only perform the transfer if there is a good socket to work with. Having both BAD is a signal to skip immediately to DONE */ @@ -2770,7 +2756,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, if(multi_ischanged(multi, TRUE)) { CURL_TRC_M(data, "multi changed, check CONNECT_PEND queue"); - process_pending_handles(multi); /* multiplexed */ + multi_schedule_pending(multi); /* multiplexed */ } if(data->mstate > MSTATE_CONNECT && @@ -2923,7 +2909,7 @@ static CURLMcode multi_perform(struct Curl_multi *multi, sigpipe_restore(&sigpipe_ctx); if(multi_ischanged(multi, TRUE)) - process_pending_handles(multi); + multi_schedule_pending(multi); if(!returncode && CURL_MNTFY_HAS_ENTRIES(multi)) returncode = Curl_mntfy_dispatch_all(multi); @@ -3330,7 +3316,7 @@ out: sigpipe_restore(&pipe_ctx); if(multi_ischanged(multi, TRUE)) - process_pending_handles(multi); + multi_schedule_pending(multi); if(!mresult && CURL_MNTFY_HAS_ENTRIES(multi)) mresult = Curl_mntfy_dispatch_all(multi); @@ -3879,7 +3865,7 @@ static void move_pending_to_connect(struct Curl_multi *multi, Curl_multi_mark_dirty(data); /* make it run */ } -/* process_pending_handles() moves a handle from PENDING back into the process +/* multi_schedule_pending() moves a handle from PENDING back into the process list and change state to CONNECT. We do not move all transfers because that can be a significant amount. @@ -3892,7 +3878,7 @@ static void move_pending_to_connect(struct Curl_multi *multi, We could consider an improvement where we store the queue reason and allow more pipewait rechecks than others. */ -static void process_pending_handles(struct Curl_multi *multi) +static void multi_schedule_pending(struct Curl_multi *multi) { uint32_t mid = multi->last_pending_mid; diff --git a/lib/pop3.c b/lib/pop3.c index 7557c74350..071872687c 100644 --- a/lib/pop3.c +++ b/lib/pop3.c @@ -1478,7 +1478,8 @@ static CURLcode pop3_done(struct Curl_easy *data, CURLcode status, return CURLE_OK; if(status) { - connclose(data->conn, "POP3 done with bad status"); + CURL_TRC_M(data, "POP3 done with bad status"); + connclose(data->conn); result = status; /* use the already set error code */ } diff --git a/lib/sendf.c b/lib/sendf.c index 449a4bc447..27f3055214 100644 --- a/lib/sendf.c +++ b/lib/sendf.c @@ -210,7 +210,7 @@ static CURLcode cw_download_write(struct Curl_easy *data, if(data->req.no_body && nbytes > 0) { /* BODY arrives although we want none, bail out */ - streamclose(data->conn, "ignoring body"); + streamclose(data->conn); CURL_TRC_WRITE(data, "download_write body(type=%x, blen=%zu), " "did not want a BODY", (unsigned int)type, nbytes); data->req.download_done = TRUE; @@ -277,7 +277,7 @@ static CURLcode cw_download_write(struct Curl_easy *data, ", bytecount = %" FMT_OFF_T, excess_len, data->req.size, data->req.maxdownload, data->req.bytecount); - connclose(data->conn, "excess found in a read"); + connclose(data->conn); } } else if((nwrite < nbytes) && !data->req.ignorebody) { diff --git a/lib/smb.c b/lib/smb.c index 70af8d5970..2d1a859aa1 100644 --- a/lib/smb.c +++ b/lib/smb.c @@ -912,7 +912,8 @@ static CURLcode smb_connection_state(struct Curl_easy *data, bool *done) result = smb_send_negotiate(data, smbc, req); if(result) { - connclose(conn, "SMB: failed to send negotiate message"); + CURL_TRC_M(data, "SMB: failed to send negotiate message"); + connclose(conn); return result; } @@ -922,7 +923,8 @@ static CURLcode smb_connection_state(struct Curl_easy *data, bool *done) /* Send the previous message and check for a response */ result = smb_send_and_recv(data, smbc, &msg); if(result && result != CURLE_AGAIN) { - connclose(conn, "SMB: failed to communicate"); + CURL_TRC_M(data, "SMB: failed to communicate"); + connclose(conn); return result; } @@ -935,7 +937,8 @@ static CURLcode smb_connection_state(struct Curl_easy *data, bool *done) case SMB_NEGOTIATE: if((smbc->got < sizeof(*nrsp) + sizeof(smbc->challenge) - 1) || h->status) { - connclose(conn, "SMB: negotiation failed"); + CURL_TRC_M(data, "SMB: negotiation failed"); + connclose(conn); return CURLE_COULDNT_CONNECT; } nrsp = msg; @@ -952,7 +955,8 @@ static CURLcode smb_connection_state(struct Curl_easy *data, bool *done) smbc->session_key = smb_swap32(nrsp->session_key); result = smb_send_setup(data); if(result) { - connclose(conn, "SMB: failed to send setup message"); + CURL_TRC_M(data, "SMB: failed to send setup message"); + connclose(conn); return result; } conn_state(data, smbc, SMB_SETUP); @@ -960,7 +964,8 @@ static CURLcode smb_connection_state(struct Curl_easy *data, bool *done) case SMB_SETUP: if(h->status) { - connclose(conn, "SMB: authentication failed"); + CURL_TRC_M(data, "SMB: authentication failed"); + connclose(conn); return CURLE_LOGIN_DENIED; } smbc->uid = smb_swap16(h->uid); @@ -1025,7 +1030,8 @@ static CURLcode smb_request_state(struct Curl_easy *data, bool *done) if(req->state == SMB_REQUESTING) { result = smb_send_tree_connect(data, smbc, req); if(result) { - connclose(conn, "SMB: failed to send tree connect message"); + CURL_TRC_M(data, "SMB: failed to send tree connect message"); + connclose(conn); return result; } @@ -1035,7 +1041,8 @@ static CURLcode smb_request_state(struct Curl_easy *data, bool *done) /* Send the previous message and check for a response */ result = smb_send_and_recv(data, smbc, &msg); if(result && result != CURLE_AGAIN) { - connclose(conn, "SMB: failed to communicate"); + CURL_TRC_M(data, "SMB: failed to communicate"); + connclose(conn); return result; } @@ -1180,7 +1187,8 @@ static CURLcode smb_request_state(struct Curl_easy *data, bool *done) } if(result) { - connclose(conn, "SMB: failed to send message"); + CURL_TRC_M(data, "SMB: failed to send message"); + connclose(conn); return result; } diff --git a/lib/smtp.c b/lib/smtp.c index f5d922ddb4..a37a573381 100644 --- a/lib/smtp.c +++ b/lib/smtp.c @@ -1739,7 +1739,8 @@ static CURLcode smtp_done(struct Curl_easy *data, CURLcode status, curlx_safefree(smtp->custom); if(status) { - connclose(conn, "SMTP done with bad status"); /* marked for closure */ + CURL_TRC_M(data, "SMTP done with bad status"); + connclose(conn); /* marked for closure */ result = status; /* use the already set error code */ } else if(!data->set.connect_only && data->set.mail_rcpt && diff --git a/lib/tftp.c b/lib/tftp.c index 00e89752bd..d5c85a3100 100644 --- a/lib/tftp.c +++ b/lib/tftp.c @@ -942,7 +942,7 @@ static CURLcode tftp_connect(struct Curl_easy *data, bool *done) /* we do not keep TFTP connections up because there is none or little gain * for UDP */ - connclose(conn, "TFTP"); + connclose(conn); state->data = data; state->sockfd = conn->sock[FIRSTSOCKET]; diff --git a/lib/transfer.c b/lib/transfer.c index b7902de9a1..3a7b6bc136 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -658,7 +658,7 @@ CURLcode Curl_retry_request(struct Curl_easy *data, char **url) if(!*url) return CURLE_OUT_OF_MEMORY; - connclose(conn, "retry"); /* close this connection */ + connclose(conn); /* close this connection */ conn->bits.retry = TRUE; /* mark this as a connection we are about to retry. Marking it this way should prevent i.e HTTP transfers to return error because nothing diff --git a/lib/vquic/cf-ngtcp2-proxy.c b/lib/vquic/cf-ngtcp2-proxy.c index 9a43a443cc..3a7023f300 100644 --- a/lib/vquic/cf-ngtcp2-proxy.c +++ b/lib/vquic/cf-ngtcp2-proxy.c @@ -665,7 +665,7 @@ static ssize_t cf_h3_proxy_recv_closed_stream(struct Curl_cfilter *cf, if(stream->error3 == CURL_H3_ERR_REQUEST_REJECTED) { infof(data, "HTTP/3 stream %" PRId64 " refused by server, try again " "on a new connection", stream->id); - connclose(cf->conn, "REFUSED_STREAM"); + connclose(cf->conn); data->state.refused_stream = TRUE; *err = CURLE_RECV_ERROR; goto out; diff --git a/lib/vquic/cf-ngtcp2.c b/lib/vquic/cf-ngtcp2.c index 9a905a745a..980516d05e 100644 --- a/lib/vquic/cf-ngtcp2.c +++ b/lib/vquic/cf-ngtcp2.c @@ -449,7 +449,7 @@ static CURLcode recv_closed_stream(struct Curl_cfilter *cf, if(stream->error3 == CURL_H3_ERR_REQUEST_REJECTED) { infof(data, "HTTP/3 stream %" PRId64 " refused by server, try again " "on a new connection", stream->id); - connclose(cf->conn, "REFUSED_STREAM"); /* do not use this anymore */ + connclose(cf->conn); /* do not use this anymore */ data->state.refused_stream = TRUE; return CURLE_RECV_ERROR; /* trigger Curl_retry_request() later */ } diff --git a/lib/vquic/cf-quiche.c b/lib/vquic/cf-quiche.c index e0016d37d2..c23b810254 100644 --- a/lib/vquic/cf-quiche.c +++ b/lib/vquic/cf-quiche.c @@ -866,7 +866,7 @@ static CURLcode recv_closed_stream(struct Curl_cfilter *cf, if(stream->error3 == CURL_H3_ERR_REQUEST_REJECTED) { infof(data, "HTTP/3 stream %" PRIu64 " refused by server, try again " "on a new connection", stream->id); - connclose(cf->conn, "REFUSED_STREAM"); /* do not use this anymore */ + connclose(cf->conn); /* do not use this anymore */ data->state.refused_stream = TRUE; return CURLE_RECV_ERROR; /* trigger Curl_retry_request() later */ } diff --git a/lib/vssh/libssh.c b/lib/vssh/libssh.c index 89d2f33bcc..f1a206ba2d 100644 --- a/lib/vssh/libssh.c +++ b/lib/vssh/libssh.c @@ -2238,7 +2238,7 @@ static CURLcode myssh_in_SESSION_FREE(struct Curl_easy *data, /* the code we are about to return */ result = sshc->actualcode; memset(sshc, 0, sizeof(struct ssh_conn)); - connclose(data->conn, "SSH session free"); + connclose(data->conn); sshc->state = SSH_SESSION_FREE; /* current */ sshc->nextstate = SSH_NO_STATE; myssh_to(data, sshc, SSH_STOP); diff --git a/lib/vssh/libssh2.c b/lib/vssh/libssh2.c index 96c1157669..effcc9beaf 100644 --- a/lib/vssh/libssh2.c +++ b/lib/vssh/libssh2.c @@ -2850,7 +2850,7 @@ static CURLcode ssh_state_session_free(struct Curl_easy *data, if(result) return result; memset(sshc, 0, sizeof(struct ssh_conn)); - connclose(conn, "SSH session free"); + connclose(conn); sshc->state = SSH_SESSION_FREE; /* current */ myssh_to(data, sshc, SSH_STOP); return CURLE_OK; diff --git a/tests/data/test1554 b/tests/data/test1554 index fa071b09b2..f766785647 100644 --- a/tests/data/test1554 +++ b/tests/data/test1554 @@ -33,8 +33,6 @@ run 1: foobar and so on fun! run 1: foobar and so on fun! -] Mutex lock CONNECT [- Mutex unlock CONNECT --] Mutex lock CONNECT -[- Mutex unlock CONNECT -] Mutex lock SHARE [- Mutex unlock SHARE -] Mutex lock SHARE @@ -48,8 +46,6 @@ run 1: foobar and so on fun! run 1: foobar and so on fun! -] Mutex lock CONNECT [- Mutex unlock CONNECT --] Mutex lock CONNECT -[- Mutex unlock CONNECT -] Mutex lock SHARE [- Mutex unlock SHARE -] Mutex lock SHARE @@ -63,8 +59,6 @@ run 1: foobar and so on fun! run 1: foobar and so on fun! -] Mutex lock CONNECT [- Mutex unlock CONNECT --] Mutex lock CONNECT -[- Mutex unlock CONNECT -] Mutex lock SHARE [- Mutex unlock SHARE -] Mutex lock SHARE