From eafccdb3150115e94f16a3a21ea989aeb65a0210 Mon Sep 17 00:00:00 2001 From: Yang Tse Date: Wed, 19 Dec 2012 19:52:11 +0100 Subject: [PATCH] bundles connection caching: some out of memory handling fixes --- lib/bundles.c | 13 +++++++++++-- lib/conncache.c | 21 ++++++++++++++++----- lib/multi.c | 29 ++++++++++++++++++++--------- lib/ssh.c | 3 +++ lib/url.c | 21 ++++++++++++++++++--- 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, + * Copyright (C) 2012, Daniel Stenberg, , 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, + * Copyright (C) 2012, Daniel Stenberg, , 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