From 476adfeac019edec71d67e144f184f4dbf1a46fb Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Fri, 26 Jan 2024 12:05:08 +0100 Subject: [PATCH] multi: add xfer_buf to multi handle - can be borrowed by transfer during recv-write operation - needs to be released before borrowing again - adjustis size to `data->set.buffer_size` - used in transfer.c readwrite_data() Closes #12805 --- docs/libcurl/opts/CURLOPT_BUFFERSIZE.md | 5 ++ lib/conncache.c | 6 -- lib/easy.c | 1 - lib/multi.c | 78 ++++++++++++++++++++++--- lib/multihandle.h | 5 ++ lib/multiif.h | 24 ++++++++ lib/setopt.c | 3 - lib/transfer.c | 14 +++-- lib/url.c | 1 - lib/urldata.h | 1 - 10 files changed, 113 insertions(+), 25 deletions(-) diff --git a/docs/libcurl/opts/CURLOPT_BUFFERSIZE.md b/docs/libcurl/opts/CURLOPT_BUFFERSIZE.md index 1faebeef54..b4759ea653 100644 --- a/docs/libcurl/opts/CURLOPT_BUFFERSIZE.md +++ b/docs/libcurl/opts/CURLOPT_BUFFERSIZE.md @@ -42,6 +42,11 @@ transfer as that may lead to unintended consequences. The maximum size was 512kB until 7.88.0. +Starting in libcurl 8.7.0, there is just a single transfer buffer allocated +per multi handle. This buffer is used by all easy handles added to a multi +handle no matter how many parallel transfers there are. The buffer remains +allocated as long as there are active transfers. + # DEFAULT CURL_MAX_WRITE_SIZE (16kB) diff --git a/lib/conncache.c b/lib/conncache.c index 66f18ecb85..63128e1a34 100644 --- a/lib/conncache.c +++ b/lib/conncache.c @@ -395,8 +395,6 @@ bool Curl_conncache_return_conn(struct Curl_easy *data, important that details from this (unrelated) disconnect does not taint meta-data in the data handle. */ struct conncache *connc = data->state.conn_cache; - connc->closure_handle->state.buffer = data->state.buffer; - connc->closure_handle->set.buffer_size = data->set.buffer_size; Curl_disconnect(connc->closure_handle, conn_candidate, /* dead_connection */ FALSE); } @@ -522,12 +520,9 @@ Curl_conncache_extract_oldest(struct Curl_easy *data) void Curl_conncache_close_all_connections(struct conncache *connc) { struct connectdata *conn; - char buffer[READBUFFER_MIN + 1]; SIGPIPE_VARIABLE(pipe_st); if(!connc->closure_handle) return; - connc->closure_handle->state.buffer = buffer; - connc->closure_handle->set.buffer_size = READBUFFER_MIN; conn = conncache_find_first_connection(connc); while(conn) { @@ -541,7 +536,6 @@ void Curl_conncache_close_all_connections(struct conncache *connc) conn = conncache_find_first_connection(connc); } - connc->closure_handle->state.buffer = NULL; sigpipe_ignore(connc->closure_handle, &pipe_st); Curl_hostcache_clean(connc->closure_handle, diff --git a/lib/easy.c b/lib/easy.c index 067b6d7b69..763d43b925 100644 --- a/lib/easy.c +++ b/lib/easy.c @@ -1021,7 +1021,6 @@ fail: #ifndef CURL_DISABLE_COOKIES free(outcurl->cookies); #endif - free(outcurl->state.buffer); Curl_dyn_free(&outcurl->state.headerb); Curl_altsvc_cleanup(&outcurl->asi); Curl_hsts_cleanup(&outcurl->hsts); diff --git a/lib/multi.c b/lib/multi.c index 0926b0d85e..48a92928c8 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -94,6 +94,7 @@ static CURLMcode add_next_timeout(struct curltime now, static CURLMcode multi_timeout(struct Curl_multi *multi, long *timeout_ms); static void process_pending_handles(struct Curl_multi *multi); +static void multi_xfer_buf_free(struct Curl_multi *multi); #ifdef DEBUGBUILD static const char * const multi_statename[]={ @@ -189,6 +190,10 @@ static void mstate(struct Curl_easy *data, CURLMstate state /* changing to COMPLETED means there's one less easy handle 'alive' */ DEBUGASSERT(data->multi->num_alive > 0); data->multi->num_alive--; + if(!data->multi->num_alive) { + /* free the transfer buffer when we have no more active transfers */ + multi_xfer_buf_free(data->multi); + } } /* if this state has an init-function, run it */ @@ -784,7 +789,6 @@ static CURLcode multi_done(struct Curl_easy *data, data->state.lastconnect_id = -1; } - Curl_safefree(data->state.buffer); return result; } @@ -1891,12 +1895,9 @@ static CURLcode readrewind(struct Curl_easy *data) */ CURLcode Curl_preconnect(struct Curl_easy *data) { - if(!data->state.buffer) { - data->state.buffer = malloc(data->set.buffer_size + 1); - if(!data->state.buffer) - return CURLE_OUT_OF_MEMORY; - } - + /* this used to do data->state.buffer allocation, + maybe remove completely now? */ + (void)data; return CURLE_OK; } @@ -2450,7 +2451,6 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, { char *newurl = NULL; bool retry = FALSE; - DEBUGASSERT(data->state.buffer); /* check if over send speed */ send_timeout_ms = 0; if(data->set.max_send_speed) @@ -2883,6 +2883,7 @@ CURLMcode curl_multi_cleanup(struct Curl_multi *multi) Curl_free_multi_ssl_backend_data(multi->ssl_backend_data); #endif + multi_xfer_buf_free(multi); free(multi); return CURLM_OK; @@ -3819,3 +3820,64 @@ struct Curl_easy **curl_multi_get_handles(struct Curl_multi *multi) } return a; } + +CURLcode Curl_multi_xfer_buf_borrow(struct Curl_easy *data, + char **pbuf, size_t *pbuflen) +{ + DEBUGASSERT(data); + DEBUGASSERT(data->multi); + *pbuf = NULL; + *pbuflen = 0; + if(!data->multi) { + failf(data, "transfer has no multi handle"); + return CURLE_FAILED_INIT; + } + if(!data->set.buffer_size) { + failf(data, "transfer buffer size is 0"); + return CURLE_FAILED_INIT; + } + if(data->multi->xfer_buf_borrowed) { + failf(data, "attempt to borrow xfer_buf when already borrowed"); + return CURLE_AGAIN; + } + + if(data->multi->xfer_buf && + data->set.buffer_size > data->multi->xfer_buf_len) { + /* not large enough, get a new one */ + free(data->multi->xfer_buf); + data->multi->xfer_buf = NULL; + data->multi->xfer_buf_len = 0; + } + + if(!data->multi->xfer_buf) { + data->multi->xfer_buf = malloc((size_t)data->set.buffer_size); + if(!data->multi->xfer_buf) { + failf(data, "could not allocate xfer_buf of %zu bytes", + (size_t)data->set.buffer_size); + return CURLE_OUT_OF_MEMORY; + } + data->multi->xfer_buf_len = data->set.buffer_size; + } + + data->multi->xfer_buf_borrowed = TRUE; + *pbuf = data->multi->xfer_buf; + *pbuflen = data->multi->xfer_buf_len; + return CURLE_OK; +} + +void Curl_multi_xfer_buf_release(struct Curl_easy *data, char *buf) +{ + (void)buf; + DEBUGASSERT(data); + DEBUGASSERT(data->multi); + DEBUGASSERT(!buf || data->multi->xfer_buf == buf); + data->multi->xfer_buf_borrowed = FALSE; +} + +static void multi_xfer_buf_free(struct Curl_multi *multi) +{ + DEBUGASSERT(multi); + Curl_safefree(multi->xfer_buf); + multi->xfer_buf_len = 0; + multi->xfer_buf_borrowed = FALSE; +} diff --git a/lib/multihandle.h b/lib/multihandle.h index e03e382e28..3cccd343fe 100644 --- a/lib/multihandle.h +++ b/lib/multihandle.h @@ -124,6 +124,10 @@ struct Curl_multi { times of all currently set timers */ struct Curl_tree *timetree; + /* buffer used for transfer data, lazy initialized */ + char *xfer_buf; /* the actual buffer */ + size_t xfer_buf_len; /* the allocated length */ + #if defined(USE_SSL) struct multi_ssl_backend_data *ssl_backend_data; #endif @@ -171,6 +175,7 @@ struct Curl_multi { #endif BIT(dead); /* a callback returned error, everything needs to crash and burn */ + BIT(xfer_buf_borrowed); /* xfer_buf is currently being borrowed */ #ifdef DEBUGBUILD BIT(warned); /* true after user warned of DEBUGBUILD */ #endif diff --git a/lib/multiif.h b/lib/multiif.h index 7a344fa9fd..de2ffeb9f4 100644 --- a/lib/multiif.h +++ b/lib/multiif.h @@ -94,4 +94,28 @@ CURLMcode Curl_multi_add_perform(struct Curl_multi *multi, /* Return the value of the CURLMOPT_MAX_CONCURRENT_STREAMS option */ unsigned int Curl_multi_max_concurrent_streams(struct Curl_multi *multi); +/** + * Borrow the transfer buffer from the multi, suitable + * for the given transfer `data`. The buffer may only be used in one + * multi processing of the easy handle. It MUST be returned to the + * multi before it can be borrowed again. + * Pointers into the buffer remain only valid as long as it is borrowed. + * + * @param data the easy handle + * @param pbuf on return, the buffer to use or NULL on error + * @param pbuflen on return, the size of *pbuf or 0 on error + * @return CURLE_OK when buffer is available and is returned. + * CURLE_OUT_OF_MEMORy on failure to allocate the buffer, + * CURLE_FAILED_INIT if the easy handle is without multi. + * CURLE_AGAIN if the buffer is borrowed already. + */ +CURLcode Curl_multi_xfer_buf_borrow(struct Curl_easy *data, + char **pbuf, size_t *pbuflen); +/** + * Release the borrowed buffer. All references into the buffer become + * invalid after this. + * @param buf the buffer pointer borrowed for coding error checks. + */ +void Curl_multi_xfer_buf_release(struct Curl_easy *data, char *buf); + #endif /* HEADER_CURL_MULTIIF_H */ diff --git a/lib/setopt.c b/lib/setopt.c index a5270773f3..e5614cd351 100644 --- a/lib/setopt.c +++ b/lib/setopt.c @@ -2210,9 +2210,6 @@ CURLcode Curl_vsetopt(struct Curl_easy *data, CURLoption option, va_list param) * The application kindly asks for a differently sized receive buffer. * If it seems reasonable, we'll use it. */ - if(data->state.buffer) - return CURLE_BAD_FUNCTION_ARGUMENT; - arg = va_arg(param, long); if(arg > READBUFFER_MAX) diff --git a/lib/transfer.c b/lib/transfer.c index 3ae4b61c0e..7d9fa6bd4f 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -466,15 +466,18 @@ static CURLcode readwrite_data(struct Curl_easy *data, { struct connectdata *conn = data->conn; CURLcode result = CURLE_OK; - char *buf; - size_t blen; + char *buf, *xfer_buf; + size_t blen, xfer_blen; int maxloops = 10; curl_off_t total_received = 0; bool is_multiplex = FALSE; - DEBUGASSERT(data->state.buffer); *done = FALSE; + result = Curl_multi_xfer_buf_borrow(data, &xfer_buf, &xfer_blen); + if(result) + goto out; + /* This is where we loop until we have read everything there is to read or we get a CURLE_AGAIN */ do { @@ -489,8 +492,8 @@ static CURLcode readwrite_data(struct Curl_easy *data, is_multiplex = Curl_conn_is_multiplex(conn, FIRSTSOCKET); } - buf = data->state.buffer; - bytestoread = data->set.buffer_size; + buf = xfer_buf; + bytestoread = xfer_blen; /* Observe any imposed speed limit */ if(bytestoread && data->set.max_recv_speed) { @@ -564,6 +567,7 @@ static CURLcode readwrite_data(struct Curl_easy *data, } out: + Curl_multi_xfer_buf_release(data, xfer_buf); if(result) DEBUGF(infof(data, "readwrite_data() -> %d", result)); return result; diff --git a/lib/url.c b/lib/url.c index 36395a155f..0f85099272 100644 --- a/lib/url.c +++ b/lib/url.c @@ -280,7 +280,6 @@ CURLcode Curl_close(struct Curl_easy **datap) data->state.referer = NULL; up_free(data); - Curl_safefree(data->state.buffer); Curl_dyn_free(&data->state.headerb); Curl_safefree(data->state.ulbuf); Curl_flush_cookies(data, TRUE); diff --git a/lib/urldata.h b/lib/urldata.h index 49aed15fda..fabc30ea4b 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -1337,7 +1337,6 @@ struct UrlState { struct dynbuf headerb; /* buffer to store headers in */ struct curl_slist *hstslist; /* list of HSTS files set by curl_easy_setopt(HSTS) calls */ - char *buffer; /* download buffer */ char *ulbuf; /* allocated upload buffer or NULL */ curl_off_t current_speed; /* the ProgressShow() function sets this, bytes / second */ -- 2.47.2