moparisthebest/curl
1
0
Fork 0
mirror of https://github.com/moparisthebest/curl synced 2025-02-28 17:31:46 -05:00
Code Issues Releases Wiki Activity

conncache: various concept cleanups

Browse Source 
More connection cache accesses are protected by locks.

CONNCACHE_* is a beter prefix for the connection cache lock macros.

Curl_attach_connnection: now called as soon as there's a connection
struct available and before the connection is added to the connection
cache.

Curl_disconnect: now assumes that the connection is already removed from
the connection cache.

Ref: #4915
Closes #5009
This commit is contained in:
Daniel Stenberg 2020-04-27 00:33:21 +02:00
parent 9a8fa076bf
commit c069027139
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
10 changed files with 139 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

87
lib/conncache.c
View File

@ -49,53 +49,51 @@ static void conn_llist_dtor(void *user, void *element)
conn->bundle = NULL; conn->bundle = NULL;
} }
static CURLcode bundle_create(struct Curl_easy *data, static CURLcode bundle_create(struct connectbundle **bundlep)
struct connectbundle **cb_ptr)
{ {
(void)data; DEBUGASSERT(*bundlep == NULL);
DEBUGASSERT(*cb_ptr == NULL); *bundlep = malloc(sizeof(struct connectbundle));
*cb_ptr = malloc(sizeof(struct connectbundle)); if(!*bundlep)
if(!*cb_ptr)
return CURLE_OUT_OF_MEMORY; return CURLE_OUT_OF_MEMORY;
(*cb_ptr)->num_connections = 0; (*bundlep)->num_connections = 0;
(*cb_ptr)->multiuse = BUNDLE_UNKNOWN; (*bundlep)->multiuse = BUNDLE_UNKNOWN;
Curl_llist_init(&(*cb_ptr)->conn_list, (curl_llist_dtor) conn_llist_dtor); Curl_llist_init(&(*bundlep)->conn_list, (curl_llist_dtor) conn_llist_dtor);
return CURLE_OK; return CURLE_OK;
} }
static void bundle_destroy(struct connectbundle *cb_ptr) static void bundle_destroy(struct connectbundle *bundle)
{ {
if(!cb_ptr) if(!bundle)
return; return;
Curl_llist_destroy(&cb_ptr->conn_list, NULL); Curl_llist_destroy(&bundle->conn_list, NULL);
free(cb_ptr); free(bundle);
} }
/* Add a connection to a bundle */ /* Add a connection to a bundle */
static void bundle_add_conn(struct connectbundle *cb_ptr, static void bundle_add_conn(struct connectbundle *bundle,
struct connectdata *conn) struct connectdata *conn)
{ {
Curl_llist_insert_next(&cb_ptr->conn_list, cb_ptr->conn_list.tail, conn, Curl_llist_insert_next(&bundle->conn_list, bundle->conn_list.tail, conn,
&conn->bundle_node); &conn->bundle_node);
conn->bundle = cb_ptr; conn->bundle = bundle;
cb_ptr->num_connections++; bundle->num_connections++;
} }
/* Remove a connection from a bundle */ /* Remove a connection from a bundle */
static int bundle_remove_conn(struct connectbundle *cb_ptr, static int bundle_remove_conn(struct connectbundle *bundle,
struct connectdata *conn) struct connectdata *conn)
{ {
struct curl_llist_element *curr; struct curl_llist_element *curr;
curr = cb_ptr->conn_list.head; curr = bundle->conn_list.head;
while(curr) { while(curr) {
if(curr->ptr == conn) { if(curr->ptr == conn) {
Curl_llist_remove(&cb_ptr->conn_list, curr, NULL); Curl_llist_remove(&bundle->conn_list, curr, NULL);
cb_ptr->num_connections--; bundle->num_connections--;
conn->bundle = NULL; conn->bundle = NULL;
return 1; /* we removed a handle */ return 1; /* we removed a handle */
} }
@ -162,20 +160,15 @@ static void hashkey(struct connectdata *conn, char *buf,
msnprintf(buf, len, "%ld%s", port, hostname); msnprintf(buf, len, "%ld%s", port, hostname);
} }
void Curl_conncache_unlock(struct Curl_easy *data)
{
CONN_UNLOCK(data);
}
/* Returns number of connections currently held in the connection cache. /* Returns number of connections currently held in the connection cache.
Locks/unlocks the cache itself! Locks/unlocks the cache itself!
*/ */
size_t Curl_conncache_size(struct Curl_easy *data) size_t Curl_conncache_size(struct Curl_easy *data)
{ {
size_t num; size_t num;
CONN_LOCK(data); CONNCACHE_LOCK(data);
num = data->state.conn_cache->num_conn; num = data->state.conn_cache->num_conn;
CONN_UNLOCK(data); CONNCACHE_UNLOCK(data);
return num; return num;
} }
@ -188,7 +181,7 @@ struct connectbundle *Curl_conncache_find_bundle(struct connectdata *conn,
const char **hostp) const char **hostp)
{ {
struct connectbundle *bundle = NULL; struct connectbundle *bundle = NULL;
CONN_LOCK(conn->data); CONNCACHE_LOCK(conn->data);
if(connc) { if(connc) {
char key[HASHKEY_SIZE]; char key[HASHKEY_SIZE];
hashkey(conn, key, sizeof(key), hostp); hashkey(conn, key, sizeof(key), hostp);
@ -235,8 +228,7 @@ CURLcode Curl_conncache_add_conn(struct conncache *connc,
struct connectdata *conn) struct connectdata *conn)
{ {
CURLcode result = CURLE_OK; CURLcode result = CURLE_OK;
struct connectbundle *bundle; struct connectbundle *bundle = NULL;
struct connectbundle *new_bundle = NULL;
struct Curl_easy *data = conn->data; struct Curl_easy *data = conn->data;
/* *find_bundle() locks the connection cache */ /* *find_bundle() locks the connection cache */
@ -245,20 +237,19 @@ CURLcode Curl_conncache_add_conn(struct conncache *connc,
int rc; int rc;
char key[HASHKEY_SIZE]; char key[HASHKEY_SIZE];
result = bundle_create(data, &new_bundle); result = bundle_create(&bundle);
if(result) { if(result) {
goto unlock; goto unlock;
} }
hashkey(conn, key, sizeof(key), NULL); hashkey(conn, key, sizeof(key), NULL);
rc = conncache_add_bundle(data->state.conn_cache, key, new_bundle); rc = conncache_add_bundle(data->state.conn_cache, key, bundle);
if(!rc) { if(!rc) {
bundle_destroy(new_bundle); bundle_destroy(bundle);
result = CURLE_OUT_OF_MEMORY; result = CURLE_OUT_OF_MEMORY;
goto unlock; goto unlock;
} }
bundle = new_bundle;
} }
bundle_add_conn(bundle, conn); bundle_add_conn(bundle, conn);
@ -270,15 +261,17 @@ CURLcode Curl_conncache_add_conn(struct conncache *connc,
conn->connection_id, connc->num_conn)); conn->connection_id, connc->num_conn));
unlock: unlock:
CONN_UNLOCK(data); CONNCACHE_UNLOCK(data);
return result; return result;
} }
/* /*
* Removes the connectdata object from the connection cache *and* clears the * Removes the connectdata object from the connection cache, but does *not*
* ->data pointer association. Pass TRUE/FALSE in the 'lock' argument * clear the conn->data association. The transfer still owns this connection.
* depending on if the parent function already holds the lock or not. *
* Pass TRUE/FALSE in the 'lock' argument depending on if the parent function
* already holds the lock or not.
*/ */
void Curl_conncache_remove_conn(struct Curl_easy *data, void Curl_conncache_remove_conn(struct Curl_easy *data,
struct connectdata *conn, bool lock) struct connectdata *conn, bool lock)
@ -290,7 +283,7 @@ void Curl_conncache_remove_conn(struct Curl_easy *data,
due to a failed connection attempt, before being added to a bundle */ due to a failed connection attempt, before being added to a bundle */
if(bundle) { if(bundle) {
if(lock) { if(lock) {
CONN_LOCK(data); CONNCACHE_LOCK(data);
} }
bundle_remove_conn(bundle, conn); bundle_remove_conn(bundle, conn);
if(bundle->num_connections == 0) if(bundle->num_connections == 0)
@ -301,9 +294,8 @@ void Curl_conncache_remove_conn(struct Curl_easy *data,
DEBUGF(infof(data, "The cache now contains %zu members\n", DEBUGF(infof(data, "The cache now contains %zu members\n",
connc->num_conn)); connc->num_conn));
} }
conn->data = NULL; /* clear the association */
if(lock) { if(lock) {
CONN_UNLOCK(data); CONNCACHE_UNLOCK(data);
} }
} }
} }
@ -332,7 +324,7 @@ bool Curl_conncache_foreach(struct Curl_easy *data,
if(!connc) if(!connc)
return FALSE; return FALSE;
CONN_LOCK(data); CONNCACHE_LOCK(data);
Curl_hash_start_iterate(&connc->hash, &iter); Curl_hash_start_iterate(&connc->hash, &iter);
he = Curl_hash_next_element(&iter); he = Curl_hash_next_element(&iter);
@ -350,12 +342,12 @@ bool Curl_conncache_foreach(struct Curl_easy *data,
curr = curr->next; curr = curr->next;
if(1 == func(conn, param)) { if(1 == func(conn, param)) {
CONN_UNLOCK(data); CONNCACHE_UNLOCK(data);
return TRUE; return TRUE;
} }
} }
} }
CONN_UNLOCK(data); CONNCACHE_UNLOCK(data);
return FALSE; return FALSE;
} }
@ -494,7 +486,7 @@ Curl_conncache_extract_oldest(struct Curl_easy *data)
now = Curl_now(); now = Curl_now();
CONN_LOCK(data); CONNCACHE_LOCK(data);
Curl_hash_start_iterate(&connc->hash, &iter); Curl_hash_start_iterate(&connc->hash, &iter);
he = Curl_hash_next_element(&iter); he = Curl_hash_next_element(&iter);
@ -531,7 +523,7 @@ Curl_conncache_extract_oldest(struct Curl_easy *data)
connc->num_conn)); connc->num_conn));
conn_candidate->data = data; /* associate! */ conn_candidate->data = data; /* associate! */
} }
CONN_UNLOCK(data); CONNCACHE_UNLOCK(data);
return conn_candidate; return conn_candidate;
} }
@ -548,6 +540,7 @@ void Curl_conncache_close_all_connections(struct conncache *connc)
sigpipe_ignore(conn->data, &pipe_st); sigpipe_ignore(conn->data, &pipe_st);
/* This will remove the connection from the cache */ /* This will remove the connection from the cache */
connclose(conn, "kill all"); connclose(conn, "kill all");
Curl_conncache_remove_conn(conn->data, conn, TRUE);
(void)Curl_disconnect(connc->closure_handle, conn, FALSE); (void)Curl_disconnect(connc->closure_handle, conn, FALSE);
sigpipe_restore(&pipe_st); sigpipe_restore(&pipe_st);

