Skip to content

Commit

Permalink
url: alloc the download buffer at transfer start
Browse files Browse the repository at this point in the history
... and free it as soon as the transfer is done. It removes the extra
alloc when a new size is set with setopt() and reduces memory for unused
easy handles.

In addition: the closure_handle now doesn't use an allocated buffer at
all but the smallest supported size as a stack based one.

Closes #5472
  • Loading branch information
bagder committed May 30, 2020
1 parent 842f73d commit c4e6968
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 105 deletions.
6 changes: 6 additions & 0 deletions lib/conncache.c
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,11 @@ Curl_conncache_extract_oldest(struct Curl_easy *data)
void Curl_conncache_close_all_connections(struct conncache *connc)
{
struct connectdata *conn;
char buffer[READBUFFER_MIN];
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) {
Expand All @@ -547,6 +552,7 @@ void Curl_conncache_close_all_connections(struct conncache *connc)
conn = conncache_find_first_connection(connc);
}

connc->closure_handle->state.buffer = NULL;
if(connc->closure_handle) {
SIGPIPE_VARIABLE(pipe_st);
sigpipe_ignore(connc->closure_handle, &pipe_st);
Expand Down
17 changes: 0 additions & 17 deletions lib/easy.c
Original file line number Diff line number Diff line change
Expand Up @@ -829,9 +829,6 @@ struct Curl_easy *curl_easy_duphandle(struct Curl_easy *data)
* the likeliness of us forgetting to init a buffer here in the future.
*/
outcurl->set.buffer_size = data->set.buffer_size;
outcurl->state.buffer = malloc(outcurl->set.buffer_size + 1);
if(!outcurl->state.buffer)
goto fail;

/* copy all userdefined values */
if(dupset(outcurl, data))
Expand Down Expand Up @@ -947,8 +944,6 @@ struct Curl_easy *curl_easy_duphandle(struct Curl_easy *data)
*/
void curl_easy_reset(struct Curl_easy *data)
{
long old_buffer_size = data->set.buffer_size;

Curl_free_request_state(data);

/* zero out UserDefined data: */
Expand All @@ -972,18 +967,6 @@ void curl_easy_reset(struct Curl_easy *data)
#if !defined(CURL_DISABLE_HTTP) && !defined(CURL_DISABLE_CRYPTO_AUTH)
Curl_http_auth_cleanup_digest(data);
#endif

/* resize receive buffer */
if(old_buffer_size != data->set.buffer_size) {
char *newbuff = realloc(data->state.buffer, data->set.buffer_size + 1);
if(!newbuff) {
DEBUGF(fprintf(stderr, "Error: realloc of buffer failed\n"));
/* nothing we can do here except use the old size */
data->set.buffer_size = old_buffer_size;
}
else
data->state.buffer = newbuff;
}
}

/*
Expand Down
9 changes: 6 additions & 3 deletions lib/http2.c
Original file line number Diff line number Diff line change
Expand Up @@ -258,15 +258,14 @@ static unsigned int http2_conncheck(struct connectdata *check,
void Curl_http2_setup_req(struct Curl_easy *data)
{
struct HTTP *http = data->req.protop;

http->bodystarted = FALSE;
http->status_code = -1;
http->pausedata = NULL;
http->pauselen = 0;
http->closed = FALSE;
http->close_handled = FALSE;
http->mem = data->state.buffer;
http->len = data->set.buffer_size;
http->mem = NULL;
http->len = 0;
http->memlen = 0;
}

Expand Down Expand Up @@ -2103,6 +2102,8 @@ CURLcode Curl_http2_setup(struct connectdata *conn)
struct http_conn *httpc = &conn->proto.httpc;
struct HTTP *stream = conn->data->req.protop;

DEBUGASSERT(conn->data->state.buffer);

stream->stream_id = -1;

Curl_dyn_init(&stream->header_recvbuf, DYN_H2_HEADERS);
Expand All @@ -2126,6 +2127,8 @@ CURLcode Curl_http2_setup(struct connectdata *conn)
stream->upload_left = 0;
stream->upload_mem = NULL;
stream->upload_len = 0;
stream->mem = conn->data->state.buffer;
stream->len = conn->data->set.buffer_size;

httpc->inbuflen = 0;
httpc->nread_inbuf = 0;
Expand Down
22 changes: 21 additions & 1 deletion lib/multi.c
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,7 @@ static CURLcode multi_done(struct Curl_easy *data,
data->state.lastconnect = NULL;
}

Curl_safefree(data->state.buffer);
Curl_free_request_state(data);
return result;
}
Expand Down Expand Up @@ -1522,6 +1523,20 @@ static CURLcode protocol_connect(struct connectdata *conn,
return result; /* pass back status */
}

