Skip to content

Commit

Permalink
http2: separate multiplex/pipelining + cleanup memory leaks
Browse files Browse the repository at this point in the history
  • Loading branch information
bagder committed May 18, 2015
1 parent 02ec1ce commit 783b3c7
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 40 deletions.
3 changes: 3 additions & 0 deletions docs/libcurl/symbols-in-versions
Expand Up @@ -558,6 +558,9 @@ CURLPAUSE_RECV 7.18.0
CURLPAUSE_RECV_CONT 7.18.0
CURLPAUSE_SEND 7.18.0
CURLPAUSE_SEND_CONT 7.18.0
CURLPIPE_HTTP1 7.43.0
CURLPIPE_MULTIPLEX 7.43.0
CURLPIPE_NOTHING 7.43.0
CURLPROTO_ALL 7.19.4
CURLPROTO_DICT 7.19.4
CURLPROTO_FILE 7.19.4
Expand Down
43 changes: 33 additions & 10 deletions lib/http.c
Expand Up @@ -86,6 +86,7 @@
* Forward declarations.
*/

static CURLcode http_disconnect(struct connectdata *conn, bool dead);
static int http_getsock_do(struct connectdata *conn,
curl_socket_t *socks,
int numsocks);
Expand Down Expand Up @@ -116,7 +117,7 @@ const struct Curl_handler Curl_handler_http = {
http_getsock_do, /* doing_getsock */
ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
http_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
PORT_HTTP, /* defport */
CURLPROTO_HTTP, /* protocol */
Expand All @@ -140,7 +141,7 @@ const struct Curl_handler Curl_handler_https = {
http_getsock_do, /* doing_getsock */
ZERO_NULL, /* domore_getsock */
ZERO_NULL, /* perform_getsock */
ZERO_NULL, /* disconnect */
http_disconnect, /* disconnect */
ZERO_NULL, /* readwrite */
PORT_HTTPS, /* defport */
CURLPROTO_HTTPS, /* protocol */
Expand Down Expand Up @@ -179,6 +180,17 @@ CURLcode Curl_http_setup_conn(struct connectdata *conn)
return CURLE_OK;
}

static CURLcode http_disconnect(struct connectdata *conn, bool dead_connection)
{
struct HTTP *http = conn->data->req.protop;
(void)dead_connection;
if(http) {
Curl_add_buffer_free(http->header_recvbuf);
http->header_recvbuf = NULL; /* clear the pointer */
}
return CURLE_OK;
}

/*
* checkheaders() checks the linked list of custom HTTP headers for a
* particular header (prefix).
Expand Down Expand Up @@ -1043,6 +1055,16 @@ Curl_send_buffer *Curl_add_buffer_init(void)
return calloc(1, sizeof(Curl_send_buffer));
}

/*
* Curl_add_buffer_free() frees all associated resources.
*/
void Curl_add_buffer_free(Curl_send_buffer *buff)
{
if(buff) /* deal with NULL input */
free(buff->buffer);
free(buff);
}

/*
* Curl_add_buffer_send() sends a header buffer and frees all associated
* memory. Body data may be appended to the header data if desired.
Expand Down Expand Up @@ -1089,8 +1111,7 @@ CURLcode Curl_add_buffer_send(Curl_send_buffer *in,
/* Curl_convert_to_network calls failf if unsuccessful */
if(result) {
/* conversion failed, free memory and return to the caller */
free(in->buffer);
free(in);
Curl_add_buffer_free(in);
return result;
}

Expand Down Expand Up @@ -1192,8 +1213,7 @@ CURLcode Curl_add_buffer_send(Curl_send_buffer *in,
conn->writechannel_inuse = FALSE;
}
}
free(in->buffer);
free(in);
Curl_add_buffer_free(in);

