From a0f94800d507de580f04a447cf14edc9f52697dc Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Fri, 5 Jan 2024 12:28:09 +0100 Subject: [PATCH] transfer: adjust_pollset improvements - let `multi_getsock()` initialize the pollset in what the transfer state requires in regards to SEND/RECV - change connection filters `adjust_pollset()` implementation to react on the presence of POLLIN/-OUT in the pollset and no longer check CURL_WANT_SEND/CURL_WANT_RECV - cf-socket will no longer add POLLIN on its own - http2 and http/3 filters will only do adjustments if the passed pollset wants to POLLIN/OUT for the transfer on the socket. This is similar to the HTTP/2 proxy filter and works in stacked filters. Closes #12640 --- lib/cf-socket.c | 2 +- lib/cfilters.c | 14 ----- lib/cfilters.h | 5 -- lib/http2.c | 12 ++-- lib/multi.c | 131 +++++++++++++++++++++++++++++++--------- lib/transfer.c | 46 -------------- lib/transfer.h | 2 - lib/urldata.h | 2 +- lib/vquic/curl_ngtcp2.c | 9 ++- lib/vquic/curl_quiche.c | 9 ++- 10 files changed, 125 insertions(+), 107 deletions(-) diff --git a/lib/cf-socket.c b/lib/cf-socket.c index c86aa7e7c..2e985a77f 100644 --- a/lib/cf-socket.c +++ b/lib/cf-socket.c @@ -1243,7 +1243,7 @@ static void cf_socket_adjust_pollset(struct Curl_cfilter *cf, if(ctx->sock != CURL_SOCKET_BAD) { if(!cf->connected) Curl_pollset_set_out_only(data, ps, ctx->sock); - else if(CURL_WANT_RECV(data)) + else if(!ctx->active) Curl_pollset_add_in(data, ps, ctx->sock); CURL_TRC_CF(data, cf, "adjust_pollset -> %d socks", ps->num); } diff --git a/lib/cfilters.c b/lib/cfilters.c index e78ecd71d..823e90c3f 100644 --- a/lib/cfilters.c +++ b/lib/cfilters.c @@ -760,25 +760,11 @@ static void ps_add(struct Curl_easy *data, struct easy_pollset *ps, void Curl_pollset_add_socks(struct Curl_easy *data, struct easy_pollset *ps, int (*get_socks_cb)(struct Curl_easy *data, - struct connectdata *conn, curl_socket_t *socks)) { curl_socket_t socks[MAX_SOCKSPEREASYHANDLE]; int bitmap; - DEBUGASSERT(data->conn); - bitmap = get_socks_cb(data, data->conn, socks); - ps_add(data, ps, bitmap, socks); -} - -void Curl_pollset_add_socks2(struct Curl_easy *data, - struct easy_pollset *ps, - int (*get_socks_cb)(struct Curl_easy *data, - curl_socket_t *socks)) -{ - curl_socket_t socks[MAX_SOCKSPEREASYHANDLE]; - int bitmap; - bitmap = get_socks_cb(data, socks); ps_add(data, ps, bitmap, socks); } diff --git a/lib/cfilters.h b/lib/cfilters.h index 09a3f162a..f83842920 100644 --- a/lib/cfilters.h +++ b/lib/cfilters.h @@ -530,12 +530,7 @@ void Curl_pollset_set(struct Curl_easy *data, void Curl_pollset_add_socks(struct Curl_easy *data, struct easy_pollset *ps, int (*get_socks_cb)(struct Curl_easy *data, - struct connectdata *conn, curl_socket_t *socks)); -void Curl_pollset_add_socks2(struct Curl_easy *data, - struct easy_pollset *ps, - int (*get_socks_cb)(struct Curl_easy *data, - curl_socket_t *socks)); /** * Check if the pollset, as is, wants to read and/or write regarding diff --git a/lib/http2.c b/lib/http2.c index b7a086079..c3157d1ef 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -2331,12 +2331,16 @@ static void cf_h2_adjust_pollset(struct Curl_cfilter *cf, struct easy_pollset *ps) { struct cf_h2_ctx *ctx = cf->ctx; - bool want_recv = CURL_WANT_RECV(data); - bool want_send = CURL_WANT_SEND(data); + curl_socket_t sock; + bool want_recv, want_send; - if(ctx->h2 && (want_recv || want_send)) { + if(!ctx->h2) + return; + + sock = Curl_conn_cf_get_socket(cf, data); + Curl_pollset_check(data, ps, sock, &want_recv, &want_send); + if(want_recv || want_send) { struct stream_ctx *stream = H2_STREAM_CTX(data); - curl_socket_t sock = Curl_conn_cf_get_socket(cf, data); struct cf_call_data save; bool c_exhaust, s_exhaust; diff --git a/lib/multi.c b/lib/multi.c index 9c7cde8f1..e4491d912 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -994,31 +994,90 @@ void Curl_attach_connection(struct Curl_easy *data, Curl_conn_ev_data_attach(conn, data); } -static int domore_getsock(struct Curl_easy *data, - struct connectdata *conn, - curl_socket_t *socks) +static int connecting_getsock(struct Curl_easy *data, curl_socket_t *socks) { + struct connectdata *conn = data->conn; + (void)socks; + if(conn && conn->sockfd != CURL_SOCKET_BAD) { + /* Default is to wait to something from the server */ + socks[0] = conn->sockfd; + return GETSOCK_READSOCK(0); + } + return GETSOCK_BLANK; +} + +static int protocol_getsock(struct Curl_easy *data, curl_socket_t *socks) +{ + struct connectdata *conn = data->conn; + if(conn && conn->handler->proto_getsock) + return conn->handler->proto_getsock(data, conn, socks); + else if(conn && conn->sockfd != CURL_SOCKET_BAD) { + /* Default is to wait to something from the server */ + socks[0] = conn->sockfd; + return GETSOCK_READSOCK(0); + } + return GETSOCK_BLANK; +} + +static int domore_getsock(struct Curl_easy *data, curl_socket_t *socks) +{ + struct connectdata *conn = data->conn; if(conn && conn->handler->domore_getsock) return conn->handler->domore_getsock(data, conn, socks); + else if(conn && conn->sockfd != CURL_SOCKET_BAD) { + /* Default is that we want to send something to the server */ + socks[0] = conn->sockfd; + return GETSOCK_WRITESOCK(0); + } return GETSOCK_BLANK; } -static int doing_getsock(struct Curl_easy *data, - struct connectdata *conn, - curl_socket_t *socks) +static int doing_getsock(struct Curl_easy *data, curl_socket_t *socks) { + struct connectdata *conn = data->conn; if(conn && conn->handler->doing_getsock) return conn->handler->doing_getsock(data, conn, socks); + else if(conn && conn->sockfd != CURL_SOCKET_BAD) { + /* Default is that we want to send something to the server */ + socks[0] = conn->sockfd; + return GETSOCK_WRITESOCK(0); + } return GETSOCK_BLANK; } -static int protocol_getsock(struct Curl_easy *data, - struct connectdata *conn, - curl_socket_t *socks) +static int perform_getsock(struct Curl_easy *data, curl_socket_t *sock) { - if(conn->handler->proto_getsock) - return conn->handler->proto_getsock(data, conn, socks); - return GETSOCK_BLANK; + struct connectdata *conn = data->conn; + + if(!conn) + return GETSOCK_BLANK; + else if(conn->handler->perform_getsock) + return conn->handler->perform_getsock(data, conn, sock); + else { + /* Default is to obey the data->req.keepon flags for send/recv */ + int bitmap = GETSOCK_BLANK; + unsigned sockindex = 0; + if(CURL_WANT_RECV(data)) { + DEBUGASSERT(conn->sockfd != CURL_SOCKET_BAD); + bitmap |= GETSOCK_READSOCK(sockindex); + sock[sockindex] = conn->sockfd; + } + + if(CURL_WANT_SEND(data)) { + if((conn->sockfd != conn->writesockfd) || + bitmap == GETSOCK_BLANK) { + /* only if they are not the same socket and we have a readable + one, we increase index */ + if(bitmap != GETSOCK_BLANK) + sockindex++; /* increase index if we need two entries */ + + DEBUGASSERT(conn->writesockfd != CURL_SOCKET_BAD); + sock[sockindex] = conn->writesockfd; + } + bitmap |= GETSOCK_WRITESOCK(sockindex); + } + return bitmap; + } } /* Initializes `poll_set` with the current socket poll actions needed @@ -1034,45 +1093,61 @@ static void multi_getsock(struct Curl_easy *data, return; switch(data->mstate) { - default: + case MSTATE_INIT: + case MSTATE_PENDING: + case MSTATE_CONNECT: + /* nothing to poll for yet */ break; case MSTATE_RESOLVING: - Curl_pollset_add_socks2(data, ps, Curl_resolv_getsock); + Curl_pollset_add_socks(data, ps, Curl_resolv_getsock); /* connection filters are not involved in this phase */ - return; + break; + + case MSTATE_CONNECTING: + case MSTATE_TUNNELING: + Curl_pollset_add_socks(data, ps, connecting_getsock); + Curl_conn_adjust_pollset(data, ps); + break; - case MSTATE_PROTOCONNECTING: case MSTATE_PROTOCONNECT: + case MSTATE_PROTOCONNECTING: Curl_pollset_add_socks(data, ps, protocol_getsock); + Curl_conn_adjust_pollset(data, ps); break; case MSTATE_DO: case MSTATE_DOING: Curl_pollset_add_socks(data, ps, doing_getsock); - break; - - case MSTATE_TUNNELING: - case MSTATE_CONNECTING: + Curl_conn_adjust_pollset(data, ps); break; case MSTATE_DOING_MORE: Curl_pollset_add_socks(data, ps, domore_getsock); + Curl_conn_adjust_pollset(data, ps); break; - case MSTATE_DID: /* since is set after DO is completed, we switch to - waiting for the same as the PERFORMING state */ + case MSTATE_DID: /* same as PERFORMING in regard to polling */ case MSTATE_PERFORMING: - Curl_pollset_add_socks(data, ps, Curl_single_getsock); + Curl_pollset_add_socks(data, ps, perform_getsock); + Curl_conn_adjust_pollset(data, ps); break; case MSTATE_RATELIMITING: - /* nothing to wait for */ - return; - } + /* we need to let time pass, ignore socket(s) */ + break; - /* Let connection filters add/remove as needed */ - Curl_conn_adjust_pollset(data, ps); + case MSTATE_DONE: + case MSTATE_COMPLETED: + case MSTATE_MSGSENT: + /* nothing more to poll for */ + break; + + default: + failf(data, "multi_getsock: unexpected multi state %d", data->mstate); + DEBUGASSERT(0); + break; + } } CURLMcode curl_multi_fdset(struct Curl_multi *multi, diff --git a/lib/transfer.c b/lib/transfer.c index 59a3f05cb..0d7226c8a 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -1220,52 +1220,6 @@ out: return result; } -/* - * Curl_single_getsock() gets called by the multi interface code when the app - * has requested to get the sockets for the current connection. This function - * will then be called once for every connection that the multi interface - * keeps track of. This function will only be called for connections that are - * in the proper state to have this information available. - */ -int Curl_single_getsock(struct Curl_easy *data, - struct connectdata *conn, - curl_socket_t *sock) -{ - int bitmap = GETSOCK_BLANK; - unsigned sockindex = 0; - - if(conn->handler->perform_getsock) - return conn->handler->perform_getsock(data, conn, sock); - - /* don't include HOLD and PAUSE connections */ - if((data->req.keepon & KEEP_RECVBITS) == KEEP_RECV) { - - DEBUGASSERT(conn->sockfd != CURL_SOCKET_BAD); - - bitmap |= GETSOCK_READSOCK(sockindex); - sock[sockindex] = conn->sockfd; - } - - /* don't include HOLD and PAUSE connections */ - if((data->req.keepon & KEEP_SENDBITS) == KEEP_SEND) { - if((conn->sockfd != conn->writesockfd) || - bitmap == GETSOCK_BLANK) { - /* only if they are not the same socket and we have a readable - one, we increase index */ - if(bitmap != GETSOCK_BLANK) - sockindex++; /* increase index if we need two entries */ - - DEBUGASSERT(conn->writesockfd != CURL_SOCKET_BAD); - - sock[sockindex] = conn->writesockfd; - } - - bitmap |= GETSOCK_WRITESOCK(sockindex); - } - - return bitmap; -} - /* Curl_init_CONNECT() gets called each time the handle switches to CONNECT which means this gets called once for each subsequent redirect etc */ void Curl_init_CONNECT(struct Curl_easy *data) diff --git a/lib/transfer.h b/lib/transfer.h index f1969a62a..b057c50d2 100644 --- a/lib/transfer.h +++ b/lib/transfer.h @@ -47,8 +47,6 @@ CURLcode Curl_follow(struct Curl_easy *data, char *newurl, followtype type); CURLcode Curl_readwrite(struct connectdata *conn, 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, size_t *nreadp); CURLcode Curl_retry_request(struct Curl_easy *data, char **url); diff --git a/lib/urldata.h b/lib/urldata.h index 210fca76c..de192dade 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -583,7 +583,7 @@ struct hostname { (((data)->req.keepon & KEEP_SENDBITS) == KEEP_SEND) /* transfer receive is not on PAUSE or HOLD */ #define CURL_WANT_RECV(data) \ - (!((data)->req.keepon & (KEEP_RECV_PAUSE|KEEP_RECV_HOLD))) + (((data)->req.keepon & KEEP_RECVBITS) == KEEP_RECV) #if defined(CURLRES_ASYNCH) || !defined(CURL_DISABLE_DOH) #define USE_CURL_ASYNC diff --git a/lib/vquic/curl_ngtcp2.c b/lib/vquic/curl_ngtcp2.c index 89f690462..e391e4f42 100644 --- a/lib/vquic/curl_ngtcp2.c +++ b/lib/vquic/curl_ngtcp2.c @@ -1157,10 +1157,13 @@ static void cf_ngtcp2_adjust_pollset(struct Curl_cfilter *cf, struct easy_pollset *ps) { struct cf_ngtcp2_ctx *ctx = cf->ctx; - bool want_recv = CURL_WANT_RECV(data); - bool want_send = CURL_WANT_SEND(data); + bool want_recv, want_send; - if(ctx->qconn && (want_recv || want_send)) { + if(!ctx->qconn) + return; + + Curl_pollset_check(data, ps, ctx->q.sockfd, &want_recv, &want_send); + if(want_recv || want_send) { struct h3_stream_ctx *stream = H3_STREAM_CTX(data); struct cf_call_data save; bool c_exhaust, s_exhaust; diff --git a/lib/vquic/curl_quiche.c b/lib/vquic/curl_quiche.c index 9c4df2df0..ed0bde367 100644 --- a/lib/vquic/curl_quiche.c +++ b/lib/vquic/curl_quiche.c @@ -1180,10 +1180,13 @@ static void cf_quiche_adjust_pollset(struct Curl_cfilter *cf, struct easy_pollset *ps) { struct cf_quiche_ctx *ctx = cf->ctx; - bool want_recv = CURL_WANT_RECV(data); - bool want_send = CURL_WANT_SEND(data); + bool want_recv, want_send; - if(ctx->qconn && (want_recv || want_send)) { + if(!ctx->qconn) + return; + + Curl_pollset_check(data, ps, ctx->q.sockfd, &want_recv, &want_send); + if(want_recv || want_send) { struct stream_ctx *stream = H3_STREAM_CTX(data); bool c_exhaust, s_exhaust;