connect: fix connection shutdown for event based processing

connections being shutdown would register sockets for events, but then
never remove these sockets again. Nor would the shutdown effectively
been performed.

- If a socket event involves a transfer, check if that is the
  connection cache internal handle and run its multi_perform()
  instead (the internal handle is used for all shutdowns).
- When a timer triggers for a transfer, check also if it is
  about the connection cache internal handle.
- During processing shutdowns in the connection cache, assess
  the shutdown timeouts. Register a Curl_expire() of the lowest
  value for the cache's internal handle.

Reported-by: Gordon Parke
Fixes #14280
Closes #14296
This commit is contained in:
Stefan Eissing 2024-07-29 10:23:20 +02:00 committed by Daniel Stenberg
parent 14f630ecf6
commit 17e6f06ea3
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
6 changed files with 103 additions and 39 deletions

View File

@ -584,15 +584,15 @@ static void connc_close_all(struct conncache *connc)
return; return;
/* Move all connections to the shutdown list */ /* Move all connections to the shutdown list */
sigpipe_init(&pipe_st);
conn = connc_find_first_connection(connc); conn = connc_find_first_connection(connc);
while(conn) { while(conn) {
connc_remove_conn(connc, conn); connc_remove_conn(connc, conn);
sigpipe_ignore(data, &pipe_st); sigpipe_apply(data, &pipe_st);
/* This will remove the connection from the cache */ /* This will remove the connection from the cache */
connclose(conn, "kill all"); connclose(conn, "kill all");
Curl_conncache_remove_conn(connc->closure_handle, conn, TRUE); Curl_conncache_remove_conn(connc->closure_handle, conn, TRUE);
connc_discard_conn(connc, connc->closure_handle, conn, FALSE); connc_discard_conn(connc, connc->closure_handle, conn, FALSE);
sigpipe_restore(&pipe_st);
conn = connc_find_first_connection(connc); conn = connc_find_first_connection(connc);
} }
@ -613,7 +613,7 @@ static void connc_close_all(struct conncache *connc)
/* discard all connections in the shutdown list */ /* discard all connections in the shutdown list */
connc_shutdown_discard_all(connc); connc_shutdown_discard_all(connc);
sigpipe_ignore(data, &pipe_st); sigpipe_apply(data, &pipe_st);
Curl_hostcache_clean(data, data->dns.hostcache); Curl_hostcache_clean(data, data->dns.hostcache);
Curl_close(&data); Curl_close(&data);
sigpipe_restore(&pipe_st); sigpipe_restore(&pipe_st);
@ -628,7 +628,6 @@ static void connc_shutdown_discard_oldest(struct conncache *connc)
{ {
struct Curl_llist_element *e; struct Curl_llist_element *e;
struct connectdata *conn; struct connectdata *conn;
SIGPIPE_VARIABLE(pipe_st);
DEBUGASSERT(!connc->shutdowns.iter_locked); DEBUGASSERT(!connc->shutdowns.iter_locked);
if(connc->shutdowns.iter_locked) if(connc->shutdowns.iter_locked)
@ -636,9 +635,11 @@ static void connc_shutdown_discard_oldest(struct conncache *connc)
e = connc->shutdowns.conn_list.head; e = connc->shutdowns.conn_list.head;
if(e) { if(e) {
SIGPIPE_VARIABLE(pipe_st);
conn = e->ptr; conn = e->ptr;
Curl_llist_remove(&connc->shutdowns.conn_list, e, NULL); Curl_llist_remove(&connc->shutdowns.conn_list, e, NULL);
sigpipe_ignore(connc->closure_handle, &pipe_st); sigpipe_init(&pipe_st);
sigpipe_apply(connc->closure_handle, &pipe_st);
connc_disconnect(NULL, conn, connc, FALSE); connc_disconnect(NULL, conn, connc, FALSE);
sigpipe_restore(&pipe_st); sigpipe_restore(&pipe_st);
} }
@ -900,6 +901,9 @@ static void connc_perform(struct conncache *connc)
struct Curl_llist_element *e = connc->shutdowns.conn_list.head; struct Curl_llist_element *e = connc->shutdowns.conn_list.head;
struct Curl_llist_element *enext; struct Curl_llist_element *enext;
struct connectdata *conn; struct connectdata *conn;
struct curltime *nowp = NULL;
struct curltime now;
timediff_t next_from_now_ms = 0, ms;
bool done; bool done;
if(!e) if(!e)
@ -922,9 +926,22 @@ static void connc_perform(struct conncache *connc)
Curl_llist_remove(&connc->shutdowns.conn_list, e, NULL); Curl_llist_remove(&connc->shutdowns.conn_list, e, NULL);
connc_disconnect(NULL, conn, connc, FALSE); connc_disconnect(NULL, conn, connc, FALSE);
} }
else {
/* Not done, when does this connection time out? */
if(!nowp) {
now = Curl_now();
nowp = &now;
}
ms = Curl_conn_shutdown_timeleft(conn, nowp);
if(ms && ms < next_from_now_ms)
next_from_now_ms = ms;
}
e = enext; e = enext;
} }
connc->shutdowns.iter_locked = FALSE; connc->shutdowns.iter_locked = FALSE;
if(next_from_now_ms)
Curl_expire(data, next_from_now_ms, EXPIRE_RUN_NOW);
} }
void Curl_conncache_multi_perform(struct Curl_multi *multi) void Curl_conncache_multi_perform(struct Curl_multi *multi)