/*
* preconnect() is called immediately before a connect starts. When a redirect
* is followed, this is then called multiple times during a single transfer.
*/
static CURLcode 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;
}
return CURLE_OK;
}


static CURLMcode multi_runsingle(struct Curl_multi *multi,
struct curltime now,
Expand Down Expand Up @@ -1629,6 +1644,11 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,

case CURLM_STATE_CONNECT:
/* Connect. We want to get a connection identifier filled in. */
/* init this transfer. */
result = preconnect(data);
if(result)
break;

Curl_pgrsTime(data, TIMER_STARTSINGLE);
if(data->set.timeout)
Curl_expire(data, data->set.timeout, EXPIRE_TIMEOUT);
Expand Down Expand Up @@ -2058,7 +2078,7 @@ 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;
if(data->set.max_send_speed > 0)
Expand Down
2 changes: 1 addition & 1 deletion lib/setopt.c
Original file line number Diff line number Diff line change
Expand Up @@ -2076,7 +2076,7 @@ CURLcode Curl_vsetopt(struct Curl_easy *data, CURLoption option, va_list param)
arg = READBUFFER_MIN;

/* Resize if new size */
if(arg != data->set.buffer_size) {
if((arg != data->set.buffer_size) && data->state.buffer) {
char *newbuff = realloc(data->state.buffer, arg + 1);
if(!newbuff) {
DEBUGF(fprintf(stderr, "Error: realloc of buffer failed\n"));
Expand Down
14 changes: 7 additions & 7 deletions lib/transfer.c
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,8 @@ static CURLcode readwrite_data(struct Curl_easy *data,
size_t excess = 0; /* excess bytes read */
bool readmore = FALSE; /* used by RTP to signal for more data */
int maxloops = 100;
char *buf = data->state.buffer;
DEBUGASSERT(buf);

*done = FALSE;
*comeback = FALSE;
Expand Down Expand Up @@ -592,7 +594,7 @@ static CURLcode readwrite_data(struct Curl_easy *data,

if(bytestoread) {
/* receive data from the network! */
result = Curl_read(conn, conn->sockfd, k->buf, bytestoread, &nread);
result = Curl_read(conn, conn->sockfd, buf, bytestoread, &nread);

/* read would've blocked */
if(CURLE_AGAIN == result)
Expand All @@ -619,9 +621,8 @@ static CURLcode readwrite_data(struct Curl_easy *data,
/* indicates data of zero size, i.e. empty file */
is_empty_data = ((nread == 0) && (k->bodywrites == 0)) ? TRUE : FALSE;

/* NUL terminate, allowing string ops to be used */
if(0 < nread || is_empty_data) {
k->buf[nread] = 0;
buf[nread] = 0;
}
else {
/* if we receive 0 or less here, either the http2 stream is closed or the
Expand All @@ -638,7 +639,7 @@ static CURLcode readwrite_data(struct Curl_easy *data,

/* Default buffer to use when we write the buffer, it may be changed
in the flow below before the actual storing is done. */
k->str = k->buf;
k->str = buf;

if(conn->handler->readwrite) {
result = conn->handler->readwrite(data, conn, &nread, &readmore);
Expand Down Expand Up @@ -905,10 +906,10 @@ static CURLcode readwrite_data(struct Curl_easy *data,
/* Parse the excess data */
k->str += nread;

if(&k->str[excess] > &k->buf[data->set.buffer_size]) {
if(&k->str[excess] > &buf[data->set.buffer_size]) {
/* the excess amount was too excessive(!), make sure
it doesn't read out of buffer */
excess = &k->buf[data->set.buffer_size] - k->str;
excess = &buf[data->set.buffer_size] - k->str;
}
nread = (ssize_t)excess;

Expand Down Expand Up @@ -1467,7 +1468,6 @@ CURLcode Curl_pretransfer(struct Curl_easy *data)
data->state.authhost.want = data->set.httpauth;
data->state.authproxy.want = data->set.proxyauth;
Curl_safefree(data->info.wouldredirect);
data->info.wouldredirect = NULL;

if(data->set.httpreq == HTTPREQ_PUT)
data->state.infilesize = data->set.filesize;
Expand Down
32 changes: 9 additions & 23 deletions lib/url.c
Original file line number Diff line number Diff line change
Expand Up @@ -610,31 +610,21 @@ CURLcode Curl_open(struct Curl_easy **curl)
return result;
}

/* We do some initial setup here, all those fields that can't be just 0 */

data->state.buffer = malloc(READBUFFER_SIZE + 1);
if(!data->state.buffer) {
DEBUGF(fprintf(stderr, "Error: malloc of buffer failed\n"));
result = CURLE_OUT_OF_MEMORY;
}
else {
result = Curl_init_userdefined(data);
if(!result) {
Curl_dyn_init(&data->state.headerb, CURL_MAX_HTTP_HEADER);
Curl_convert_init(data);
Curl_initinfo(data);
result = Curl_init_userdefined(data);
if(!result) {
Curl_dyn_init(&data->state.headerb, CURL_MAX_HTTP_HEADER);
Curl_convert_init(data);
Curl_initinfo(data);

/* most recent connection is not yet defined */
data->state.lastconnect = NULL;
/* most recent connection is not yet defined */
data->state.lastconnect = NULL;

data->progress.flags |= PGRS_HIDE;
data->state.current_speed = -1; /* init to negative == impossible */
}
data->progress.flags |= PGRS_HIDE;
data->state.current_speed = -1; /* init to negative == impossible */
}

if(result) {
Curl_resolver_cleanup(data->state.resolver);
free(data->state.buffer);
Curl_dyn_free(&data->state.headerb);
Curl_freeset(data);
free(data);
Expand Down Expand Up @@ -3973,14 +3963,10 @@ CURLcode Curl_init_do(struct Curl_easy *data, struct connectdata *conn)
k->start = Curl_now(); /* start time */
k->now = k->start; /* current time is now */
k->header = TRUE; /* assume header */

k->bytecount = 0;

k->buf = data->state.buffer;
k->ignorebody = FALSE;

Curl_speedinit(data);

Curl_pgrsSetUploadCounter(data, 0);
Curl_pgrsSetDownloadCounter(data, 0);

Expand Down
1 change: 0 additions & 1 deletion lib/urldata.h
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,6 @@ struct SingleRequest {
struct contenc_writer *writer_stack;
time_t timeofdoc;
long bodywrites;
char *buf;
int keepon;
char *location; /* This points to an allocated version of the Location:
header data */
Expand Down
5 changes: 1 addition & 4 deletions tests/data/test509
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,7 @@ nothing
# Verify data after the test has been "shot"
<verify>
<stdout>
seen custom_calloc()
seen custom_malloc()
seen custom_realloc()
seen custom_free()
Callbacks were invoked!
</stdout>
</verify>

Expand Down
63 changes: 15 additions & 48 deletions tests/libtest/lib509.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
* Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
* Copyright (C) 1998 - 2020, Daniel Stenberg, <daniel@haxx.se>, et al.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
Expand Down Expand Up @@ -34,70 +34,35 @@
* memory callbacks which should be calling 'the real thing'.
*/

/*
#include "memdebug.h"
*/

static int seen_malloc = 0;
static int seen_free = 0;
static int seen_realloc = 0;
static int seen_strdup = 0;
static int seen_calloc = 0;

void *custom_malloc(size_t size);
void custom_free(void *ptr);
void *custom_realloc(void *ptr, size_t size);
char *custom_strdup(const char *ptr);
void *custom_calloc(size_t nmemb, size_t size);
static int seen;


void *custom_calloc(size_t nmemb, size_t size)
static void *custom_calloc(size_t nmemb, size_t size)
{
if(!seen_calloc) {
printf("seen custom_calloc()\n");
seen_calloc = 1;
}
seen++;
return (calloc)(nmemb, size);
}

void *custom_malloc(size_t size)
static void *custom_malloc(size_t size)
{
if(!seen_malloc && seen_calloc) {
printf("seen custom_malloc()\n");
seen_malloc = 1;
}
seen++;
return (malloc)(size);
}

char *custom_strdup(const char *ptr)
static char *custom_strdup(const char *ptr)
{
if(!seen_strdup && seen_malloc) {
/* currently (2013.03.13), memory tracking enabled builds do not call
the strdup callback, in this case malloc callback and memcpy are used
instead. If some day this is changed the following printf() should be
uncommented, and a line added to test definition.
printf("seen custom_strdup()\n");
*/
seen_strdup = 1;
}
seen++;
return (strdup)(ptr);
}

void *custom_realloc(void *ptr, size_t size)
static void *custom_realloc(void *ptr, size_t size)
{
if(!seen_realloc && seen_malloc) {
printf("seen custom_realloc()\n");
seen_realloc = 1;
}
seen++;
return (realloc)(ptr, size);
}

void custom_free(void *ptr)
static void custom_free(void *ptr)
{
if(!seen_free && seen_realloc) {
printf("seen custom_free()\n");
seen_free = 1;
}
seen++;
(free)(ptr);
}

Expand All @@ -110,7 +75,6 @@ int test(char *URL)
CURL *curl;
int asize;
char *str = NULL;

(void)URL;

res = curl_global_init_mem(CURL_GLOBAL_ALL,
Expand All @@ -136,6 +100,9 @@ int test(char *URL)
asize = (int)sizeof(a);
str = curl_easy_escape(curl, (char *)a, asize); /* uses realloc() */

if(seen)
printf("Callbacks were invoked!\n");

test_cleanup:

if(str)
Expand Down

0 comments on commit c4e6968

Please sign in to comment.