9
lib/conncache.h
View File

@ -45,21 +45,21 @@ struct conncache {
#ifdef CURLDEBUG #ifdef CURLDEBUG
/* the debug versions of these macros make extra certain that the lock is /* the debug versions of these macros make extra certain that the lock is
never doubly locked or unlocked */ never doubly locked or unlocked */
#define CONN_LOCK(x) if((x)->share) { \ #define CONNCACHE_LOCK(x) if((x)->share) { \
Curl_share_lock((x), CURL_LOCK_DATA_CONNECT, CURL_LOCK_ACCESS_SINGLE); \ Curl_share_lock((x), CURL_LOCK_DATA_CONNECT, CURL_LOCK_ACCESS_SINGLE); \
DEBUGASSERT(!(x)->state.conncache_lock); \ DEBUGASSERT(!(x)->state.conncache_lock); \
(x)->state.conncache_lock = TRUE; \ (x)->state.conncache_lock = TRUE; \
} }
#define CONN_UNLOCK(x) if((x)->share) { \ #define CONNCACHE_UNLOCK(x) if((x)->share) { \
DEBUGASSERT((x)->state.conncache_lock); \ DEBUGASSERT((x)->state.conncache_lock); \
(x)->state.conncache_lock = FALSE; \ (x)->state.conncache_lock = FALSE; \
Curl_share_unlock((x), CURL_LOCK_DATA_CONNECT); \ Curl_share_unlock((x), CURL_LOCK_DATA_CONNECT); \
} }
#else #else
#define CONN_LOCK(x) if((x)->share) \ #define CONNCACHE_LOCK(x) if((x)->share) \
Curl_share_lock((x), CURL_LOCK_DATA_CONNECT, CURL_LOCK_ACCESS_SINGLE) Curl_share_lock((x), CURL_LOCK_DATA_CONNECT, CURL_LOCK_ACCESS_SINGLE)
#define CONN_UNLOCK(x) if((x)->share) \ #define CONNCACHE_UNLOCK(x) if((x)->share) \
Curl_share_unlock((x), CURL_LOCK_DATA_CONNECT) Curl_share_unlock((x), CURL_LOCK_DATA_CONNECT)
#endif #endif
@ -77,7 +77,6 @@ void Curl_conncache_destroy(struct conncache *connc);
struct connectbundle *Curl_conncache_find_bundle(struct connectdata *conn, struct connectbundle *Curl_conncache_find_bundle(struct connectdata *conn,
struct conncache *connc, struct conncache *connc,
const char **hostp); const char **hostp);
void Curl_conncache_unlock(struct Curl_easy *data);
/* returns number of connections currently held in the connection cache */ /* returns number of connections currently held in the connection cache */
size_t Curl_conncache_size(struct Curl_easy *data); size_t Curl_conncache_size(struct Curl_easy *data);