View File

@ -161,6 +161,7 @@ timediff_t Curl_shutdown_timeleft(struct connectdata *conn, int sockindex,
struct curltime *nowp) struct curltime *nowp)
{ {
struct curltime now; struct curltime now;
timediff_t left_ms;
if(!conn->shutdown.start[sockindex].tv_sec || !conn->shutdown.timeout_ms) if(!conn->shutdown.start[sockindex].tv_sec || !conn->shutdown.timeout_ms)
return 0; /* not started or no limits */ return 0; /* not started or no limits */
@ -169,8 +170,30 @@ timediff_t Curl_shutdown_timeleft(struct connectdata *conn, int sockindex,
now = Curl_now(); now = Curl_now();
nowp = &now; nowp = &now;
} }
return conn->shutdown.timeout_ms - left_ms = conn->shutdown.timeout_ms -
Curl_timediff(*nowp, conn->shutdown.start[sockindex]); Curl_timediff(*nowp, conn->shutdown.start[sockindex]);
return left_ms? left_ms : -1;
}
timediff_t Curl_conn_shutdown_timeleft(struct connectdata *conn,
struct curltime *nowp)
{
timediff_t left_ms = 0, ms;
struct curltime now;
int i;
for(i = 0; conn->shutdown.timeout_ms && (i < 2); ++i) {
if(!conn->shutdown.start[i].tv_sec)
continue;
if(!nowp) {
now = Curl_now();
nowp = &now;
}
ms = Curl_shutdown_timeleft(conn, i, nowp);
if(ms && (!left_ms || ms < left_ms))
left_ms = ms;
}
return left_ms;
} }
void Curl_shutdown_clear(struct Curl_easy *data, int sockindex) void Curl_shutdown_clear(struct Curl_easy *data, int sockindex)

View File

@ -46,10 +46,15 @@ void Curl_shutdown_start(struct Curl_easy *data, int sockindex,
struct curltime *nowp); struct curltime *nowp);
/* return how much time there is left to shutdown the connection at /* return how much time there is left to shutdown the connection at
* sockindex. */ * sockindex. Returns 0 if there is no limit or shutdown has not started. */
timediff_t Curl_shutdown_timeleft(struct connectdata *conn, int sockindex, timediff_t Curl_shutdown_timeleft(struct connectdata *conn, int sockindex,
struct curltime *nowp); struct curltime *nowp);
/* return how much time there is left to shutdown the connection.
* Returns 0 if there is no limit or shutdown has not started. */
timediff_t Curl_conn_shutdown_timeleft(struct connectdata *conn,
struct curltime *nowp);
void Curl_shutdown_clear(struct Curl_easy *data, int sockindex); void Curl_shutdown_clear(struct Curl_easy *data, int sockindex);
/* TRUE iff shutdown has been started */ /* TRUE iff shutdown has been started */

View File

