From 8706b680108919d9cdca1acd3a17fff6a57e245c Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Wed, 13 Dec 2023 11:25:20 +0100 Subject: [PATCH] lib: eliminate `conn->cselect_bits` - use `data->state.dselect_bits` everywhere instead - remove `bool *comeback` parameter as non-zero `data->state.dselect_bits` will indicate that IO is incomplete. Closes #12512 --- lib/cf-h2-proxy.c | 6 +++--- lib/easy.c | 2 +- lib/http2.c | 6 +++--- lib/imap.c | 2 +- lib/multi.c | 7 +++---- lib/transfer.c | 34 +++++++++------------------------- lib/transfer.h | 3 +-- lib/urldata.h | 3 +-- lib/vquic/curl_msh3.c | 8 ++++---- lib/vquic/curl_ngtcp2.c | 4 ++-- lib/vquic/curl_quiche.c | 4 ++-- lib/vssh/libssh.c | 8 ++++---- lib/vssh/libssh2.c | 8 ++++---- lib/vssh/wolfssh.c | 4 ++-- 14 files changed, 40 insertions(+), 59 deletions(-) diff --git a/lib/cf-h2-proxy.c b/lib/cf-h2-proxy.c index 147acdc86f..78902386ec 100644 --- a/lib/cf-h2-proxy.c +++ b/lib/cf-h2-proxy.c @@ -221,10 +221,10 @@ static void drain_tunnel(struct Curl_cfilter *cf, bits = CURL_CSELECT_IN; if(!tunnel->closed && !tunnel->reset && tunnel->upload_blocked_len) bits |= CURL_CSELECT_OUT; - if(data->state.dselect_bits != bits) { - CURL_TRC_CF(data, cf, "[%d] DRAIN dselect_bits=%x", + if(data->state.select_bits != bits) { + CURL_TRC_CF(data, cf, "[%d] DRAIN select_bits=%x", tunnel->stream_id, bits); - data->state.dselect_bits = bits; + data->state.select_bits = bits; Curl_expire(data, 0, EXPIRE_RUN_NOW); } } diff --git a/lib/easy.c b/lib/easy.c index 322d1a41b8..dae2afebf6 100644 --- a/lib/easy.c +++ b/lib/easy.c @@ -1111,7 +1111,7 @@ CURLcode curl_easy_pause(struct Curl_easy *data, int action) if(!data->state.tempcount) /* if not pausing again, force a recv/send check of this connection as the data might've been read off the socket already */ - data->conn->cselect_bits = CURL_CSELECT_IN | CURL_CSELECT_OUT; + data->state.select_bits = CURL_CSELECT_IN | CURL_CSELECT_OUT; if(data->multi) { if(Curl_update_timer(data->multi)) return CURLE_ABORTED_BY_CALLBACK; diff --git a/lib/http2.c b/lib/http2.c index 973848484a..59903cfa72 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -219,10 +219,10 @@ static void drain_stream(struct Curl_cfilter *cf, if(!stream->send_closed && (stream->upload_left || stream->upload_blocked_len)) bits |= CURL_CSELECT_OUT; - if(data->state.dselect_bits != bits) { - CURL_TRC_CF(data, cf, "[%d] DRAIN dselect_bits=%x", + if(data->state.select_bits != bits) { + CURL_TRC_CF(data, cf, "[%d] DRAIN select_bits=%x", stream->id, bits); - data->state.dselect_bits = bits; + data->state.select_bits = bits; Curl_expire(data, 0, EXPIRE_RUN_NOW); } } diff --git a/lib/imap.c b/lib/imap.c index 47cff4897c..1be89eccbd 100644 --- a/lib/imap.c +++ b/lib/imap.c @@ -1220,7 +1220,7 @@ static CURLcode imap_state_fetch_resp(struct Curl_easy *data, data->req.maxdownload = size; /* force a recv/send check of this connection, as the data might've been read off the socket already */ - data->conn->cselect_bits = CURL_CSELECT_IN; + data->state.select_bits = CURL_CSELECT_IN; Curl_setup_transfer(data, FIRSTSOCKET, size, FALSE, -1); } } diff --git a/lib/multi.c b/lib/multi.c index 5456113be7..53dee05e00 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -2371,7 +2371,6 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, { char *newurl = NULL; bool retry = FALSE; - bool comeback = FALSE; DEBUGASSERT(data->state.buffer); /* check if over send speed */ send_timeout_ms = 0; @@ -2402,7 +2401,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, } /* read/write data if it is ready to do so */ - result = Curl_readwrite(data->conn, data, &done, &comeback); + result = Curl_readwrite(data->conn, data, &done); if(done || (result == CURLE_RECV_ERROR)) { /* If CURLE_RECV_ERROR happens early enough, we assume it was a race @@ -2512,7 +2511,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, } } } - else if(comeback) { + else if(data->state.select_bits) { /* This avoids CURLM_CALL_MULTI_PERFORM so that a very fast transfer won't get stuck on this transfer at the expense of other concurrent transfers */ @@ -3164,7 +3163,7 @@ static CURLMcode multi_socket(struct Curl_multi *multi, if(data->conn && !(data->conn->handler->flags & PROTOPT_DIRLOCK)) /* set socket event bitmask if they're not locked */ - data->conn->cselect_bits = (unsigned char)ev_bitmask; + data->state.select_bits = (unsigned char)ev_bitmask; Curl_expire(data, 0, EXPIRE_RUN_NOW); } diff --git a/lib/transfer.c b/lib/transfer.c index 96f1fde755..d92b7edd41 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -417,15 +417,11 @@ bool Curl_meets_timecondition(struct Curl_easy *data, time_t timeofdoc) * Go ahead and do a read if we have a readable socket or if * the stream was rewound (in which case we have data in a * buffer) - * - * return '*comeback' TRUE if we didn't properly drain the socket so this - * function should get called again without select() or similar in between! */ static CURLcode readwrite_data(struct Curl_easy *data, struct connectdata *conn, struct SingleRequest *k, - int *didwhat, bool *done, - bool *comeback) + int *didwhat, bool *done) { CURLcode result = CURLE_OK; char *buf; @@ -438,7 +434,6 @@ static CURLcode readwrite_data(struct Curl_easy *data, DEBUGASSERT(data->state.buffer); *done = FALSE; - *comeback = FALSE; /* This is where we loop until we have read everything there is to read or we get a CURLE_AGAIN */ @@ -722,8 +717,7 @@ static CURLcode readwrite_data(struct Curl_easy *data, if(maxloops <= 0 || max_recv <= 0) { /* we mark it as read-again-please */ - data->state.dselect_bits = CURL_CSELECT_IN; - *comeback = TRUE; + data->state.select_bits = CURL_CSELECT_IN; } if(((k->keepon & (KEEP_RECV|KEEP_SEND)) == KEEP_SEND) && @@ -1032,14 +1026,10 @@ static int select_bits_paused(struct Curl_easy *data, int select_bits) /* * Curl_readwrite() is the low-level function to be called when data is to * be read and written to/from the connection. - * - * return '*comeback' TRUE if we didn't properly drain the socket so this - * function should get called again without select() or similar in between! */ CURLcode Curl_readwrite(struct connectdata *conn, struct Curl_easy *data, - bool *done, - bool *comeback) + bool *done) { struct SingleRequest *k = &data->req; CURLcode result; @@ -1047,22 +1037,16 @@ CURLcode Curl_readwrite(struct connectdata *conn, int didwhat = 0; int select_bits; - if(data->state.dselect_bits) { - if(select_bits_paused(data, data->state.dselect_bits)) { + if(data->state.select_bits) { + if(select_bits_paused(data, data->state.select_bits)) { /* leave the bits unchanged, so they'll tell us what to do when * this transfer gets unpaused. */ - DEBUGF(infof(data, "readwrite, dselect_bits, early return on PAUSED")); + DEBUGF(infof(data, "readwrite, select_bits, early return on PAUSED")); result = CURLE_OK; goto out; } - select_bits = data->state.dselect_bits; - data->state.dselect_bits = 0; - } - else if(conn->cselect_bits) { - /* CAVEAT: adding `select_bits_paused()` check here makes test640 hang - * (among others). Which hints at strange state handling in FTP land... */ - select_bits = conn->cselect_bits; - conn->cselect_bits = 0; + select_bits = data->state.select_bits; + data->state.select_bits = 0; } else { curl_socket_t fd_read; @@ -1100,7 +1084,7 @@ CURLcode Curl_readwrite(struct connectdata *conn, the stream was rewound (in which case we have data in a buffer) */ if((k->keepon & KEEP_RECV) && (select_bits & CURL_CSELECT_IN)) { - result = readwrite_data(data, conn, k, &didwhat, done, comeback); + result = readwrite_data(data, conn, k, &didwhat, done); if(result || *done) goto out; } diff --git a/lib/transfer.h b/lib/transfer.h index 536ac249b7..f1969a62a9 100644 --- a/lib/transfer.h +++ b/lib/transfer.h @@ -46,8 +46,7 @@ typedef enum { CURLcode Curl_follow(struct Curl_easy *data, char *newurl, followtype type); CURLcode Curl_readwrite(struct connectdata *conn, - struct Curl_easy *data, bool *done, - bool *comeback); + struct Curl_easy *data, bool *done); int Curl_single_getsock(struct Curl_easy *data, struct connectdata *conn, curl_socket_t *socks); CURLcode Curl_fillreadbuffer(struct Curl_easy *data, size_t bytes, diff --git a/lib/urldata.h b/lib/urldata.h index ff661482ea..14ddee2770 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -1103,7 +1103,6 @@ struct connectdata { unsigned short localport; unsigned short secondary_port; /* secondary socket remote port to connect to (ftp) */ - unsigned char cselect_bits; /* bitmask of socket events */ unsigned char alpn; /* APLN TLS negotiated protocol, a CURL_HTTP_VERSION* value */ #ifndef CURL_DISABLE_PROXY @@ -1479,7 +1478,7 @@ struct UrlState { server involved in this request */ unsigned char httpreq; /* Curl_HttpReq; what kind of HTTP request (if any) is this */ - unsigned char dselect_bits; /* != 0 -> bitmask of socket events for this + unsigned char select_bits; /* != 0 -> bitmask of socket events for this transfer overriding anything the socket may report */ #ifdef CURLDEBUG diff --git a/lib/vquic/curl_msh3.c b/lib/vquic/curl_msh3.c index 8ae3672400..7674bc1fcf 100644 --- a/lib/vquic/curl_msh3.c +++ b/lib/vquic/curl_msh3.c @@ -204,8 +204,8 @@ static void drain_stream_from_other_thread(struct Curl_easy *data, bits = CURL_CSELECT_IN; if(stream && !stream->upload_done) bits |= CURL_CSELECT_OUT; - if(data->state.dselect_bits != bits) { - data->state.dselect_bits = bits; + if(data->state.select_bits != bits) { + data->state.select_bits = bits; /* cannot expire from other thread */ } } @@ -220,8 +220,8 @@ static void drain_stream(struct Curl_cfilter *cf, bits = CURL_CSELECT_IN; if(stream && !stream->upload_done) bits |= CURL_CSELECT_OUT; - if(data->state.dselect_bits != bits) { - data->state.dselect_bits = bits; + if(data->state.select_bits != bits) { + data->state.select_bits = bits; Curl_expire(data, 0, EXPIRE_RUN_NOW); } } diff --git a/lib/vquic/curl_ngtcp2.c b/lib/vquic/curl_ngtcp2.c index f09b10bef7..0d0f2fab39 100644 --- a/lib/vquic/curl_ngtcp2.c +++ b/lib/vquic/curl_ngtcp2.c @@ -292,8 +292,8 @@ static void h3_drain_stream(struct Curl_cfilter *cf, bits = CURL_CSELECT_IN; if(stream && stream->upload_left && !stream->send_closed) bits |= CURL_CSELECT_OUT; - if(data->state.dselect_bits != bits) { - data->state.dselect_bits = bits; + if(data->state.select_bits != bits) { + data->state.select_bits = bits; Curl_expire(data, 0, EXPIRE_RUN_NOW); } } diff --git a/lib/vquic/curl_quiche.c b/lib/vquic/curl_quiche.c index 7123d63ca4..91395876b4 100644 --- a/lib/vquic/curl_quiche.c +++ b/lib/vquic/curl_quiche.c @@ -337,8 +337,8 @@ static void drain_stream(struct Curl_cfilter *cf, bits = CURL_CSELECT_IN; if(stream && !stream->send_closed && stream->upload_left) bits |= CURL_CSELECT_OUT; - if(data->state.dselect_bits != bits) { - data->state.dselect_bits = bits; + if(data->state.select_bits != bits) { + data->state.select_bits = bits; Curl_expire(data, 0, EXPIRE_RUN_NOW); } } diff --git a/lib/vssh/libssh.c b/lib/vssh/libssh.c index 97143c4776..fdb8d3e12e 100644 --- a/lib/vssh/libssh.c +++ b/lib/vssh/libssh.c @@ -1370,7 +1370,7 @@ static CURLcode myssh_statemach_act(struct Curl_easy *data, bool *block) /* we want to use the _sending_ function even when the socket turns out readable as the underlying libssh sftp send function will deal with both accordingly */ - conn->cselect_bits = CURL_CSELECT_OUT; + data->state.select_bits = CURL_CSELECT_OUT; /* since we don't really wait for anything at this point, we want the state machine to move on as soon as possible so we set a very short @@ -1740,7 +1740,7 @@ static CURLcode myssh_statemach_act(struct Curl_easy *data, bool *block) /* we want to use the _receiving_ function even when the socket turns out writableable as the underlying libssh recv function will deal with both accordingly */ - conn->cselect_bits = CURL_CSELECT_IN; + data->state.select_bits = CURL_CSELECT_IN; if(result) { /* this should never occur; the close state should be entered @@ -1868,7 +1868,7 @@ static CURLcode myssh_statemach_act(struct Curl_easy *data, bool *block) /* we want to use the _sending_ function even when the socket turns out readable as the underlying libssh scp send function will deal with both accordingly */ - conn->cselect_bits = CURL_CSELECT_OUT; + data->state.select_bits = CURL_CSELECT_OUT; state(data, SSH_STOP); @@ -1908,7 +1908,7 @@ static CURLcode myssh_statemach_act(struct Curl_easy *data, bool *block) /* we want to use the _receiving_ function even when the socket turns out writableable as the underlying libssh recv function will deal with both accordingly */ - conn->cselect_bits = CURL_CSELECT_IN; + data->state.select_bits = CURL_CSELECT_IN; state(data, SSH_STOP); break; diff --git a/lib/vssh/libssh2.c b/lib/vssh/libssh2.c index 11f5f4fd5d..2eaddb2440 100644 --- a/lib/vssh/libssh2.c +++ b/lib/vssh/libssh2.c @@ -2213,7 +2213,7 @@ static CURLcode ssh_statemach_act(struct Curl_easy *data, bool *block) /* we want to use the _sending_ function even when the socket turns out readable as the underlying libssh2 sftp send function will deal with both accordingly */ - conn->cselect_bits = CURL_CSELECT_OUT; + data->state.select_bits = CURL_CSELECT_OUT; /* since we don't really wait for anything at this point, we want the state machine to move on as soon as possible so we set a very short @@ -2602,7 +2602,7 @@ static CURLcode ssh_statemach_act(struct Curl_easy *data, bool *block) /* we want to use the _receiving_ function even when the socket turns out writableable as the underlying libssh2 recv function will deal with both accordingly */ - conn->cselect_bits = CURL_CSELECT_IN; + data->state.select_bits = CURL_CSELECT_IN; if(result) { /* this should never occur; the close state should be entered @@ -2757,7 +2757,7 @@ static CURLcode ssh_statemach_act(struct Curl_easy *data, bool *block) /* we want to use the _sending_ function even when the socket turns out readable as the underlying libssh2 scp send function will deal with both accordingly */ - conn->cselect_bits = CURL_CSELECT_OUT; + data->state.select_bits = CURL_CSELECT_OUT; state(data, SSH_STOP); } @@ -2819,7 +2819,7 @@ static CURLcode ssh_statemach_act(struct Curl_easy *data, bool *block) /* we want to use the _receiving_ function even when the socket turns out writableable as the underlying libssh2 recv function will deal with both accordingly */ - conn->cselect_bits = CURL_CSELECT_IN; + data->state.select_bits = CURL_CSELECT_IN; if(result) { state(data, SSH_SCP_CHANNEL_FREE); diff --git a/lib/vssh/wolfssh.c b/lib/vssh/wolfssh.c index 2fca327077..368f163d1d 100644 --- a/lib/vssh/wolfssh.c +++ b/lib/vssh/wolfssh.c @@ -694,7 +694,7 @@ static CURLcode wssh_statemach_act(struct Curl_easy *data, bool *block) /* we want to use the _sending_ function even when the socket turns out readable as the underlying libssh2 sftp send function will deal with both accordingly */ - conn->cselect_bits = CURL_CSELECT_OUT; + data->state.select_bits = CURL_CSELECT_OUT; /* since we don't really wait for anything at this point, we want the state machine to move on as soon as possible so we set a very short @@ -790,7 +790,7 @@ static CURLcode wssh_statemach_act(struct Curl_easy *data, bool *block) /* we want to use the _receiving_ function even when the socket turns out writableable as the underlying libssh2 recv function will deal with both accordingly */ - conn->cselect_bits = CURL_CSELECT_IN; + data->state.select_bits = CURL_CSELECT_IN; if(result) { /* this should never occur; the close state should be entered