From: Stefan Eissing Date: Fri, 11 Apr 2025 10:05:05 +0000 (+0200) Subject: cpool/cshutdown: force close connections under pressure X-Git-Tag: curl-8_14_0~315 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=ff37657e4d94cf4db80ab9652ae8e6acb84f3019;p=thirdparty%2Fcurl.git cpool/cshutdown: force close connections under pressure when CURLMOPT_MAX_HOST_CONNECTIONS or CURLMOPT_MAX_TOTAL_CONNECTIONS limits are reached, force close connections in shutdown to go below limit when possible. Fixes #17020 Reported-by: Fujii Hironori Closes #17022 --- diff --git a/lib/conncache.c b/lib/conncache.c index fe0b07dcb7..072fdd44fa 100644 --- a/lib/conncache.c +++ b/lib/conncache.c @@ -375,24 +375,33 @@ int Curl_cpool_check_limits(struct Curl_easy *data, bundle = cpool_find_bundle(cpool, conn); live = bundle ? Curl_llist_count(&bundle->conns) : 0; - shutdowns = Curl_cshutdn_dest_count(data, conn->destination); - while(!shutdowns && bundle && live >= dest_limit) { - struct connectdata *oldest_idle = NULL; - /* The bundle is full. Extract the oldest connection that may - * be removed now, if there is one. */ - oldest_idle = cpool_bundle_get_oldest_idle(bundle); - if(!oldest_idle) + shutdowns = Curl_cshutdn_dest_count(data, conn->destination); + while((live + shutdowns) >= dest_limit) { + if(shutdowns) { + /* close one connection in shutdown right away, if we can */ + if(!Curl_cshutdn_close_oldest(data, conn->destination)) + break; + } + else if(!bundle) break; - /* disconnect the old conn and continue */ - CURL_TRC_M(data, "Discarding connection #%" - FMT_OFF_T " from %zu to reach destination " - "limit of %zu", oldest_idle->connection_id, - Curl_llist_count(&bundle->conns), dest_limit); - Curl_conn_terminate(cpool->idata, oldest_idle, FALSE); - - /* in case the bundle was destroyed in disconnect, look it up again */ - bundle = cpool_find_bundle(cpool, conn); - live = bundle ? Curl_llist_count(&bundle->conns) : 0; + else { + struct connectdata *oldest_idle = NULL; + /* The bundle is full. Extract the oldest connection that may + * be removed now, if there is one. */ + oldest_idle = cpool_bundle_get_oldest_idle(bundle); + if(!oldest_idle) + break; + /* disconnect the old conn and continue */ + CURL_TRC_M(data, "Discarding connection #%" + FMT_OFF_T " from %zu to reach destination " + "limit of %zu", oldest_idle->connection_id, + Curl_llist_count(&bundle->conns), dest_limit); + Curl_conn_terminate(cpool->idata, oldest_idle, FALSE); + + /* in case the bundle was destroyed in disconnect, look it up again */ + bundle = cpool_find_bundle(cpool, conn); + live = bundle ? Curl_llist_count(&bundle->conns) : 0; + } shutdowns = Curl_cshutdn_dest_count(cpool->idata, conn->destination); } if((live + shutdowns) >= dest_limit) { @@ -404,15 +413,22 @@ int Curl_cpool_check_limits(struct Curl_easy *data, if(total_limit) { shutdowns = Curl_cshutdn_count(cpool->idata); while((cpool->num_conn + shutdowns) >= total_limit) { - struct connectdata *oldest_idle = cpool_get_oldest_idle(cpool); - if(!oldest_idle) - break; - /* disconnect the old conn and continue */ - CURL_TRC_M(data, "Discarding connection #%" - FMT_OFF_T " from %zu to reach total " - "limit of %zu", - oldest_idle->connection_id, cpool->num_conn, total_limit); - Curl_conn_terminate(cpool->idata, oldest_idle, FALSE); + if(shutdowns) { + /* close one connection in shutdown right away, if we can */ + if(!Curl_cshutdn_close_oldest(data, NULL)) + break; + } + else { + struct connectdata *oldest_idle = cpool_get_oldest_idle(cpool); + if(!oldest_idle) + break; + /* disconnect the old conn and continue */ + CURL_TRC_M(data, "Discarding connection #%" + FMT_OFF_T " from %zu to reach total " + "limit of %zu", + oldest_idle->connection_id, cpool->num_conn, total_limit); + Curl_conn_terminate(cpool->idata, oldest_idle, FALSE); + } shutdowns = Curl_cshutdn_count(cpool->idata); } if((cpool->num_conn + shutdowns) >= total_limit) { diff --git a/lib/cshutdn.c b/lib/cshutdn.c index 45581bd081..83972e3756 100644 --- a/lib/cshutdn.c +++ b/lib/cshutdn.c @@ -166,7 +166,9 @@ void Curl_cshutdn_terminate(struct Curl_easy *data, * not done so already. */ cshutdn_run_once(admin, conn, &done); } - CURL_TRC_M(admin, "[SHUTDOWN] closing connection"); + CURL_TRC_M(admin, "[SHUTDOWN] %sclosing connection #%" FMT_OFF_T, + conn->bits.shutdown_filters ? "" : "force ", + conn->connection_id); Curl_conn_close(admin, SECONDARYSOCKET); Curl_conn_close(admin, FIRSTSOCKET); Curl_detach_connection(admin); @@ -181,13 +183,21 @@ void Curl_cshutdn_terminate(struct Curl_easy *data, } } -static void cshutdn_destroy_oldest(struct cshutdn *cshutdn, - struct Curl_easy *data) +static bool cshutdn_destroy_oldest(struct cshutdn *cshutdn, + struct Curl_easy *data, + const char *destination) { struct Curl_llist_node *e; struct connectdata *conn; e = Curl_llist_head(&cshutdn->list); + while(e) { + conn = Curl_node_elem(e); + if(!destination || !strcmp(destination, conn->destination)) + break; + e = Curl_node_next(e); + } + if(e) { SIGPIPE_VARIABLE(pipe_st); conn = Curl_node_elem(e); @@ -196,7 +206,19 @@ static void cshutdn_destroy_oldest(struct cshutdn *cshutdn, sigpipe_apply(data, &pipe_st); Curl_cshutdn_terminate(data, conn, FALSE); sigpipe_restore(&pipe_st); + return TRUE; + } + return FALSE; +} + +bool Curl_cshutdn_close_oldest(struct Curl_easy *data, + const char *destination) +{ + if(data && data->multi) { + struct cshutdn *csd = &data->multi->cshutdn; + return cshutdn_destroy_oldest(csd, data, destination); } + return FALSE; } #define NUM_POLLS_ON_STACK 10 @@ -414,7 +436,7 @@ void Curl_cshutdn_add(struct cshutdn *cshutdn, (conns_in_pool + Curl_llist_count(&cshutdn->list)))) { CURL_TRC_M(data, "[SHUTDOWN] discarding oldest shutdown connection " "due to connection limit of %zu", max_total); - cshutdn_destroy_oldest(cshutdn, data); + cshutdn_destroy_oldest(cshutdn, data, NULL); } if(cshutdn->multi->socket_cb) { diff --git a/lib/cshutdn.h b/lib/cshutdn.h index 202e869838..7b95144479 100644 --- a/lib/cshutdn.h +++ b/lib/cshutdn.h @@ -75,6 +75,12 @@ size_t Curl_cshutdn_count(struct Curl_easy *data); size_t Curl_cshutdn_dest_count(struct Curl_easy *data, const char *destination); +/* Close the oldest connection in shutdown to destination or, + * when destination is NULL for any destination. + * Return TRUE if a connection has been closed. */ +bool Curl_cshutdn_close_oldest(struct Curl_easy *data, + const char *destination); + /* Add a connection to have it shut down. Will terminate the oldest * connection when total connection limit of multi is being reached. */ void Curl_cshutdn_add(struct cshutdn *cshutdn, diff --git a/lib/url.c b/lib/url.c index 2125a97afd..d94a29375e 100644 --- a/lib/url.c +++ b/lib/url.c @@ -3597,10 +3597,12 @@ static CURLcode create_conn(struct Curl_easy *data, conn->bits.tls_enable_alpn = TRUE; } - if(waitpipe) + if(waitpipe) { /* There is a connection that *might* become usable for multiplexing "soon", and we wait for that */ + infof(data, "Waiting on connection to negotiate possible multiplexing."); connections_available = FALSE; + } else { switch(Curl_cpool_check_limits(data, conn)) { case CPOOL_LIMIT_DEST: @@ -3614,7 +3616,8 @@ static CURLcode create_conn(struct Curl_easy *data, else #endif { - infof(data, "No connections available in cache"); + infof(data, "No connections available, total of %ld reached.", + data->multi->max_total_connections); connections_available = FALSE; } break; @@ -3624,8 +3627,6 @@ static CURLcode create_conn(struct Curl_easy *data, } if(!connections_available) { - infof(data, "No connections available."); - Curl_conn_free(data, conn); *in_connect = NULL; diff --git a/tests/http/test_19_shutdown.py b/tests/http/test_19_shutdown.py index ea68391357..fad1314fc4 100644 --- a/tests/http/test_19_shutdown.py +++ b/tests/http/test_19_shutdown.py @@ -153,7 +153,7 @@ class TestShutdown: r.check_response(http_status=200, count=count) # check that we closed all connections closings = [line for line in r.trace_lines - if re.match(r'.*SHUTDOWN\] closing', line)] + if re.match(r'.*SHUTDOWN\] (force )?closing', line)] assert len(closings) == count, f'{closings}' # check that all connection sockets were removed from event removes = [line for line in r.trace_lines @@ -180,3 +180,32 @@ class TestShutdown: shutdowns = [line for line in r.trace_lines if re.match(r'.*SHUTDOWN\] shutdown, done=1', line)] assert len(shutdowns) == 1, f'{shutdowns}' + + # run connection pressure, many small transfers, not reusing connections, + # limited total + @pytest.mark.parametrize("proto", ['http/1.1']) + def test_19_07_shutdown_by_curl(self, env: Env, httpd, proto): + if not env.curl_is_debug(): + pytest.skip('only works for curl debug builds') + count = 500 + docname = 'data.json' + url = f'https://localhost:{env.https_port}/{docname}' + client = LocalClient(name='hx-download', env=env, run_env={ + 'CURL_GRACEFUL_SHUTDOWN': '2000', + 'CURL_DEBUG': 'ssl,multi' + }) + if not client.exists(): + pytest.skip(f'example client not built: {client.name}') + r = client.run(args=[ + '-n', f'{count}', #that many transfers + '-f', # forbid conn reuse + '-m', '10', # max parallel + '-T', '5', # max total conns at a time + '-V', proto, + url + ]) + r.check_exit_code(0) + shutdowns = [line for line in r.trace_lines + if re.match(r'.*SHUTDOWN\] shutdown, done=1', line)] + # we see less clean shutdowns as total limit forces early closes + assert len(shutdowns) < count, f'{shutdowns}'