@ -764,7 +764,8 @@ static CURLcode easy_perform(struct Curl_easy *data, bool events)
/* assign this after curl_multi_add_handle() */ /* assign this after curl_multi_add_handle() */
data->multi_easy = multi; data->multi_easy = multi;
sigpipe_ignore(data, &pipe_st); sigpipe_init(&pipe_st);
sigpipe_apply(data, &pipe_st);
/* run the transfer */ /* run the transfer */
result = events ? easy_events(multi) : easy_transfer(multi); result = events ? easy_events(multi) : easy_transfer(multi);

View File

@ -2714,6 +2714,7 @@ CURLMcode curl_multi_perform(struct Curl_multi *multi, int *running_handles)
CURLMcode returncode = CURLM_OK; CURLMcode returncode = CURLM_OK;
struct Curl_tree *t; struct Curl_tree *t;
struct curltime now = Curl_now(); struct curltime now = Curl_now();
SIGPIPE_VARIABLE(pipe_st);
if(!GOOD_MULTI_HANDLE(multi)) if(!GOOD_MULTI_HANDLE(multi))
return CURLM_BAD_HANDLE; return CURLM_BAD_HANDLE;
@ -2721,12 +2722,10 @@ CURLMcode curl_multi_perform(struct Curl_multi *multi, int *running_handles)
if(multi->in_callback) if(multi->in_callback)
return CURLM_RECURSIVE_API_CALL; return CURLM_RECURSIVE_API_CALL;
sigpipe_init(&pipe_st);
data = multi->easyp; data = multi->easyp;
if(data) { if(data) {
CURLMcode result; CURLMcode result;
bool nosig = data->set.no_signal;
SIGPIPE_VARIABLE(pipe_st);
sigpipe_ignore(data, &pipe_st);
/* Do the loop and only alter the signal ignore state if the next handle /* Do the loop and only alter the signal ignore state if the next handle
has a different NO_SIGNAL state than the previous */ has a different NO_SIGNAL state than the previous */
do { do {
@ -2734,22 +2733,23 @@ CURLMcode curl_multi_perform(struct Curl_multi *multi, int *running_handles)
pointer now */ pointer now */
struct Curl_easy *datanext = data->next; struct Curl_easy *datanext = data->next;
if(data->set.no_signal != nosig) { if(data != multi->conn_cache.closure_handle) {
sigpipe_restore(&pipe_st); /* connection cache handle is processed below */
sigpipe_ignore(data, &pipe_st); sigpipe_apply(data, &pipe_st);
nosig = data->set.no_signal;
}
result = multi_runsingle(multi, &now, data); result = multi_runsingle(multi, &now, data);
if(result) if(result)
returncode = result; returncode = result;
}
data = datanext; /* operate on next handle */ data = datanext; /* operate on next handle */
} while(data); } while(data);
sigpipe_restore(&pipe_st);
} }
sigpipe_apply(multi->conn_cache.closure_handle, &pipe_st);
Curl_conncache_multi_perform(multi); Curl_conncache_multi_perform(multi);
sigpipe_restore(&pipe_st);
/* /*
* Simply remove all expired timers from the splay since handles are dealt * Simply remove all expired timers from the splay since handles are dealt
* with unconditionally by this function and curl_multi_timeout() requires * with unconditionally by this function and curl_multi_timeout() requires
@ -3189,8 +3189,7 @@ static CURLMcode multi_socket(struct Curl_multi *multi,
struct Curl_easy *data = NULL; struct Curl_easy *data = NULL;
struct Curl_tree *t; struct Curl_tree *t;
struct curltime now = Curl_now(); struct curltime now = Curl_now();
bool first = FALSE; bool run_conn_cache = FALSE;
bool nosig = FALSE;
SIGPIPE_VARIABLE(pipe_st); SIGPIPE_VARIABLE(pipe_st);
if(checkall) { if(checkall) {
@ -3235,12 +3234,16 @@ static CURLMcode multi_socket(struct Curl_multi *multi,
DEBUGASSERT(data); DEBUGASSERT(data);
DEBUGASSERT(data->magic == CURLEASY_MAGIC_NUMBER); DEBUGASSERT(data->magic == CURLEASY_MAGIC_NUMBER);
if(data == multi->conn_cache.closure_handle)
run_conn_cache = TRUE;
else {
if(data->conn && !(data->conn->handler->flags & PROTOPT_DIRLOCK)) if(data->conn && !(data->conn->handler->flags & PROTOPT_DIRLOCK))
/* set socket event bitmask if they are not locked */ /* set socket event bitmask if they are not locked */
data->state.select_bits |= (unsigned char)ev_bitmask; data->state.select_bits |= (unsigned char)ev_bitmask;
Curl_expire(data, 0, EXPIRE_RUN_NOW); Curl_expire(data, 0, EXPIRE_RUN_NOW);
} }
}
/* Now we fall-through and do the timer-based stuff, since we do not want /* Now we fall-through and do the timer-based stuff, since we do not want
to force the user to have to deal with timeouts as long as at least to force the user to have to deal with timeouts as long as at least
@ -3266,19 +3269,13 @@ static CURLMcode multi_socket(struct Curl_multi *multi,
* to process in the splay and 'data' will be re-assigned for every expired * to process in the splay and 'data' will be re-assigned for every expired
* handle we deal with. * handle we deal with.
*/ */
sigpipe_init(&pipe_st);
do { do {
if(data == multi->conn_cache.closure_handle)
run_conn_cache = TRUE;
/* the first loop lap 'data' can be NULL */ /* the first loop lap 'data' can be NULL */
if(data) { else if(data) {
if(!first) { sigpipe_apply(data, &pipe_st);
first = TRUE;
nosig = data->set.no_signal; /* initial state */
sigpipe_ignore(data, &pipe_st);
}
else if(data->set.no_signal != nosig) {
sigpipe_restore(&pipe_st);
sigpipe_ignore(data, &pipe_st);
nosig = data->set.no_signal; /* remember new state */
}
result = multi_runsingle(multi, &now, data); result = multi_runsingle(multi, &now, data);
if(CURLM_OK >= result) { if(CURLM_OK >= result) {
@ -3300,7 +3297,12 @@ static CURLMcode multi_socket(struct Curl_multi *multi,
} }
} while(t); } while(t);
if(first)
if(run_conn_cache) {
sigpipe_apply(multi->conn_cache.closure_handle, &pipe_st);
Curl_conncache_multi_perform(multi);
}
sigpipe_restore(&pipe_st); sigpipe_restore(&pipe_st);
if(running_handles) if(running_handles)

View File

@ -36,6 +36,11 @@ struct sigpipe_ignore {
#define SIGPIPE_VARIABLE(x) struct sigpipe_ignore x #define SIGPIPE_VARIABLE(x) struct sigpipe_ignore x
static void sigpipe_init(struct sigpipe_ignore *ig)
{
memset(ig, 0, sizeof(*ig));
}
/* /*
* sigpipe_ignore() makes sure we ignore SIGPIPE while running libcurl * sigpipe_ignore() makes sure we ignore SIGPIPE while running libcurl
* internals, and then sigpipe_restore() will restore the situation when we * internals, and then sigpipe_restore() will restore the situation when we
@ -70,9 +75,20 @@ static void sigpipe_restore(struct sigpipe_ignore *ig)
sigaction(SIGPIPE, &ig->old_pipe_act, NULL); sigaction(SIGPIPE, &ig->old_pipe_act, NULL);
} }
static void sigpipe_apply(struct Curl_easy *data,
struct sigpipe_ignore *ig)
{
if(data->set.no_signal != ig->no_signal) {
sigpipe_restore(ig);
sigpipe_ignore(data, ig);
}
}
#else #else
/* for systems without sigaction */ /* for systems without sigaction */
#define sigpipe_ignore(x,y) Curl_nop_stmt #define sigpipe_ignore(x,y) Curl_nop_stmt
#define sigpipe_apply(x,y) Curl_nop_stmt
#define sigpipe_init(x) Curl_nop_stmt
#define sigpipe_restore(x) Curl_nop_stmt #define sigpipe_restore(x) Curl_nop_stmt
#define SIGPIPE_VARIABLE(x) #define SIGPIPE_VARIABLE(x)
#endif #endif