Skip to content

Commit

Permalink
multi: introduce SETUP state for better timeouts
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@bagder
bagder committed Apr 15, 2024
1 parent a1ec035 commit 8af03f8
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 58 deletions.
115 changes: 74 additions & 41 deletions lib/multi.c
View file
Expand Up @@ -86,6 +86,8 @@
((x) && (x)->magic == CURL_MULTI_HANDLE)
#endif

static void move_pending_to_connect(struct Curl_multi *multi,
struct Curl_easy *data);
static CURLMcode singlesocket(struct Curl_multi *multi,
struct Curl_easy *data);
static CURLMcode add_next_timeout(struct curltime now,
Expand All @@ -100,6 +102,7 @@ static void multi_xfer_bufs_free(struct Curl_multi *multi);
static const char * const multi_statename[]={
"INIT",
"PENDING",
"SETUP",
"CONNECT",
"RESOLVING",
"CONNECTING",
Expand Down Expand Up @@ -149,6 +152,7 @@ static void mstate(struct Curl_easy *data, CURLMstate state
static const init_multistate_func finit[MSTATE_LAST] = {
NULL, /* INIT */
NULL, /* PENDING */
NULL, /* SETUP */
Curl_init_CONNECT, /* CONNECT */
NULL, /* RESOLVING */
NULL, /* CONNECTING */
Expand Down Expand Up @@ -1110,6 +1114,7 @@ static void multi_getsock(struct Curl_easy *data,
switch(data->mstate) {
case MSTATE_INIT:
case MSTATE_PENDING:
case MSTATE_SETUP:
case MSTATE_CONNECT:
/* nothing to poll for yet */
break;
Expand Down Expand Up @@ -1745,7 +1750,6 @@ static bool multi_handle_timeout(struct Curl_easy *data,
bool connect_timeout)
{
timediff_t timeout_ms = Curl_timeleft(data, now, connect_timeout);

if(timeout_ms < 0) {
/* Handle timed out */
struct curltime since;
Expand Down Expand Up @@ -1952,31 +1956,39 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,

switch(data->mstate) {
case MSTATE_INIT:
/* init this transfer. */
/* Transitional state. init this transfer. A handle never comes
back to this state. */
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)
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);
if(data->set.timeout)
Curl_expire(data, data->set.timeout, EXPIRE_TIMEOUT);

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

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);
if(CURLE_NO_CONNECTION_AVAILABLE == result) {
/* There was no connection available. We will go to the pending
Expand All @@ -1990,11 +2002,8 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
result = CURLE_OK;
break;
}
else if(data->state.previouslypending) {
/* this transfer comes from the pending queue so try move another */
infof(data, "Transfer was pending, now try another");
else
process_pending_handles(data->multi);
}

if(!result) {
*nowp = Curl_pgrsTime(data, TIMER_POSTQUEUE);
Expand Down Expand Up @@ -2275,7 +2284,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
follow = FOLLOW_RETRY;
drc = Curl_follow(data, newurl, follow);
if(!drc) {
multistate(data, MSTATE_CONNECT);
multistate(data, MSTATE_SETUP);
rc = CURLM_CALL_MULTI_PERFORM;
result = CURLE_OK;
}
Expand Down Expand Up @@ -2535,7 +2544,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
/* multi_done() might return CURLE_GOT_NOTHING */
result = Curl_follow(data, newurl, follow);
if(!result) {
multistate(data, MSTATE_CONNECT);
multistate(data, MSTATE_SETUP);
rc = CURLM_CALL_MULTI_PERFORM;
}
free(newurl);
Expand Down Expand Up @@ -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
* declaring the connection timed out as we may almost have a completed
* connection. */
multi_handle_timeout(data, nowp, &stream_error, &result, TRUE);
multi_handle_timeout(data, nowp, &stream_error, &result, FALSE);
}

statemachine_end:
Expand Down Expand Up @@ -2768,10 +2777,20 @@ CURLMcode curl_multi_perform(struct Curl_multi *multi, int *running_handles)
*/
do {
multi->timetree = Curl_splaygetbest(now, multi->timetree, &t);
if(t)
if(t) {
/* 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);

}
} while(t);

*running_handles = multi->num_alive;
Expand Down Expand Up @@ -3725,29 +3744,43 @@ void Curl_multiuse_state(struct Curl_easy *data,
process_pending_handles(data->multi);
}

/* process_pending_handles() moves all handles from PENDING
back into the main list and change state to CONNECT */
static void process_pending_handles(struct Curl_multi *multi)
static void move_pending_to_connect(struct Curl_multi *multi,
struct Curl_easy *data)
{
struct Curl_llist_element *e = multi->pending.head;
if(e) {
struct Curl_easy *data = e->ptr;
DEBUGASSERT(data->mstate == MSTATE_PENDING);

/* put it back into the main list */
link_easy(multi, data);

DEBUGASSERT(data->mstate == MSTATE_PENDING);
multistate(data, MSTATE_CONNECT);

/* put it back into the main list */
link_easy(multi, data);
/* Remove this node from the pending list */
Curl_llist_remove(&multi->pending, &data->connect_queue, NULL);

multistate(data, MSTATE_CONNECT);
/* Make sure that the handle will be processed soonish. */
Curl_expire(data, 0, EXPIRE_RUN_NOW);
}

/* Remove this node from the list */
Curl_llist_remove(&multi->pending, e, NULL);
/* process_pending_handles() moves a handle from PENDING back into the main
list and change state to CONNECT.
/* Make sure that the handle will be processed soonish. */
Curl_expire(data, 0, EXPIRE_RUN_NOW);
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.
/* mark this as having been in the pending queue */
data->state.previouslypending = TRUE;
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);
}
}

Expand Down
33 changes: 17 additions & 16 deletions lib/multihandle.h
View file
Expand Up @@ -44,24 +44,25 @@ struct Curl_message {
typedef enum {
MSTATE_INIT, /* 0 - start in this state */
MSTATE_PENDING, /* 1 - no connections, waiting for one */
MSTATE_CONNECT, /* 2 - resolve/connect has been sent off */
MSTATE_RESOLVING, /* 3 - awaiting the resolve to finalize */
MSTATE_CONNECTING, /* 4 - awaiting the TCP connect to finalize */
MSTATE_TUNNELING, /* 5 - awaiting HTTPS proxy SSL initialization to
MSTATE_SETUP, /* 2 - start a new transfer */
MSTATE_CONNECT, /* 3 - resolve/connect has been sent off */
MSTATE_RESOLVING, /* 4 - awaiting the resolve to finalize */
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 */
MSTATE_PROTOCONNECT, /* 6 - initiate protocol connect procedure */
MSTATE_PROTOCONNECTING, /* 7 - completing the protocol-specific connect
MSTATE_PROTOCONNECT, /* 7 - initiate protocol connect procedure */
MSTATE_PROTOCONNECTING, /* 8 - completing the protocol-specific connect
phase */
MSTATE_DO, /* 8 - start send off the request (part 1) */
MSTATE_DOING, /* 9 - sending off the request (part 1) */
MSTATE_DOING_MORE, /* 10 - send off the request (part 2) */
MSTATE_DID, /* 11 - done sending off request */
MSTATE_PERFORMING, /* 12 - transfer data */
MSTATE_RATELIMITING, /* 13 - wait because limit-rate exceeded */
MSTATE_DONE, /* 14 - post data transfer operation */
MSTATE_COMPLETED, /* 15 - operation complete */
MSTATE_MSGSENT, /* 16 - the operation complete message is sent */
MSTATE_LAST /* 17 - not a true state, never use this */
MSTATE_DO, /* 9 - start send off the request (part 1) */
MSTATE_DOING, /* 10 - sending off the request (part 1) */
MSTATE_DOING_MORE, /* 11 - send off the request (part 2) */
MSTATE_DID, /* 12 - done sending off request */
MSTATE_PERFORMING, /* 13 - transfer data */
MSTATE_RATELIMITING, /* 14 - wait because limit-rate exceeded */
MSTATE_DONE, /* 15 - post data transfer operation */
MSTATE_COMPLETED, /* 16 - operation complete */
MSTATE_MSGSENT, /* 17 - the operation complete message is sent */
MSTATE_LAST /* 18 - not a true state, never use this */
} CURLMstate;

/* we support N sockets per easy handle. Set the corresponding bit to what
Expand Down
1 change: 0 additions & 1 deletion lib/urldata.h
View file
Expand Up @@ -1386,7 +1386,6 @@ struct UrlState {
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
invoked twice when the multi interface is used. */
BIT(previouslypending); /* this transfer WAS in the multi->pending queue */
#ifndef CURL_DISABLE_COOKIES
BIT(cookie_engine);
#endif
Expand Down

0 comments on commit 8af03f8

Please sign in to comment.