return result;
}
Expand Down Expand Up @@ -1472,13 +1492,16 @@ CURLcode Curl_http_done(struct connectdata *conn,
return CURLE_OK;

if(http->send_buffer) {
Curl_send_buffer *buff = http->send_buffer;

free(buff->buffer);
free(buff);
Curl_add_buffer_free(http->send_buffer);
http->send_buffer = NULL; /* clear the pointer */
}

if(http->header_recvbuf) {
DEBUGF(infof(data, "free header_recvbuf!!\n"));
Curl_add_buffer_free(http->header_recvbuf);
http->header_recvbuf = NULL; /* clear the pointer */
}

if(HTTPREQ_POST_FORM == data->set.httpreq) {
data->req.bytecount = http->readbytecount + http->writebytecount;

Expand Down
1 change: 1 addition & 0 deletions lib/http.h
Expand Up @@ -60,6 +60,7 @@ struct Curl_send_buffer {
typedef struct Curl_send_buffer Curl_send_buffer;

Curl_send_buffer *Curl_add_buffer_init(void);
void Curl_add_buffer_free(Curl_send_buffer *buff);
CURLcode Curl_add_bufferf(Curl_send_buffer *in, const char *fmt, ...);
CURLcode Curl_add_buffer(Curl_send_buffer *in, const void *inptr, size_t size);
CURLcode Curl_add_buffer_send(Curl_send_buffer *in,
Expand Down
2 changes: 1 addition & 1 deletion lib/http2.c
Expand Up @@ -85,8 +85,8 @@ static CURLcode http2_disconnect(struct connectdata *conn,
DEBUGF(infof(conn->data, "HTTP/2 DISCONNECT starts now\n"));

nghttp2_session_del(c->h2);

Curl_safefree(c->inbuf);
Curl_hash_clean(&c->streamsh);

DEBUGF(infof(conn->data, "HTTP/2 DISCONNECT done\n"));

Expand Down
8 changes: 4 additions & 4 deletions lib/http_proxy.c
Expand Up @@ -135,7 +135,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,

host_port = aprintf("%s:%hu", hostname, remote_port);
if(!host_port) {
free(req_buffer);
Curl_add_buffer_free(req_buffer);
return CURLE_OUT_OF_MEMORY;
}

Expand All @@ -155,15 +155,15 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
hostname, conn->bits.ipv6_ip?"]":"",
remote_port);
if(!hostheader) {
free(req_buffer);
Curl_add_buffer_free(req_buffer);
return CURLE_OUT_OF_MEMORY;
}

if(!Curl_checkProxyheaders(conn, "Host:")) {
host = aprintf("Host: %s\r\n", hostheader);
if(!host) {
free(hostheader);
free(req_buffer);
Curl_add_buffer_free(req_buffer);
return CURLE_OUT_OF_MEMORY;
}
}
Expand Down Expand Up @@ -212,7 +212,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn,
failf(data, "Failed sending CONNECT to proxy");
}

free(req_buffer);
Curl_add_buffer_free(req_buffer);
if(result)
return result;

Expand Down
1 change: 1 addition & 0 deletions lib/multi.c
Expand Up @@ -1170,6 +1170,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
if(data->easy_conn->bits.proxy_connect_closed) {
/* connect back to proxy again */
result = CURLE_OK;
Curl_done(&data->easy_conn, CURLE_OK, FALSE);
multistate(data, CURLM_STATE_CONNECT);
}
else if(!result) {
Expand Down
39 changes: 19 additions & 20 deletions lib/url.c
Expand Up @@ -5951,36 +5951,18 @@ CURLcode Curl_done(struct connectdata **connp,

DEBUGF(infof(data, "Curl_done\n"));

if(conn->bits.done)
if(data->state.done)
/* Stop if Curl_done() has already been called */
return CURLE_OK;

Curl_getoff_all_pipelines(data, conn);

if((conn->send_pipe->size + conn->recv_pipe->size != 0 &&
!data->set.reuse_forbid &&
!conn->bits.close)) {
/* Stop if pipeline is not empty and we do not have to close
connection. */
DEBUGF(infof(data, "Connection still in use, no more Curl_done now!\n"));
return CURLE_OK;
}

conn->bits.done = TRUE; /* called just now! */

/* Cleanup possible redirect junk */
free(data->req.newurl);
data->req.newurl = NULL;
free(data->req.location);
data->req.location = NULL;

Curl_resolver_cancel(conn);

if(conn->dns_entry) {
Curl_resolv_unlock(data, conn->dns_entry); /* done with this */
conn->dns_entry = NULL;
}

switch(status) {
case CURLE_ABORTED_BY_CALLBACK:
case CURLE_READ_ERROR:
Expand All @@ -6003,6 +5985,23 @@ CURLcode Curl_done(struct connectdata **connp,
if(!result && Curl_pgrsDone(conn))
result = CURLE_ABORTED_BY_CALLBACK;

if((conn->send_pipe->size + conn->recv_pipe->size != 0 &&
!data->set.reuse_forbid &&
!conn->bits.close)) {
/* Stop if pipeline is not empty and we do not have to close
connection. */
DEBUGF(infof(data, "Connection still in use, no more Curl_done now!\n"));
return CURLE_OK;
}

data->state.done = TRUE; /* called just now! */
Curl_resolver_cancel(conn);

if(conn->dns_entry) {
Curl_resolv_unlock(data, conn->dns_entry); /* done with this */
conn->dns_entry = NULL;
}

/* if the transfer was completed in a paused state there can be buffered
data left to write and then kill */
free(data->state.tempwrite);
Expand Down Expand Up @@ -6072,7 +6071,7 @@ static CURLcode do_init(struct connectdata *conn)
struct SessionHandle *data = conn->data;
struct SingleRequest *k = &data->req;

conn->bits.done = FALSE; /* Curl_done() is not called yet */
data->state.done = FALSE; /* Curl_done() is not called yet */
conn->bits.do_more = FALSE; /* by default there's no curl_do_more() to use */
data->state.expect100header = FALSE;

Expand Down
8 changes: 3 additions & 5 deletions lib/urldata.h
Expand Up @@ -516,11 +516,6 @@ struct ConnectBits {
requests */
bool netrc; /* name+password provided by netrc */
bool userpwd_in_url; /* name+password found in url */

bool done; /* set to FALSE when Curl_do() is called and set to TRUE
when Curl_done() is called, to prevent Curl_done() to
get invoked twice when the multi interface is
used. */
bool stream_was_rewound; /* Indicates that the stream was rewound after a
request read past the end of its response byte
boundary */
Expand Down Expand Up @@ -1314,6 +1309,9 @@ struct UrlState {
int drain; /* Increased when this stream has data to read, even if its
socket not necessarily is readable. Decreased when
checked. */
bool done; /* set to FALSE when Curl_do() is called and set to TRUE when
Curl_done() is called, to prevent Curl_done() to get invoked
twice when the multi interface is used. */
};


Expand Down

0 comments on commit 783b3c7

Please sign in to comment.