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
This commit is contained in:
Stefan Eissing 2025-04-11 12:05:05 +02:00 committed by Daniel Stenberg
parent 9f8bdd0eae
commit ff37657e4d
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
5 changed files with 108 additions and 34 deletions

View File

@ -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);
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;
/* 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) {

View File

@ -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) {

View File

@ -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,

View File

@ -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;

View File

@ -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}'