multi: introduce SETUP state for better timeouts

Since we can go to the CONNECT state from PENDING, potentially multiple
times for a single transfer, this change introdues a SETUP state that
happens before CONNECT when doing a new transfer.

Now, doing a redirect on a handle goes back to SETUP (not CONNECT like
before) and we initilize the connect timeout etc in SETUP. Previously,
we would do it in CONNECT but that would make it unreliable in cases
where a transfer goes in and out between CONNECT and PENDING multiple
times.

SETUP is transient, so the handle never actually stays in that state.

Additionally: take care of timeouts of PENDING transfers in
curl_multi_perform()

Ref: #13227
Closes #13371
This commit is contained in:
Daniel Stenberg 2024-04-05 13:07:16 +02:00
parent 6eb9e65781
commit be659030ba
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
3 changed files with 93 additions and 60 deletions

View File

@ -86,6 +86,8 @@
((x) && (x)->magic == CURL_MULTI_HANDLE) ((x) && (x)->magic == CURL_MULTI_HANDLE)
#endif #endif
static void move_pending_to_connect(struct Curl_multi *multi,
struct Curl_easy *data);
static CURLMcode singlesocket(struct Curl_multi *multi, static CURLMcode singlesocket(struct Curl_multi *multi,
struct Curl_easy *data); struct Curl_easy *data);
static CURLMcode add_next_timeout(struct curltime now, static CURLMcode add_next_timeout(struct curltime now,
@ -100,6 +102,7 @@ static void multi_xfer_bufs_free(struct Curl_multi *multi);
static const char * const multi_statename[]={ static const char * const multi_statename[]={
"INIT", "INIT",
"PENDING", "PENDING",
"SETUP",
"CONNECT", "CONNECT",
"RESOLVING", "RESOLVING",
"CONNECTING", "CONNECTING",
@ -149,6 +152,7 @@ static void mstate(struct Curl_easy *data, CURLMstate state
static const init_multistate_func finit[MSTATE_LAST] = { static const init_multistate_func finit[MSTATE_LAST] = {
NULL, /* INIT */ NULL, /* INIT */
NULL, /* PENDING */ NULL, /* PENDING */
NULL, /* SETUP */
Curl_init_CONNECT, /* CONNECT */ Curl_init_CONNECT, /* CONNECT */
NULL, /* RESOLVING */ NULL, /* RESOLVING */
NULL, /* CONNECTING */ NULL, /* CONNECTING */
@ -1110,6 +1114,7 @@ static void multi_getsock(struct Curl_easy *data,
switch(data->mstate) { switch(data->mstate) {
case MSTATE_INIT: case MSTATE_INIT:
case MSTATE_PENDING: case MSTATE_PENDING:
case MSTATE_SETUP:
case MSTATE_CONNECT: case MSTATE_CONNECT:
/* nothing to poll for yet */ /* nothing to poll for yet */
break; break;
@ -1745,7 +1750,6 @@ static bool multi_handle_timeout(struct Curl_easy *data,
bool connect_timeout) bool connect_timeout)
{ {
timediff_t timeout_ms = Curl_timeleft(data, now, connect_timeout); timediff_t timeout_ms = Curl_timeleft(data, now, connect_timeout);
if(timeout_ms < 0) { if(timeout_ms < 0) {
/* Handle timed out */ /* Handle timed out */
struct curltime since; struct curltime since;
@ -1952,31 +1956,39 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
switch(data->mstate) { switch(data->mstate) {
case MSTATE_INIT: case MSTATE_INIT:
/* init this transfer. */ /* Transitional state. init this transfer. A handle never comes
back to this state. */
result = Curl_pretransfer(data); result = Curl_pretransfer(data);
if(!result) {
/* after init, go CONNECT */
multistate(data, MSTATE_CONNECT);
*nowp = Curl_pgrsTime(data, TIMER_STARTOP);
rc = CURLM_CALL_MULTI_PERFORM;
}
break;
case MSTATE_CONNECT:
/* Connect. We want to get a connection identifier filled in. */
/* init this transfer. */
result = Curl_preconnect(data);
if(result) if(result)
break; break;
/* after init, go SETUP */
multistate(data, MSTATE_SETUP);
(void)Curl_pgrsTime(data, TIMER_STARTOP);
FALLTHROUGH();
case MSTATE_SETUP:
/* Transitional state. Setup things for a new transfer. The handle
can come back to this state on a redirect. */
*nowp = Curl_pgrsTime(data, TIMER_STARTSINGLE); *nowp = Curl_pgrsTime(data, TIMER_STARTSINGLE);
if(data->set.timeout) if(data->set.timeout)
Curl_expire(data, data->set.timeout, EXPIRE_TIMEOUT); Curl_expire(data, data->set.timeout, EXPIRE_TIMEOUT);
if(data->set.connecttimeout) if(data->set.connecttimeout)
/* Since a connection might go to pending and back to CONNECT several
times before it actually takes off, we need to set the timeout once
in SETUP before we enter CONNECT the first time. */
Curl_expire(data, data->set.connecttimeout, EXPIRE_CONNECTTIMEOUT); Curl_expire(data, data->set.connecttimeout, EXPIRE_CONNECTTIMEOUT);
multistate(data, MSTATE_CONNECT);
FALLTHROUGH();
case MSTATE_CONNECT:
/* Connect. We want to get a connection identifier filled in. This state
can be entered from SETUP and from PENDING. */
result = Curl_preconnect(data);
if(result)
break;
result = Curl_connect(data, &async, &connected); result = Curl_connect(data, &async, &connected);
if(CURLE_NO_CONNECTION_AVAILABLE == result) { if(CURLE_NO_CONNECTION_AVAILABLE == result) {
/* There was no connection available. We will go to the pending /* There was no connection available. We will go to the pending
@ -1990,11 +2002,8 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
result = CURLE_OK; result = CURLE_OK;
break; break;
} }
else if(data->state.previouslypending) { else
/* this transfer comes from the pending queue so try move another */
infof(data, "Transfer was pending, now try another");
process_pending_handles(data->multi); process_pending_handles(data->multi);
}
if(!result) { if(!result) {
*nowp = Curl_pgrsTime(data, TIMER_POSTQUEUE); *nowp = Curl_pgrsTime(data, TIMER_POSTQUEUE);
@ -2275,7 +2284,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
follow = FOLLOW_RETRY; follow = FOLLOW_RETRY;
drc = Curl_follow(data, newurl, follow); drc = Curl_follow(data, newurl, follow);
if(!drc) { if(!drc) {
multistate(data, MSTATE_CONNECT); multistate(data, MSTATE_SETUP);
rc = CURLM_CALL_MULTI_PERFORM; rc = CURLM_CALL_MULTI_PERFORM;
result = CURLE_OK; result = CURLE_OK;
} }
@ -2535,7 +2544,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
/* multi_done() might return CURLE_GOT_NOTHING */ /* multi_done() might return CURLE_GOT_NOTHING */
result = Curl_follow(data, newurl, follow); result = Curl_follow(data, newurl, follow);
if(!result) { if(!result) {
multistate(data, MSTATE_CONNECT); multistate(data, MSTATE_SETUP);
rc = CURLM_CALL_MULTI_PERFORM; rc = CURLM_CALL_MULTI_PERFORM;
} }
free(newurl); free(newurl);
@ -2629,7 +2638,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
* (i.e. CURLM_CALL_MULTI_PERFORM == TRUE) then we should do that before * (i.e. CURLM_CALL_MULTI_PERFORM == TRUE) then we should do that before
* declaring the connection timed out as we may almost have a completed * declaring the connection timed out as we may almost have a completed
* connection. */ * connection. */
multi_handle_timeout(data, nowp, &stream_error, &result, TRUE); multi_handle_timeout(data, nowp, &stream_error, &result, FALSE);
} }
statemachine_end: statemachine_end:
@ -2768,10 +2777,20 @@ CURLMcode curl_multi_perform(struct Curl_multi *multi, int *running_handles)
*/ */
do { do {
multi->timetree = Curl_splaygetbest(now, multi->timetree, &t); multi->timetree = Curl_splaygetbest(now, multi->timetree, &t);
if(t) if(t) {
/* the removed may have another timeout in queue */ /* the removed may have another timeout in queue */
data = t->payload;
if(data->mstate == MSTATE_PENDING) {
bool stream_unused;
CURLcode result_unused;
if(multi_handle_timeout(data, &now, &stream_unused, &result_unused,
FALSE)) {
infof(data, "PENDING handle timeout");
move_pending_to_connect(multi, data);
}
}
(void)add_next_timeout(now, multi, t->payload); (void)add_next_timeout(now, multi, t->payload);
}
} while(t); } while(t);
*running_handles = multi->num_alive; *running_handles = multi->num_alive;
@ -3725,14 +3744,9 @@ void Curl_multiuse_state(struct Curl_easy *data,
process_pending_handles(data->multi); process_pending_handles(data->multi);
} }
/* process_pending_handles() moves all handles from PENDING static void move_pending_to_connect(struct Curl_multi *multi,
back into the main list and change state to CONNECT */ struct Curl_easy *data)
static void process_pending_handles(struct Curl_multi *multi)
{ {
struct Curl_llist_element *e = multi->pending.head;
if(e) {
struct Curl_easy *data = e->ptr;
DEBUGASSERT(data->mstate == MSTATE_PENDING); DEBUGASSERT(data->mstate == MSTATE_PENDING);
/* put it back into the main list */ /* put it back into the main list */
@ -3740,14 +3754,33 @@ static void process_pending_handles(struct Curl_multi *multi)
multistate(data, MSTATE_CONNECT); multistate(data, MSTATE_CONNECT);
/* Remove this node from the list */ /* Remove this node from the pending list */
Curl_llist_remove(&multi->pending, e, NULL); Curl_llist_remove(&multi->pending, &data->connect_queue, NULL);
/* Make sure that the handle will be processed soonish. */ /* Make sure that the handle will be processed soonish. */
Curl_expire(data, 0, EXPIRE_RUN_NOW); Curl_expire(data, 0, EXPIRE_RUN_NOW);
}
/* mark this as having been in the pending queue */ /* process_pending_handles() moves a handle from PENDING back into the main
data->state.previouslypending = TRUE; list and change state to CONNECT.
We do not move all transfers because that can be a significant amount.
Since this is tried every now and then doing too many too often becomes a
performance problem.
When there is a change for connection limits like max host connections etc,
this likely only allows one new transfer. When there is a pipewait change,
it can potentially allow hundreds of new transfers.
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)
{
struct Curl_llist_element *e = multi->pending.head;
if(e) {
struct Curl_easy *data = e->ptr;
move_pending_to_connect(multi, data);
} }
} }

View File

@ -44,24 +44,25 @@ struct Curl_message {
typedef enum { typedef enum {
MSTATE_INIT, /* 0 - start in this state */ MSTATE_INIT, /* 0 - start in this state */
MSTATE_PENDING, /* 1 - no connections, waiting for one */ MSTATE_PENDING, /* 1 - no connections, waiting for one */
MSTATE_CONNECT, /* 2 - resolve/connect has been sent off */ MSTATE_SETUP, /* 2 - start a new transfer */
MSTATE_RESOLVING, /* 3 - awaiting the resolve to finalize */ MSTATE_CONNECT, /* 3 - resolve/connect has been sent off */
MSTATE_CONNECTING, /* 4 - awaiting the TCP connect to finalize */ MSTATE_RESOLVING, /* 4 - awaiting the resolve to finalize */
MSTATE_TUNNELING, /* 5 - awaiting HTTPS proxy SSL initialization to MSTATE_CONNECTING, /* 5 - awaiting the TCP connect to finalize */
MSTATE_TUNNELING, /* 6 - awaiting HTTPS proxy SSL initialization to
complete and/or proxy CONNECT to finalize */ complete and/or proxy CONNECT to finalize */
MSTATE_PROTOCONNECT, /* 6 - initiate protocol connect procedure */ MSTATE_PROTOCONNECT, /* 7 - initiate protocol connect procedure */
MSTATE_PROTOCONNECTING, /* 7 - completing the protocol-specific connect MSTATE_PROTOCONNECTING, /* 8 - completing the protocol-specific connect
phase */ phase */
MSTATE_DO, /* 8 - start send off the request (part 1) */ MSTATE_DO, /* 9 - start send off the request (part 1) */
MSTATE_DOING, /* 9 - sending off the request (part 1) */ MSTATE_DOING, /* 10 - sending off the request (part 1) */
MSTATE_DOING_MORE, /* 10 - send off the request (part 2) */ MSTATE_DOING_MORE, /* 11 - send off the request (part 2) */
MSTATE_DID, /* 11 - done sending off request */ MSTATE_DID, /* 12 - done sending off request */
MSTATE_PERFORMING, /* 12 - transfer data */ MSTATE_PERFORMING, /* 13 - transfer data */
MSTATE_RATELIMITING, /* 13 - wait because limit-rate exceeded */ MSTATE_RATELIMITING, /* 14 - wait because limit-rate exceeded */
MSTATE_DONE, /* 14 - post data transfer operation */ MSTATE_DONE, /* 15 - post data transfer operation */
MSTATE_COMPLETED, /* 15 - operation complete */ MSTATE_COMPLETED, /* 16 - operation complete */
MSTATE_MSGSENT, /* 16 - the operation complete message is sent */ MSTATE_MSGSENT, /* 17 - the operation complete message is sent */
MSTATE_LAST /* 17 - not a true state, never use this */ MSTATE_LAST /* 18 - not a true state, never use this */
} CURLMstate; } CURLMstate;
/* we support N sockets per easy handle. Set the corresponding bit to what /* we support N sockets per easy handle. Set the corresponding bit to what

View File

@ -1386,7 +1386,6 @@ struct UrlState {
BIT(done); /* set to FALSE when Curl_init_do() is called and set to TRUE BIT(done); /* set to FALSE when Curl_init_do() is called and set to TRUE
when multi_done() is called, to prevent multi_done() to get when multi_done() is called, to prevent multi_done() to get
invoked twice when the multi interface is used. */ invoked twice when the multi interface is used. */
BIT(previouslypending); /* this transfer WAS in the multi->pending queue */
#ifndef CURL_DISABLE_COOKIES #ifndef CURL_DISABLE_COOKIES
BIT(cookie_engine); BIT(cookie_engine);
#endif #endif