12
lib/hostip.c
View File

@ -1085,10 +1085,12 @@ CURLcode Curl_once_resolved(struct connectdata *conn,
result = Curl_setup_conn(conn, protocol_done); result = Curl_setup_conn(conn, protocol_done);
if(result) if(result) {
/* We're not allowed to return failure with memory left allocated struct Curl_easy *data = conn->data;
in the connectdata struct, free those here */ DEBUGASSERT(data);
Curl_disconnect(conn->data, conn, TRUE); /* close the connection */ Curl_detach_connnection(data);
Curl_conncache_remove_conn(data, conn, TRUE);
Curl_disconnect(data, conn, TRUE);
}
return result; return result;
} }

6
lib/http_negotiate.h
View File

@ -7,7 +7,7 @@
* | (__| |_| | _ <| |___ * | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____| * \___|\___/|_| \_\_____|
* *
* Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al. * Copyright (C) 1998 - 2020, Daniel Stenberg, <daniel@haxx.se>, et al.
* *
* This software is licensed as described in the file COPYING, which * This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms * you should have received as part of this distribution. The terms
@ -33,6 +33,8 @@ CURLcode Curl_output_negotiate(struct connectdata *conn, bool proxy);
void Curl_http_auth_cleanup_negotiate(struct connectdata *conn); void Curl_http_auth_cleanup_negotiate(struct connectdata *conn);
#endif /* !CURL_DISABLE_HTTP && USE_SPNEGO */ #else /* !CURL_DISABLE_HTTP && USE_SPNEGO */
#define Curl_http_auth_cleanup_negotiate(x)
#endif
#endif /* HEADER_CURL_HTTP_NEGOTIATE_H */ #endif /* HEADER_CURL_HTTP_NEGOTIATE_H */

6
lib/http_ntlm.h
View File

@ -7,7 +7,7 @@
* | (__| |_| | _ <| |___ * | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____| * \___|\___/|_| \_\_____|
* *
* Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al. * Copyright (C) 1998 - 2020, Daniel Stenberg, <daniel@haxx.se>, et al.
* *
* This software is licensed as described in the file COPYING, which * This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms * you should have received as part of this distribution. The terms
@ -35,6 +35,8 @@ CURLcode Curl_output_ntlm(struct connectdata *conn, bool proxy);
void Curl_http_auth_cleanup_ntlm(struct connectdata *conn); void Curl_http_auth_cleanup_ntlm(struct connectdata *conn);
#endif /* !CURL_DISABLE_HTTP && USE_NTLM */ #else /* !CURL_DISABLE_HTTP && USE_NTLM */
#define Curl_http_auth_cleanup_ntlm(x)
#endif
#endif /* HEADER_CURL_HTTP_NTLM_H */ #endif /* HEADER_CURL_HTTP_NTLM_H */

56
lib/multi.c
View File

@ -79,7 +79,6 @@ static CURLMcode add_next_timeout(struct curltime now,
static CURLMcode multi_timeout(struct Curl_multi *multi, static CURLMcode multi_timeout(struct Curl_multi *multi,
long *timeout_ms); long *timeout_ms);
static void process_pending_handles(struct Curl_multi *multi); static void process_pending_handles(struct Curl_multi *multi);
static void detach_connnection(struct Curl_easy *data);
#ifdef DEBUGBUILD #ifdef DEBUGBUILD
static const char * const statename[]={ static const char * const statename[]={
@ -112,7 +111,7 @@ static void Curl_init_completed(struct Curl_easy *data)
/* Important: reset the conn pointer so that we don't point to memory /* Important: reset the conn pointer so that we don't point to memory
that could be freed anytime */ that could be freed anytime */
detach_connnection(data); Curl_detach_connnection(data);
Curl_expire_clear(data); /* stop all timers */ Curl_expire_clear(data); /* stop all timers */
} }
@ -506,6 +505,7 @@ CURLMcode curl_multi_add_handle(struct Curl_multi *multi,
easy handle is added */ easy handle is added */
memset(&multi->timer_lastcall, 0, sizeof(multi->timer_lastcall)); memset(&multi->timer_lastcall, 0, sizeof(multi->timer_lastcall));
CONNCACHE_LOCK(data);
/* The closure handle only ever has default timeouts set. To improve the /* The closure handle only ever has default timeouts set. To improve the
state somewhat we clone the timeouts from each added handle so that the state somewhat we clone the timeouts from each added handle so that the
closure handle always has the same timeouts as the most recently added closure handle always has the same timeouts as the most recently added
@ -515,6 +515,7 @@ CURLMcode curl_multi_add_handle(struct Curl_multi *multi,
data->set.server_response_timeout; data->set.server_response_timeout;
data->state.conn_cache->closure_handle->set.no_signal = data->state.conn_cache->closure_handle->set.no_signal =
data->set.no_signal; data->set.no_signal;
CONNCACHE_UNLOCK(data);
Curl_update_timer(multi); Curl_update_timer(multi);
return CURLM_OK; return CURLM_OK;
@ -589,14 +590,14 @@ static CURLcode multi_done(struct Curl_easy *data,
process_pending_handles(data->multi); /* connection / multiplex */ process_pending_handles(data->multi); /* connection / multiplex */
CONN_LOCK(data); CONNCACHE_LOCK(data);
detach_connnection(data); Curl_detach_connnection(data);
if(CONN_INUSE(conn)) { if(CONN_INUSE(conn)) {
/* Stop if still used. */ /* Stop if still used. */
/* conn->data must not remain pointing to this transfer since it is going /* conn->data must not remain pointing to this transfer since it is going
away! Find another to own it! */ away! Find another to own it! */
conn->data = conn->easyq.head->ptr; conn->data = conn->easyq.head->ptr;
CONN_UNLOCK(data); CONNCACHE_UNLOCK(data);
DEBUGF(infof(data, "Connection still in use %zu, " DEBUGF(infof(data, "Connection still in use %zu, "
"no more multi_done now!\n", "no more multi_done now!\n",
conn->easyq.size)); conn->easyq.size));
@ -647,7 +648,8 @@ static CURLcode multi_done(struct Curl_easy *data,
|| (premature && !(conn->handler->flags & PROTOPT_STREAM))) { || (premature && !(conn->handler->flags & PROTOPT_STREAM))) {
CURLcode res2; CURLcode res2;
connclose(conn, "disconnecting"); connclose(conn, "disconnecting");
CONN_UNLOCK(data); Curl_conncache_remove_conn(data, conn, FALSE);
CONNCACHE_UNLOCK(data);
res2 = Curl_disconnect(data, conn, premature); res2 = Curl_disconnect(data, conn, premature);
/* If we had an error already, make sure we return that one. But /* If we had an error already, make sure we return that one. But
@ -666,7 +668,7 @@ static CURLcode multi_done(struct Curl_easy *data,
conn->bits.conn_to_host ? conn->conn_to_host.dispname : conn->bits.conn_to_host ? conn->conn_to_host.dispname :
conn->host.dispname); conn->host.dispname);
/* the connection is no longer in use by this transfer */ /* the connection is no longer in use by this transfer */
CONN_UNLOCK(data); CONNCACHE_UNLOCK(data);
if(Curl_conncache_return_conn(data, conn)) { if(Curl_conncache_return_conn(data, conn)) {
/* remember the most recently used connection */ /* remember the most recently used connection */
data->state.lastconnect = conn; data->state.lastconnect = conn;
@ -778,8 +780,7 @@ CURLMcode curl_multi_remove_handle(struct Curl_multi *multi,
vanish with this handle */ vanish with this handle */
/* Remove the association between the connection and the handle */ /* Remove the association between the connection and the handle */
if(data->conn) Curl_detach_connnection(data);
detach_connnection(data);
#ifdef USE_LIBPSL #ifdef USE_LIBPSL
/* Remove the PSL association. */ /* Remove the PSL association. */
@ -828,9 +829,13 @@ bool Curl_multiplex_wanted(const struct Curl_multi *multi)
return (multi && (multi->multiplexing)); return (multi && (multi->multiplexing));
} }
/* This is the only function that should clear data->conn. This will /*
occasionally be called with the pointer already cleared. */ * Curl_detach_connnection() removes the given transfer from the connection.
static void detach_connnection(struct Curl_easy *data) *
* This is the only function that should clear data->conn. This will
* occasionally be called with the data->conn pointer already cleared.
*/
void Curl_detach_connnection(struct Curl_easy *data)
{ {
struct connectdata *conn = data->conn; struct connectdata *conn = data->conn;
if(conn) if(conn)
@ -838,7 +843,11 @@ static void detach_connnection(struct Curl_easy *data)
data->conn = NULL; data->conn = NULL;
} }
/* This is the only function that should assign data->conn */ /*
* Curl_attach_connnection() attaches this transfer to this connection.
*
* This is the only function that should assign data->conn
*/
void Curl_attach_connnection(struct Curl_easy *data, void Curl_attach_connnection(struct Curl_easy *data,
struct connectdata *conn) struct connectdata *conn)
{ {
@ -1540,19 +1549,6 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
bool stream_error = FALSE; bool stream_error = FALSE;
rc = CURLM_OK; rc = CURLM_OK;
DEBUGASSERT((data->mstate <= CURLM_STATE_CONNECT) ||
(data->mstate >= CURLM_STATE_DONE) ||
data->conn);
if(!data->conn &&
data->mstate > CURLM_STATE_CONNECT &&
data->mstate < CURLM_STATE_DONE) {
/* In all these states, the code will blindly access 'data->conn'
so this is precaution that it isn't NULL. And it silences static
analyzers. */
failf(data, "In state %d with no conn, bail out!\n", data->mstate);
return CURLM_INTERNAL_ERROR;
}
if(multi_ischanged(multi, TRUE)) { if(multi_ischanged(multi, TRUE)) {
DEBUGF(infof(data, "multi changed, check CONNECT_PEND queue!\n")); DEBUGF(infof(data, "multi changed, check CONNECT_PEND queue!\n"));
process_pending_handles(multi); /* multiplexed */ process_pending_handles(multi); /* multiplexed */
@ -2235,8 +2231,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
* access free'd data, if the connection is free'd and the handle * access free'd data, if the connection is free'd and the handle
* removed before we perform the processing in CURLM_STATE_COMPLETED * removed before we perform the processing in CURLM_STATE_COMPLETED
*/ */
if(data->conn) Curl_detach_connnection(data);
detach_connnection(data);
} }
#ifndef CURL_DISABLE_FTP #ifndef CURL_DISABLE_FTP
@ -2288,7 +2283,10 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
/* This is where we make sure that the conn pointer is reset. /* This is where we make sure that the conn pointer is reset.
We don't have to do this in every case block above where a We don't have to do this in every case block above where a
failure is detected */ failure is detected */
detach_connnection(data); Curl_detach_connnection(data);
/* remove connection from cache */
Curl_conncache_remove_conn(data, conn, TRUE);
/* disconnect properly */ /* disconnect properly */
Curl_disconnect(data, conn, dead_connection); Curl_disconnect(data, conn, dead_connection);

1
lib/multiif.h
View File

@ -33,6 +33,7 @@ void Curl_expire_done(struct Curl_easy *data, expire_id id);
void Curl_update_timer(struct Curl_multi *multi); void Curl_update_timer(struct Curl_multi *multi);
void Curl_attach_connnection(struct Curl_easy *data, void Curl_attach_connnection(struct Curl_easy *data,
struct connectdata *conn); struct connectdata *conn);
void Curl_detach_connnection(struct Curl_easy *data);
bool Curl_multiplex_wanted(const struct Curl_multi *multi); bool Curl_multiplex_wanted(const struct Curl_multi *multi);
void Curl_set_in_callback(struct Curl_easy *data, bool value); void Curl_set_in_callback(struct Curl_easy *data, bool value);
bool Curl_is_in_callback(struct Curl_easy *easy); bool Curl_is_in_callback(struct Curl_easy *easy);

69
lib/url.c
View File

@ -684,9 +684,7 @@ static void conn_reset_all_postponed_data(struct connectdata *conn)
static void conn_shutdown(struct connectdata *conn) static void conn_shutdown(struct connectdata *conn)
{ {
if(!conn) DEBUGASSERT(conn);
return;
infof(conn->data, "Closing connection %ld\n", conn->connection_id); infof(conn->data, "Closing connection %ld\n", conn->connection_id);
DEBUGASSERT(conn->data); DEBUGASSERT(conn->data);
@ -707,16 +705,11 @@ static void conn_shutdown(struct connectdata *conn)
Curl_closesocket(conn, conn->tempsock[0]); Curl_closesocket(conn, conn->tempsock[0]);
if(CURL_SOCKET_BAD != conn->tempsock[1]) if(CURL_SOCKET_BAD != conn->tempsock[1])
Curl_closesocket(conn, conn->tempsock[1]); Curl_closesocket(conn, conn->tempsock[1]);
/* unlink ourselves. this should be called last since other shutdown
procedures need a valid conn->data and this may clear it. */
Curl_conncache_remove_conn(conn->data, conn, TRUE);
} }
static void conn_free(struct connectdata *conn) static void conn_free(struct connectdata *conn)
{ {
if(!conn) DEBUGASSERT(conn);
return;
Curl_free_idnconverted_hostname(&conn->host); Curl_free_idnconverted_hostname(&conn->host);
Curl_free_idnconverted_hostname(&conn->conn_to_host); Curl_free_idnconverted_hostname(&conn->conn_to_host);
@ -783,13 +776,17 @@ static void conn_free(struct connectdata *conn)
CURLcode Curl_disconnect(struct Curl_easy *data, CURLcode Curl_disconnect(struct Curl_easy *data,
struct connectdata *conn, bool dead_connection) struct connectdata *conn, bool dead_connection)
{ {
if(!conn) /* there must be a connection to close */
return CURLE_OK; /* this is closed and fine already */ DEBUGASSERT(conn);
if(!data) { /* it must be removed from the connection cache */
DEBUGF(infof(data, "DISCONNECT without easy handle, ignoring\n")); DEBUGASSERT(!conn->bundle);
return CURLE_OK;
} /* there must be an associated transfer */
DEBUGASSERT(data);
/* the transfer must be detached from the connection */
DEBUGASSERT(!data->conn);
/* /*
* If this connection isn't marked to force-close, leave it open if there * If this connection isn't marked to force-close, leave it open if there
@ -805,16 +802,11 @@ CURLcode Curl_disconnect(struct Curl_easy *data,
conn->dns_entry = NULL; conn->dns_entry = NULL;
} }
Curl_hostcache_prune(data); /* kill old DNS cache entries */
#if !defined(CURL_DISABLE_HTTP) && defined(USE_NTLM)
/* Cleanup NTLM connection-related data */ /* Cleanup NTLM connection-related data */
Curl_http_auth_cleanup_ntlm(conn); Curl_http_auth_cleanup_ntlm(conn);
#endif
#if !defined(CURL_DISABLE_HTTP) && defined(USE_SPNEGO)
/* Cleanup NEGOTIATE connection-related data */ /* Cleanup NEGOTIATE connection-related data */
Curl_http_auth_cleanup_negotiate(conn); Curl_http_auth_cleanup_negotiate(conn);
#endif
/* the protocol specific disconnect handler and conn_shutdown need a transfer /* the protocol specific disconnect handler and conn_shutdown need a transfer
for the connection! */ for the connection! */
@ -1011,8 +1003,12 @@ static int call_extract_if_dead(struct connectdata *conn, void *param)
static void prune_dead_connections(struct Curl_easy *data) static void prune_dead_connections(struct Curl_easy *data)
{ {
struct curltime now = Curl_now(); struct curltime now = Curl_now();
timediff_t elapsed = timediff_t elapsed;
CONNCACHE_LOCK(data);
elapsed =
Curl_timediff(now, data->state.conn_cache->last_cleanup); Curl_timediff(now, data->state.conn_cache->last_cleanup);
CONNCACHE_UNLOCK(data);
if(elapsed >= 1000L) { if(elapsed >= 1000L) {
struct prunedead prune; struct prunedead prune;
@ -1020,10 +1016,17 @@ static void prune_dead_connections(struct Curl_easy *data)
prune.extracted = NULL; prune.extracted = NULL;
while(Curl_conncache_foreach(data, data->state.conn_cache, &prune, while(Curl_conncache_foreach(data, data->state.conn_cache, &prune,
call_extract_if_dead)) { call_extract_if_dead)) {
/* unlocked */
/* remove connection from cache */
Curl_conncache_remove_conn(data, prune.extracted, TRUE);
/* disconnect it */ /* disconnect it */
(void)Curl_disconnect(data, prune.extracted, /* dead_connection */TRUE); (void)Curl_disconnect(data, prune.extracted, /* dead_connection */TRUE);
} }
CONNCACHE_LOCK(data);
data->state.conn_cache->last_cleanup = now; data->state.conn_cache->last_cleanup = now;
CONNCACHE_UNLOCK(data);
} }
} }
@ -1083,7 +1086,7 @@ ConnectionExists(struct Curl_easy *data,
if(data->set.pipewait) { if(data->set.pipewait) {
infof(data, "Server doesn't support multiplex yet, wait\n"); infof(data, "Server doesn't support multiplex yet, wait\n");
*waitpipe = TRUE; *waitpipe = TRUE;
Curl_conncache_unlock(data); CONNCACHE_UNLOCK(data);
return FALSE; /* no re-use */ return FALSE; /* no re-use */
} }
@ -1407,11 +1410,12 @@ ConnectionExists(struct Curl_easy *data,
if(chosen) { if(chosen) {
/* mark it as used before releasing the lock */ /* mark it as used before releasing the lock */
chosen->data = data; /* own it! */ chosen->data = data; /* own it! */
Curl_conncache_unlock(data); Curl_attach_connnection(data, chosen);
CONNCACHE_UNLOCK(data);
*usethis = chosen; *usethis = chosen;
return TRUE; /* yes, we found one to use! */ return TRUE; /* yes, we found one to use! */
} }
Curl_conncache_unlock(data); CONNCACHE_UNLOCK(data);
if(foundPendingCandidate && data->set.pipewait) { if(foundPendingCandidate && data->set.pipewait) {
infof(data, infof(data,
@ -3529,6 +3533,7 @@ static CURLcode create_conn(struct Curl_easy *data,
if(!result) { if(!result) {
conn->bits.tcpconnect[FIRSTSOCKET] = TRUE; /* we are "connected */ conn->bits.tcpconnect[FIRSTSOCKET] = TRUE; /* we are "connected */
Curl_attach_connnection(data, conn);
result = Curl_conncache_add_conn(data->state.conn_cache, conn); result = Curl_conncache_add_conn(data->state.conn_cache, conn);
if(result) if(result)
goto out; goto out;
@ -3543,7 +3548,6 @@ static CURLcode create_conn(struct Curl_easy *data,
(void)conn->handler->done(conn, result, FALSE); (void)conn->handler->done(conn, result, FALSE);
goto out; goto out;
} }
Curl_attach_connnection(data, conn);
Curl_setup_transfer(data, -1, -1, FALSE, -1); Curl_setup_transfer(data, -1, -1, FALSE, -1);
} }
@ -3693,7 +3697,7 @@ static CURLcode create_conn(struct Curl_easy *data,
/* The bundle is full. Extract the oldest connection. */ /* The bundle is full. Extract the oldest connection. */
conn_candidate = Curl_conncache_extract_bundle(data, bundle); conn_candidate = Curl_conncache_extract_bundle(data, bundle);
Curl_conncache_unlock(data); CONNCACHE_UNLOCK(data);
if(conn_candidate) if(conn_candidate)
(void)Curl_disconnect(data, conn_candidate, (void)Curl_disconnect(data, conn_candidate,
@ -3705,7 +3709,7 @@ static CURLcode create_conn(struct Curl_easy *data,
} }
} }
else else
Curl_conncache_unlock(data); CONNCACHE_UNLOCK(data);
} }
@ -3739,6 +3743,8 @@ static CURLcode create_conn(struct Curl_easy *data,
* This is a brand new connection, so let's store it in the connection * This is a brand new connection, so let's store it in the connection
* cache of ours! * cache of ours!
*/ */
Curl_attach_connnection(data, conn);
result = Curl_conncache_add_conn(data->state.conn_cache, conn); result = Curl_conncache_add_conn(data->state.conn_cache, conn);
if(result) if(result)
goto out; goto out;
@ -3893,7 +3899,7 @@ CURLcode Curl_connect(struct Curl_easy *data,
result = create_conn(data, &conn, asyncp); result = create_conn(data, &conn, asyncp);
if(!result) { if(!result) {
if(CONN_INUSE(conn)) if(CONN_INUSE(conn) > 1)
/* multiplexed */ /* multiplexed */
*protocol_done = TRUE; *protocol_done = TRUE;
else if(!*asyncp) { else if(!*asyncp) {
@ -3910,11 +3916,10 @@ CURLcode Curl_connect(struct Curl_easy *data,
else if(result && conn) { else if(result && conn) {
/* We're not allowed to return failure with memory left allocated in the /* We're not allowed to return failure with memory left allocated in the
connectdata struct, free those here */ connectdata struct, free those here */
Curl_detach_connnection(data);
Curl_conncache_remove_conn(data, conn, TRUE);
Curl_disconnect(data, conn, TRUE); Curl_disconnect(data, conn, TRUE);
} }
else if(!result && !data->conn)
/* FILE: transfers already have the connection attached */
Curl_attach_connnection(data, conn);
return result; return result;
} }

14
tests/data/test1554
View File

@ -29,6 +29,12 @@ run 1: foobar and so on fun!
<- Mutex unlock <- Mutex unlock
-> Mutex lock -> Mutex lock
<- Mutex unlock <- Mutex unlock
-> Mutex lock
<- Mutex unlock
-> Mutex lock
<- Mutex unlock
-> Mutex lock
<- Mutex unlock
run 1: foobar and so on fun! run 1: foobar and so on fun!
-> Mutex lock -> Mutex lock
<- Mutex unlock <- Mutex unlock
@ -40,6 +46,10 @@ run 1: foobar and so on fun!
<- Mutex unlock <- Mutex unlock
-> Mutex lock -> Mutex lock
<- Mutex unlock <- Mutex unlock
-> Mutex lock
<- Mutex unlock
-> Mutex lock
<- Mutex unlock
run 1: foobar and so on fun! run 1: foobar and so on fun!
-> Mutex lock -> Mutex lock
<- Mutex unlock <- Mutex unlock
@ -51,6 +61,10 @@ run 1: foobar and so on fun!
<- Mutex unlock <- Mutex unlock
-> Mutex lock -> Mutex lock
<- Mutex unlock <- Mutex unlock
-> Mutex lock
<- Mutex unlock
-> Mutex lock
<- Mutex unlock
run 1: foobar and so on fun! run 1: foobar and so on fun!
-> Mutex lock -> Mutex lock
<- Mutex unlock <- Mutex unlock

6
tests/unit/unit1620.c
View File

@ -5,7 +5,7 @@
* | (__| |_| | _ <| |___ * | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____| * \___|\___/|_| \_\_____|
* *
* Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al. * Copyright (C) 1998 - 2020, Daniel Stenberg, <daniel@haxx.se>, et al.
* *
* This software is licensed as described in the file COPYING, which * This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms * you should have received as part of this distribution. The terms
@ -73,10 +73,6 @@ UNITTEST_START
fail_unless(rc == CURLE_OK, fail_unless(rc == CURLE_OK,
"Curl_parse_login_details() failed"); "Curl_parse_login_details() failed");
rc = Curl_disconnect(empty, empty->conn, FALSE);
fail_unless(rc == CURLE_OK,
"Curl_disconnect() with dead_connection set FALSE failed");
Curl_freeset(empty); Curl_freeset(empty);
for(i = (enum dupstring)0; i < STRING_LAST; i++) { for(i = (enum dupstring)0; i < STRING_LAST; i++) {
fail_unless(empty->set.str[i] == NULL, fail_unless(empty->set.str[i] == NULL,