http2: separate multiplex/pipelining + cleanup memory leaks

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).
@@ -1044,6 +1056,16 @@ Curl_send_buffer *Curl_add_buffer_init(void)
 }
 
 /*
+ * 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. */
 };