From 783b3c7b427dce4b1906709cef350af5e9623673 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Fri, 8 May 2015 14:42:59 +0200 Subject: http2: separate multiplex/pipelining + cleanup memory leaks --- docs/libcurl/symbols-in-versions | 3 +++ lib/http.c | 43 ++++++++++++++++++++++++++++++---------- lib/http.h | 1 + lib/http2.c | 2 +- lib/http_proxy.c | 8 ++++---- lib/multi.c | 1 + lib/url.c | 39 ++++++++++++++++++------------------ lib/urldata.h | 8 +++----- 8 files changed, 65 insertions(+), 40 deletions(-) diff --git a/docs/libcurl/symbols-in-versions b/docs/libcurl/symbols-in-versions index 18dc1beda..6c8b6b994 100644 --- a/docs/libcurl/symbols-in-versions +++ b/docs/libcurl/symbols-in-versions @@ -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 diff --git a/lib/http.c b/lib/http.c index cf2ee1c37..b2ecc9c0a 100644 --- a/lib/http.c +++ b/lib/http.c @@ -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); @@ -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 */ @@ -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 */ @@ -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). @@ -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. @@ -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; } @@ -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; } @@ -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; diff --git a/lib/http.h b/lib/http.h index 0b4aac343..67426d2b0 100644 --- a/lib/http.h +++ b/lib/http.h @@ -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, diff --git a/lib/http2.c b/lib/http2.c index 789f76a85..7070df511 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -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")); diff --git a/lib/http_proxy.c b/lib/http_proxy.c index e7d848240..5ab9915a6 100644 --- a/lib/http_proxy.c +++ b/lib/http_proxy.c @@ -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; } @@ -155,7 +155,7 @@ 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; } @@ -163,7 +163,7 @@ CURLcode Curl_proxyCONNECT(struct connectdata *conn, 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; } } @@ -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; diff --git a/lib/multi.c b/lib/multi.c index 5573151a8..498f4101f 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -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) { diff --git a/lib/url.c b/lib/url.c index 805459c82..bd95cef68 100644 --- a/lib/url.c +++ b/lib/url.c @@ -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: @@ -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); @@ -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; diff --git a/lib/urldata.h b/lib/urldata.h index b09177e7a..94b6aece2 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -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 */ @@ -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. */ }; -- cgit v1.2.3