diff options
| -rw-r--r-- | lib/conncache.c | 222 | ||||
| -rw-r--r-- | lib/conncache.h | 26 | ||||
| -rw-r--r-- | lib/multi.c | 56 | ||||
| -rw-r--r-- | lib/url.c | 174 | ||||
| -rw-r--r-- | lib/urldata.h | 10 | ||||
| -rw-r--r-- | tests/data/test1554 | 10 |
6 files changed, 315 insertions, 183 deletions
diff --git a/lib/conncache.c b/lib/conncache.c index f8ef2e88b..43d885131 100644 --- a/lib/conncache.c +++ b/lib/conncache.c @@ -40,11 +40,26 @@ #include "curl_memory.h" #include "memdebug.h" +#ifdef CURLDEBUG +/* the debug versions of these macros make extra certain that the lock is + never doubly locked or unlocked */ +#define CONN_LOCK(x) if((x)->share) { \ + Curl_share_lock((x), CURL_LOCK_DATA_CONNECT, CURL_LOCK_ACCESS_SINGLE); \ + DEBUGASSERT(!(x)->state.conncache_lock); \ + (x)->state.conncache_lock = TRUE; \ + } + +#define CONN_UNLOCK(x) if((x)->share) { \ + DEBUGASSERT((x)->state.conncache_lock); \ + (x)->state.conncache_lock = FALSE; \ + Curl_share_unlock((x), CURL_LOCK_DATA_CONNECT); \ + } +#else #define CONN_LOCK(x) if((x)->share) \ Curl_share_lock((x), CURL_LOCK_DATA_CONNECT, CURL_LOCK_ACCESS_SINGLE) #define CONN_UNLOCK(x) if((x)->share) \ Curl_share_unlock((x), CURL_LOCK_DATA_CONNECT) - +#endif static void conn_llist_dtor(void *user, void *element) { @@ -165,18 +180,48 @@ static void hashkey(struct connectdata *conn, char *buf, snprintf(buf, len, "%ld%s", conn->port, hostname); } +void Curl_conncache_unlock(struct connectdata *conn) +{ + CONN_UNLOCK(conn->data); +} + +/* Returns number of connections currently held in the connection cache. + Locks/unlocks the cache itself! +*/ +size_t Curl_conncache_size(struct Curl_easy *data) +{ + size_t num; + CONN_LOCK(data); + num = data->state.conn_cache->num_conn; + CONN_UNLOCK(data); + return num; +} + +/* Returns number of connections currently held in the connections's bundle + Locks/unlocks the cache itself! +*/ +size_t Curl_conncache_bundle_size(struct connectdata *conn) +{ + size_t num; + CONN_LOCK(conn->data); + num = conn->bundle->num_connections; + CONN_UNLOCK(conn->data); + return num; +} + /* Look up the bundle with all the connections to the same host this - connectdata struct is setup to use. */ + connectdata struct is setup to use. + + **NOTE**: When it returns, it holds the connection cache lock! */ struct connectbundle *Curl_conncache_find_bundle(struct connectdata *conn, struct conncache *connc) { struct connectbundle *bundle = NULL; + CONN_LOCK(conn->data); if(connc) { char key[128]; hashkey(conn, key, sizeof(key)); - CONN_LOCK(conn->data); bundle = Curl_hash_pick(&connc->hash, key, strlen(key)); - CONN_UNLOCK(conn->data); } return bundle; @@ -223,77 +268,89 @@ CURLcode Curl_conncache_add_conn(struct conncache *connc, struct connectbundle *new_bundle = NULL; struct Curl_easy *data = conn->data; + /* *find_bundle() locks the connection cache */ bundle = Curl_conncache_find_bundle(conn, data->state.conn_cache); if(!bundle) { int rc; char key[128]; result = bundle_create(data, &new_bundle); - if(result) - return result; + if(result) { + goto unlock; + } hashkey(conn, key, sizeof(key)); - CONN_LOCK(data); rc = conncache_add_bundle(data->state.conn_cache, key, new_bundle); - CONN_UNLOCK(data); if(!rc) { bundle_destroy(new_bundle); - return CURLE_OUT_OF_MEMORY; + result = CURLE_OUT_OF_MEMORY; + goto unlock; } bundle = new_bundle; } - CONN_LOCK(data); result = bundle_add_conn(bundle, conn); if(result) { if(new_bundle) conncache_remove_bundle(data->state.conn_cache, new_bundle); - CONN_UNLOCK(data); - return result; + goto unlock; } - CONN_UNLOCK(data); conn->connection_id = connc->next_connection_id++; - connc->num_connections++; + connc->num_conn++; DEBUGF(infof(conn->data, "Added connection %ld. " "The cache now contains %" CURL_FORMAT_CURL_OFF_TU " members\n", - conn->connection_id, (curl_off_t) connc->num_connections)); + conn->connection_id, (curl_off_t) connc->num_conn)); + + unlock: + CONN_UNLOCK(data); return CURLE_OK; } -void Curl_conncache_remove_conn(struct conncache *connc, - struct connectdata *conn) +void Curl_conncache_remove_conn(struct connectdata *conn, bool lock) { + struct Curl_easy *data = conn->data; struct connectbundle *bundle = conn->bundle; + struct conncache *connc = data->state.conn_cache; /* The bundle pointer can be NULL, since this function can be called due to a failed connection attempt, before being added to a bundle */ if(bundle) { - CONN_LOCK(conn->data); + if(lock) { + CONN_LOCK(conn->data); + } bundle_remove_conn(bundle, conn); if(bundle->num_connections == 0) conncache_remove_bundle(connc, bundle); - CONN_UNLOCK(conn->data); + conn->bundle = NULL; /* removed from it */ if(connc) { - connc->num_connections--; - + connc->num_conn--; DEBUGF(infof(conn->data, "The cache now contains %" CURL_FORMAT_CURL_OFF_TU " members\n", - (curl_off_t) connc->num_connections)); + (curl_off_t) connc->num_conn)); + } + if(lock) { + CONN_UNLOCK(conn->data); } } } -/* This function iterates the entire connection cache and calls the - function func() with the connection pointer as the first argument - and the supplied 'param' argument as the other, +/* This function iterates the entire connection cache and calls the function + func() with the connection pointer as the first argument and the supplied + 'param' argument as the other. + + The conncache lock is still held when the callback is called. It needs it, + so that it can safely continue traversing the lists once the callback + returns. + + Returns 1 if the loop was aborted due to the callback's return code. Return 0 from func() to continue the loop, return 1 to abort it. */ -void Curl_conncache_foreach(struct Curl_easy *data, +bool Curl_conncache_foreach(struct Curl_easy *data, struct conncache *connc, void *param, int (*func)(struct connectdata *conn, void *param)) @@ -303,7 +360,7 @@ void Curl_conncache_foreach(struct Curl_easy *data, struct curl_hash_element *he; if(!connc) - return; + return FALSE; CONN_LOCK(data); Curl_hash_start_iterate(&connc->hash, &iter); @@ -324,11 +381,12 @@ void Curl_conncache_foreach(struct Curl_easy *data, if(1 == func(conn, param)) { CONN_UNLOCK(data); - return; + return TRUE; } } } CONN_UNLOCK(data); + return FALSE; } /* Return the first connection found in the cache. Used when closing all @@ -363,16 +421,104 @@ Curl_conncache_find_first_connection(struct conncache *connc) } /* - * This function finds the connection in the connection - * cache that has been unused for the longest time. + * Give ownership of a connection back to the connection cache. Might + * disconnect the oldest existing in there to make space. + * + * Return TRUE if stored, FALSE if closed. + */ +bool Curl_conncache_return_conn(struct connectdata *conn) +{ + struct Curl_easy *data = conn->data; + + /* data->multi->maxconnects can be negative, deal with it. */ + size_t maxconnects = + (data->multi->maxconnects < 0) ? data->multi->num_easy * 4: + data->multi->maxconnects; + struct connectdata *conn_candidate = NULL; + + if(maxconnects > 0 && + Curl_conncache_size(data) > maxconnects) { + infof(data, "Connection cache is full, closing the oldest one.\n"); + + conn_candidate = Curl_conncache_extract_oldest(data); + + if(conn_candidate) { + /* Set the connection's owner correctly */ + conn_candidate->data = data; + + /* the winner gets the honour of being disconnected */ + (void)Curl_disconnect(conn_candidate, /* dead_connection */ FALSE); + } + } + CONN_LOCK(data); + conn->inuse = FALSE; /* Mark the connection unused */ + CONN_UNLOCK(data); + + return (conn_candidate == conn) ? FALSE : TRUE; + +} + +/* + * This function finds the connection in the connection bundle that has been + * unused for the longest time. + * + * Does not lock the connection cache! * * Returns the pointer to the oldest idle connection, or NULL if none was * found. */ struct connectdata * -Curl_conncache_oldest_idle(struct Curl_easy *data) +Curl_conncache_extract_bundle(struct Curl_easy *data, + struct connectbundle *bundle) +{ + struct curl_llist_element *curr; + timediff_t highscore = -1; + timediff_t score; + struct curltime now; + struct connectdata *conn_candidate = NULL; + struct connectdata *conn; + + (void)data; + + now = Curl_now(); + + curr = bundle->conn_list.head; + while(curr) { + conn = curr->ptr; + + if(!conn->inuse) { + /* Set higher score for the age passed since the connection was used */ + score = Curl_timediff(now, conn->now); + + if(score > highscore) { + highscore = score; + conn_candidate = conn; + } + } + curr = curr->next; + } + if(conn_candidate) { + /* remove it to prevent another thread from nicking it */ + bundle_remove_conn(bundle, conn_candidate); + data->state.conn_cache->num_conn--; + DEBUGF(infof(data, "The cache now contains %" + CURL_FORMAT_CURL_OFF_TU " members\n", + (curl_off_t) data->state.conn_cache->num_conn)); + } + + return conn_candidate; +} + +/* + * This function finds the connection in the connection cache that has been + * unused for the longest time and extracts that from the bundle. + * + * Returns the pointer to the connection, or NULL if none was found. + */ +struct connectdata * +Curl_conncache_extract_oldest(struct Curl_easy *data) { - struct conncache *bc = data->state.conn_cache; + struct conncache *connc = data->state.conn_cache; struct curl_hash_iterator iter; struct curl_llist_element *curr; struct curl_hash_element *he; @@ -381,11 +527,12 @@ Curl_conncache_oldest_idle(struct Curl_easy *data) struct curltime now; struct connectdata *conn_candidate = NULL; struct connectbundle *bundle; + struct connectbundle *bundle_candidate = NULL; now = Curl_now(); CONN_LOCK(data); - Curl_hash_start_iterate(&bc->hash, &iter); + Curl_hash_start_iterate(&connc->hash, &iter); he = Curl_hash_next_element(&iter); while(he) { @@ -404,6 +551,7 @@ Curl_conncache_oldest_idle(struct Curl_easy *data) if(score > highscore) { highscore = score; conn_candidate = conn; + bundle_candidate = bundle; } } curr = curr->next; @@ -411,6 +559,14 @@ Curl_conncache_oldest_idle(struct Curl_easy *data) he = Curl_hash_next_element(&iter); } + if(conn_candidate) { + /* remove it to prevent another thread from nicking it */ + bundle_remove_conn(bundle_candidate, conn_candidate); + connc->num_conn--; + DEBUGF(infof(data, "The cache now contains %" + CURL_FORMAT_CURL_OFF_TU " members\n", + (curl_off_t) connc->num_conn)); + } CONN_UNLOCK(data); return conn_candidate; diff --git a/lib/conncache.h b/lib/conncache.h index 0d97a6cef..d8ad80f96 100644 --- a/lib/conncache.h +++ b/lib/conncache.h @@ -23,9 +23,15 @@ * ***************************************************************************/ +/* + * All accesses to struct fields and changing of data in the connection cache + * and connectbundles must be done with the conncache LOCKED. The cache might + * be shared. + */ + struct conncache { struct curl_hash hash; - size_t num_connections; + size_t num_conn; long next_connection_id; struct curltime last_cleanup; /* handle used for closing cached connections */ @@ -50,14 +56,17 @@ void Curl_conncache_destroy(struct conncache *connc); /* return the correct bundle, to a host or a proxy */ struct connectbundle *Curl_conncache_find_bundle(struct connectdata *conn, struct conncache *connc); +void Curl_conncache_unlock(struct connectdata *conn); +/* returns number of connections currently held in the connection cache */ +size_t Curl_conncache_size(struct Curl_easy *data); +size_t Curl_conncache_bundle_size(struct connectdata *conn); +bool Curl_conncache_return_conn(struct connectdata *conn); CURLcode Curl_conncache_add_conn(struct conncache *connc, struct connectdata *conn); - -void Curl_conncache_remove_conn(struct conncache *connc, - struct connectdata *conn); - -void Curl_conncache_foreach(struct Curl_easy *data, +void Curl_conncache_remove_conn(struct connectdata *conn, + bool lock); +bool Curl_conncache_foreach(struct Curl_easy *data, struct conncache *connc, void *param, int (*func)(struct connectdata *conn, @@ -67,7 +76,10 @@ struct connectdata * Curl_conncache_find_first_connection(struct conncache *connc); struct connectdata * -Curl_conncache_oldest_idle(struct Curl_easy *data); +Curl_conncache_extract_bundle(struct Curl_easy *data, + struct connectbundle *bundle); +struct connectdata * +Curl_conncache_extract_oldest(struct Curl_easy *data); void Curl_conncache_close_all_connections(struct conncache *connc); void Curl_conncache_print(struct conncache *connc); diff --git a/lib/multi.c b/lib/multi.c index 9728e5a2f..1a4618eb2 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -479,38 +479,6 @@ static void debug_print_sock_hash(void *p) } #endif -/* Mark the connection as 'idle', or close it if the cache is full. - Returns TRUE if the connection is kept, or FALSE if it was closed. */ -static bool -ConnectionDone(struct Curl_easy *data, struct connectdata *conn) -{ - /* data->multi->maxconnects can be negative, deal with it. */ - size_t maxconnects = - (data->multi->maxconnects < 0) ? data->multi->num_easy * 4: - data->multi->maxconnects; - struct connectdata *conn_candidate = NULL; - - /* Mark the current connection as 'unused' */ - conn->inuse = FALSE; - - if(maxconnects > 0 && - data->state.conn_cache->num_connections > maxconnects) { - infof(data, "Connection cache is full, closing the oldest one.\n"); - - conn_candidate = Curl_conncache_oldest_idle(data); - - if(conn_candidate) { - /* Set the connection's owner correctly */ - conn_candidate->data = data; - - /* the winner gets the honour of being disconnected */ - (void)Curl_disconnect(conn_candidate, /* dead_connection */ FALSE); - } - } - - return (conn_candidate == conn) ? FALSE : TRUE; -} - static CURLcode multi_done(struct connectdata **connp, CURLcode status, /* an error if this is called after an error was detected */ @@ -621,17 +589,21 @@ static CURLcode multi_done(struct connectdata **connp, result = res2; } else { + char buffer[256]; + /* create string before returning the connection */ + snprintf(buffer, sizeof(buffer), + "Connection #%ld to host %s left intact", + conn->connection_id, + conn->bits.socksproxy ? conn->socks_proxy.host.dispname : + conn->bits.httpproxy ? conn->http_proxy.host.dispname : + conn->bits.conn_to_host ? conn->conn_to_host.dispname : + conn->host.dispname); + /* the connection is no longer in use */ - if(ConnectionDone(data, conn)) { + if(Curl_conncache_return_conn(conn)) { /* remember the most recently used connection */ data->state.lastconnect = conn; - - infof(data, "Connection #%ld to host %s left intact\n", - conn->connection_id, - conn->bits.socksproxy ? conn->socks_proxy.host.dispname : - conn->bits.httpproxy ? conn->http_proxy.host.dispname : - conn->bits.conn_to_host ? conn->conn_to_host.dispname : - conn->host.dispname); + infof(data, "%s\n", buffer); } else data->state.lastconnect = NULL; @@ -1357,16 +1329,16 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, } if(data->easy_conn && data->mstate > CURLM_STATE_CONNECT && - data->mstate < CURLM_STATE_COMPLETED) + data->mstate < CURLM_STATE_COMPLETED) { /* Make sure we set the connection's current owner */ data->easy_conn->data = data; + } if(data->easy_conn && (data->mstate >= CURLM_STATE_CONNECT) && (data->mstate < CURLM_STATE_COMPLETED)) { /* we need to wait for the connect state as only then is the start time stored, but we must not check already completed handles */ - timeout_ms = Curl_timeleft(data, &now, (data->mstate <= CURLM_STATE_WAITDO)? TRUE:FALSE); @@ -127,10 +127,6 @@ bool curl_win32_idn_to_ascii(const char *in, char **out); #include "curl_memory.h" #include "memdebug.h" -/* Local static prototypes */ -static struct connectdata * -find_oldest_idle_connection_in_bundle(struct Curl_easy *data, - struct connectbundle *bundle); static void conn_free(struct connectdata *conn); static void free_fixed_hostname(struct hostname *host); static void signalPipeClose(struct curl_llist *pipeline, bool pipe_broke); @@ -722,7 +718,6 @@ static void conn_free(struct connectdata *conn) #ifdef USE_SSL Curl_safefree(conn->ssl_extra); #endif - free(conn); /* free all the connection oriented data */ } @@ -777,7 +772,7 @@ CURLcode Curl_disconnect(struct connectdata *conn, bool dead_connection) /* unlink ourselves! */ infof(data, "Closing connection %ld\n", conn->connection_id); - Curl_conncache_remove_conn(data->state.conn_cache, conn); + Curl_conncache_remove_conn(conn, TRUE); free_fixed_hostname(&conn->host); free_fixed_hostname(&conn->conn_to_host); @@ -943,56 +938,17 @@ proxy_info_matches(const struct proxy_info* data, return FALSE; } - /* - * This function finds the connection in the connection - * bundle that has been unused for the longest time. + * This function checks if the given connection is dead and extracts it from + * the connection cache if so. * - * Returns the pointer to the oldest idle connection, or NULL if none was - * found. - */ -static struct connectdata * -find_oldest_idle_connection_in_bundle(struct Curl_easy *data, - struct connectbundle *bundle) -{ - struct curl_llist_element *curr; - timediff_t highscore = -1; - timediff_t score; - struct curltime now; - struct connectdata *conn_candidate = NULL; - struct connectdata *conn; - - (void)data; - - now = Curl_now(); - - curr = bundle->conn_list.head; - while(curr) { - conn = curr->ptr; - - if(!conn->inuse) { - /* Set higher score for the age passed since the connection was used */ - score = Curl_timediff(now, conn->now); - - if(score > highscore) { - highscore = score; - conn_candidate = conn; - } - } - curr = curr->next; - } - - return conn_candidate; -} - -/* - * This function checks if given connection is dead and disconnects if so. - * (That also removes it from the connection cache.) + * When this is called as a Curl_conncache_foreach() callback, the connection + * cache lock is held! * - * Returns TRUE if the connection actually was dead and disconnected. + * Returns TRUE if the connection was dead and extracted. */ -static bool disconnect_if_dead(struct connectdata *conn, - struct Curl_easy *data) +static bool extract_if_dead(struct connectdata *conn, + struct Curl_easy *data) { size_t pipeLen = conn->send_pipe.size + conn->recv_pipe.size; if(!pipeLen && !conn->inuse) { @@ -1017,25 +973,30 @@ static bool disconnect_if_dead(struct connectdata *conn, if(dead) { conn->data = data; infof(data, "Connection %ld seems to be dead!\n", conn->connection_id); - - /* disconnect resources */ - Curl_disconnect(conn, /* dead_connection */TRUE); + Curl_conncache_remove_conn(conn, FALSE); return TRUE; } } return FALSE; } +struct prunedead { + struct Curl_easy *data; + struct connectdata *extracted; +}; + /* - * Wrapper to use disconnect_if_dead() function in Curl_conncache_foreach() + * Wrapper to use extract_if_dead() function in Curl_conncache_foreach() * - * Returns always 0. */ -static int call_disconnect_if_dead(struct connectdata *conn, - void *param) +static int call_extract_if_dead(struct connectdata *conn, void *param) { - struct Curl_easy* data = (struct Curl_easy*)param; - disconnect_if_dead(conn, data); + struct prunedead *p = (struct prunedead *)param; + if(extract_if_dead(conn, p->data)) { + /* stop the iteration here, pass back the connection that was extracted */ + p->extracted = conn; + return 1; + } return 0; /* continue iteration */ } @@ -1050,8 +1011,14 @@ static void prune_dead_connections(struct Curl_easy *data) time_t elapsed = Curl_timediff(now, data->state.conn_cache->last_cleanup); if(elapsed >= 1000L) { - Curl_conncache_foreach(data, data->state.conn_cache, data, - call_disconnect_if_dead); + struct prunedead prune; + prune.data = data; + prune.extracted = NULL; + while(Curl_conncache_foreach(data, data->state.conn_cache, &prune, + call_extract_if_dead)) { + /* disconnect it */ + (void)Curl_disconnect(prune.extracted, /* dead_connection */TRUE); + } data->state.conn_cache->last_cleanup = now; } } @@ -1106,8 +1073,8 @@ ConnectionExists(struct Curl_easy *data, Curl_pipeline_site_blacklisted(data, needle)) canpipe &= ~ CURLPIPE_HTTP1; - /* Look up the bundle with all the connections to this - particular host */ + /* Look up the bundle with all the connections to this particular host. + Locks the connection cache, beware of early returns! */ bundle = Curl_conncache_find_bundle(needle, data->state.conn_cache); if(bundle) { /* Max pipe length is zero (unlimited) for multiplexed connections */ @@ -1130,6 +1097,7 @@ ConnectionExists(struct Curl_easy *data, if((bundle->multiuse == BUNDLE_UNKNOWN) && data->set.pipewait) { infof(data, "Server doesn't support multi-use yet, wait\n"); *waitpipe = TRUE; + Curl_conncache_unlock(needle); return FALSE; /* no re-use */ } @@ -1161,8 +1129,11 @@ ConnectionExists(struct Curl_easy *data, check = curr->ptr; curr = curr->next; - if(disconnect_if_dead(check, data)) + if(extract_if_dead(check, data)) { + /* disconnect it */ + (void)Curl_disconnect(check, /* dead_connection */TRUE); continue; + } pipeLen = check->send_pipe.size + check->recv_pipe.size; @@ -1479,9 +1450,13 @@ ConnectionExists(struct Curl_easy *data, } if(chosen) { + /* mark it as used before releasing the lock */ + chosen->inuse = TRUE; + Curl_conncache_unlock(needle); *usethis = chosen; return TRUE; /* yes, we found one to use! */ } + Curl_conncache_unlock(needle); if(foundPendingCandidate && data->set.pipewait) { infof(data, @@ -4409,20 +4384,21 @@ static CURLcode create_conn(struct Curl_easy *data, else reuse = ConnectionExists(data, conn, &conn_temp, &force_reuse, &waitpipe); - /* If we found a reusable connection, we may still want to - open a new connection if we are pipelining. */ + /* If we found a reusable connection that is now marked as in use, we may + still want to open a new connection if we are pipelining. */ if(reuse && !force_reuse && IsPipeliningPossible(data, conn_temp)) { size_t pipelen = conn_temp->send_pipe.size + conn_temp->recv_pipe.size; if(pipelen > 0) { infof(data, "Found connection %ld, with requests in the pipe (%zu)\n", conn_temp->connection_id, pipelen); - if(conn_temp->bundle->num_connections < max_host_connections && - data->state.conn_cache->num_connections < max_total_connections) { + if(Curl_conncache_bundle_size(conn_temp) < max_host_connections && + Curl_conncache_size(data) < max_total_connections) { /* We want a new connection anyway */ reuse = FALSE; infof(data, "We can reuse, but we want a new connection anyway\n"); + Curl_conncache_return_conn(conn_temp); } } } @@ -4434,8 +4410,6 @@ static CURLcode create_conn(struct Curl_easy *data, * just allocated before we can move along and use the previously * existing one. */ - conn_temp->inuse = TRUE; /* mark this as being in use so that no other - handle in a multi stack may nick it */ reuse_conn(conn, conn_temp); #ifdef USE_SSL free(conn->ssl_extra); @@ -4455,7 +4429,6 @@ static CURLcode create_conn(struct Curl_easy *data, /* We have decided that we want a new connection. However, we may not be able to do that if we have reached the limit of how many connections we are allowed to open. */ - struct connectbundle *bundle = NULL; if(conn->handler->flags & PROTOPT_ALPN_NPN) { /* The protocol wants it, so set the bits if enabled in the easy handle @@ -4470,35 +4443,42 @@ static CURLcode create_conn(struct Curl_easy *data, /* There is a connection that *might* become usable for pipelining "soon", and we wait for that */ connections_available = FALSE; - else - bundle = Curl_conncache_find_bundle(conn, data->state.conn_cache); - - if(max_host_connections > 0 && bundle && - (bundle->num_connections >= max_host_connections)) { - struct connectdata *conn_candidate; - - /* The bundle is full. Let's see if we can kill a connection. */ - conn_candidate = find_oldest_idle_connection_in_bundle(data, bundle); - - if(conn_candidate) { - /* Set the connection's owner correctly, then kill it */ - conn_candidate->data = data; - (void)Curl_disconnect(conn_candidate, /* dead_connection */ FALSE); - } - else { - infof(data, "No more connections allowed to host: %d\n", - max_host_connections); - connections_available = FALSE; + else { + /* this gets a lock on the conncache */ + struct connectbundle *bundle = + Curl_conncache_find_bundle(conn, data->state.conn_cache); + + if(max_host_connections > 0 && bundle && + (bundle->num_connections >= max_host_connections)) { + struct connectdata *conn_candidate; + + /* The bundle is full. Extract the oldest connection. */ + conn_candidate = Curl_conncache_extract_bundle(data, bundle); + Curl_conncache_unlock(conn); + + if(conn_candidate) { + /* Set the connection's owner correctly, then kill it */ + conn_candidate->data = data; + (void)Curl_disconnect(conn_candidate, /* dead_connection */ FALSE); + } + else { + infof(data, "No more connections allowed to host: %d\n", + max_host_connections); + connections_available = FALSE; + } } + else + Curl_conncache_unlock(conn); + } if(connections_available && (max_total_connections > 0) && - (data->state.conn_cache->num_connections >= max_total_connections)) { + (Curl_conncache_size(data) >= max_total_connections)) { struct connectdata *conn_candidate; /* The cache is full. Let's see if we can kill a connection. */ - conn_candidate = Curl_conncache_oldest_idle(data); + conn_candidate = Curl_conncache_extract_oldest(data); if(conn_candidate) { /* Set the connection's owner correctly, then kill it */ @@ -4521,6 +4501,9 @@ static CURLcode create_conn(struct Curl_easy *data, goto out; } else { + /* Mark the connection as used, before we add it */ + conn->inuse = TRUE; + /* * This is a brand new connection, so let's store it in the connection * cache of ours! @@ -4548,9 +4531,6 @@ static CURLcode create_conn(struct Curl_easy *data, #endif } - /* Mark the connection as used */ - conn->inuse = TRUE; - /* Setup and init stuff before DO starts, in preparing for the transfer. */ Curl_init_do(data, conn); diff --git a/lib/urldata.h b/lib/urldata.h index ed6bbb4f0..19cb6cafd 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -775,9 +775,10 @@ struct connectdata { void *closesocket_client; bool inuse; /* This is a marker for the connection cache logic. If this is - TRUE this handle is being used by an easy handle and cannot - be used by any other easy handle without careful - consideration (== only for pipelining). */ + TRUE this handle is being used by one or more easy handles + and can only used by any other easy handle without careful + consideration (== only for pipelining/multiplexing) and it + cannot be used by another multi handle! */ /**** Fields set when inited and not modified again */ long connection_id; /* Contains a unique number to make it easier to @@ -1325,6 +1326,9 @@ struct UrlState { struct Curl_easy *stream_depends_on; bool stream_depends_e; /* set or don't set the Exclusive bit */ int stream_weight; +#ifdef CURLDEBUG + bool conncache_lock; +#endif }; diff --git a/tests/data/test1554 b/tests/data/test1554 index 8739b2c8a..06f189724 100644 --- a/tests/data/test1554 +++ b/tests/data/test1554 @@ -29,11 +29,11 @@ run 1: foobar and so on fun! <- Mutex unlock -> Mutex lock <- Mutex unlock +run 1: foobar and so on fun! -> Mutex lock <- Mutex unlock -> Mutex lock <- Mutex unlock -run 1: foobar and so on fun! -> Mutex lock <- Mutex unlock -> Mutex lock @@ -47,11 +47,19 @@ run 1: foobar and so on fun! <- Mutex unlock -> Mutex lock <- Mutex unlock +-> Mutex lock +<- Mutex unlock +-> Mutex lock +<- Mutex unlock run 1: foobar and so on fun! -> Mutex lock <- Mutex unlock -> Mutex lock <- Mutex unlock +-> Mutex lock +<- Mutex unlock +-> Mutex lock +<- Mutex unlock </datacheck> </reply> |