bundles connection caching: some out of memory handling fixes

5 files changed, 68 insertions, 19 deletions

diff --git a/lib/bundles.c b/lib/bundles.c
index 046e3bb3b..f09ee2a35 100644
--- a/lib/bundles.c
+++ b/lib/bundles.c
@@ -6,6 +6,7 @@
  *                             \___|\___/|_| \_\_____|
  *
  * Copyright (C) 2012, Linus Nielsen Feltzing, <linus@haxx.se>
+ * Copyright (C) 2012, 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
@@ -48,6 +49,7 @@ CURLcode Curl_bundle_create(struct SessionHandle *data,
                             struct connectbundle **cb_ptr)
 {
   (void)data;
+  DEBUGASSERT(*cb_ptr == NULL);
   *cb_ptr = malloc(sizeof(struct connectbundle));
   if(!*cb_ptr)
     return CURLE_OUT_OF_MEMORY;
@@ -56,15 +58,22 @@ CURLcode Curl_bundle_create(struct SessionHandle *data,
   (*cb_ptr)->server_supports_pipelining = FALSE;
 
   (*cb_ptr)->conn_list = Curl_llist_alloc((curl_llist_dtor) conn_llist_dtor);
-  if(!(*cb_ptr)->conn_list)
+  if(!(*cb_ptr)->conn_list) {
+    Curl_safefree(*cb_ptr);
     return CURLE_OUT_OF_MEMORY;
+  }
   return CURLE_OK;
 }
 
 void Curl_bundle_destroy(struct connectbundle *cb_ptr)
 {
-  if(cb_ptr->conn_list)
+  if(!cb_ptr)
+    return;
+
+  if(cb_ptr->conn_list) {
     Curl_llist_destroy(cb_ptr->conn_list, NULL);
+    cb_ptr->conn_list = NULL;
+  }
   Curl_safefree(cb_ptr);
 }
 
diff --git a/lib/conncache.c b/lib/conncache.c
index dc5b58cbe..4bca7ba51 100644
--- a/lib/conncache.c
+++ b/lib/conncache.c
@@ -6,6 +6,7 @@
  *                             \___|\___/|_| \_\_____|
  *
  * Copyright (C) 2012, Linus Nielsen Feltzing, <linus@haxx.se>
+ * Copyright (C) 2012, 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
@@ -70,8 +71,11 @@ struct conncache *Curl_conncache_init(conncachetype type)
 
 void Curl_conncache_destroy(struct conncache *connc)
 {
-  Curl_hash_destroy(connc->hash);
-  free(connc);
+  if(connc) {
+    Curl_hash_destroy(connc->hash);
+    connc->hash = NULL;
+    free(connc);
+  }
 }
 
 struct connectbundle *Curl_conncache_find_bundle(struct conncache *connc,
@@ -125,23 +129,30 @@ CURLcode Curl_conncache_add_conn(struct conncache *connc,
 {
   CURLcode result;
   struct connectbundle *bundle;
+  struct connectbundle *new_bundle = NULL;
   struct SessionHandle *data = conn->data;
 
   bundle = Curl_conncache_find_bundle(data->state.conn_cache,
                                       conn->host.name);
   if(!bundle) {
-    result = Curl_bundle_create(data, &bundle);
+    result = Curl_bundle_create(data, &new_bundle);
     if(result != CURLE_OK)
       return result;
 
     if(!conncache_add_bundle(data->state.conn_cache,
-                             conn->host.name, bundle))
+                             conn->host.name, new_bundle)) {
+      Curl_bundle_destroy(new_bundle);
       return CURLE_OUT_OF_MEMORY;
+    }
+    bundle = new_bundle;
   }
 
   result = Curl_bundle_add_conn(bundle, conn);
-  if(result != CURLE_OK)
+  if(result != CURLE_OK) {
+    if(new_bundle)
+      conncache_remove_bundle(data->state.conn_cache, new_bundle);
     return result;
+  }
 
   connc->num_connections++;
 
diff --git a/lib/multi.c b/lib/multi.c
index bab61578a..a1dd70513 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -432,6 +432,7 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle,
   struct Curl_one_easy *easy;
   struct Curl_multi *multi = (struct Curl_multi *)multi_handle;
   struct SessionHandle *data = (struct SessionHandle *)easy_handle;
+  struct SessionHandle *new_closure = NULL;
 
   /* First, make some basic checks that the CURLM handle is a good handle */
   if(!GOOD_MULTI_HANDLE(multi))
@@ -447,15 +448,6 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle,
     /* possibly we should create a new unique error code for this condition */
     return CURLM_BAD_EASY_HANDLE;
 
-  /* This is a good time to allocate a fresh easy handle to use when closing
-     cached connections */
-  if(!multi->closure_handle) {
-    multi->closure_handle =
-      (struct SessionHandle *)curl_easy_init();
-    Curl_easy_addmulti(easy_handle, multi_handle);
-    multi->closure_handle->state.conn_cache = multi->conn_cache;
-  }
-
   /* Allocate and initialize timeout list for easy handle */
   timeoutlist = Curl_llist_alloc(multi_freetimeout);
   if(!timeoutlist)
@@ -469,6 +461,17 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle,
     return CURLM_OUT_OF_MEMORY;
   }
 
+  /* In case multi handle has no closure_handle yet, allocate
+     a new easy handle to use when closing cached connections */
+  if(!multi->closure_handle) {
+    new_closure = (struct SessionHandle *)curl_easy_init();
+    if(!new_closure) {
+      free(easy);
+      Curl_llist_destroy(timeoutlist, NULL);
+      return CURLM_OUT_OF_MEMORY;
+    }
+  }
+
   /*
   ** No failure allowed in this function beyond this point. And
   ** no modification of easy nor multi handle allowed before this
@@ -476,6 +479,14 @@ CURLMcode curl_multi_add_handle(CURLM *multi_handle,
   ** won't be undone in this function no matter what.
   */
 
+  /* In case a new closure handle has been initialized above, it
+     is associated now with the multi handle which lacked one. */
+  if(new_closure) {
+    multi->closure_handle = new_closure;
+    Curl_easy_addmulti(multi->closure_handle, multi_handle);
+    multi->closure_handle->state.conn_cache = multi->conn_cache;
+  }
+
   /* Make easy handle use timeout list initialized above */
   data->state.timeoutlist = timeoutlist;
   timeoutlist = NULL;
diff --git a/lib/ssh.c b/lib/ssh.c
index 3a9729975..cb743eb41 100644
--- a/lib/ssh.c
+++ b/lib/ssh.c
@@ -2495,6 +2495,9 @@ static CURLcode ssh_statemach_act(struct connectdata *conn, bool *block)
 #ifdef HAVE_LIBSSH2_KNOWNHOST_API
       DEBUGASSERT(sshc->kh == NULL);
 #endif
+#ifdef HAVE_LIBSSH2_AGENT_API
+      DEBUGASSERT(sshc->ssh_agent == NULL);
+#endif
 
       Curl_safefree(sshc->rsa_pub);
       Curl_safefree(sshc->rsa);
diff --git a/lib/url.c b/lib/url.c
index d93a0e996..58befecab 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -372,10 +372,15 @@ CURLcode Curl_dupset(struct SessionHandle * dst, struct SessionHandle * src)
 
 CURLcode Curl_close(struct SessionHandle *data)
 {
-  struct Curl_multi *m = data->multi;
+  struct Curl_multi *m;
+
+  if(!data)
+    return CURLE_OK;
 
   Curl_expire(data, 0); /* shut off timers */
 
+  m = data->multi;
+
   if(m)
     /* This handle is still part of a multi handle, take care of this first
        and detach this handle from there. */
@@ -3060,11 +3065,17 @@ static CURLcode ConnectionStore(struct SessionHandle *data,
 {
   static int connection_id_counter = 0;
 
+  CURLcode result;
+
   /* Assign a number to the connection for easier tracking in the log
      output */
   conn->connection_id = connection_id_counter++;
 
-  return Curl_conncache_add_conn(data->state.conn_cache, conn);
+  result = Curl_conncache_add_conn(data->state.conn_cache, conn);
+  if(result != CURLE_OK)
+    conn->connection_id = -1;
+
+  return result;
 }
 
 /* after a TCP connection to the proxy has been verified, this function does
@@ -5239,8 +5250,12 @@ CURLcode Curl_done(struct connectdata **connp,
      state it is for re-using, so we're forced to close it. In a perfect world
      we can add code that keep track of if we really must close it here or not,
      but currently we have no such detail knowledge.
+
+     connection_id == -1 here means that the connection has not been added
+     to the connection cache (OOM) and thus we must disconnect it here.
   */
-  if(data->set.reuse_forbid || conn->bits.close || premature) {
+  if(data->set.reuse_forbid || conn->bits.close || premature ||
+     (-1 == conn->connection_id)) {
     CURLcode res2 = Curl_disconnect(conn, premature); /* close connection */
 
     /* If we had an error already, make sure we return that one. But