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
conn->bundle = NULL;
}
-static CURLcode bundle_create(struct Curl_easy *data,
- struct connectbundle **cb_ptr)
+static CURLcode bundle_create(struct connectbundle **bundlep)
{
- (void)data;
- DEBUGASSERT(*cb_ptr == NULL);
- *cb_ptr = malloc(sizeof(struct connectbundle));
- if(!*cb_ptr)
+ DEBUGASSERT(*bundlep == NULL);
+ *bundlep = malloc(sizeof(struct connectbundle));
+ if(!*bundlep)
return CURLE_OUT_OF_MEMORY;
- (*cb_ptr)->num_connections = 0;
- (*cb_ptr)->multiuse = BUNDLE_UNKNOWN;
+ (*bundlep)->num_connections = 0;
+ (*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;
}
-static void bundle_destroy(struct connectbundle *cb_ptr)
+static void bundle_destroy(struct connectbundle *bundle)
{
- if(!cb_ptr)
+ if(!bundle)
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 */
-static void bundle_add_conn(struct connectbundle *cb_ptr,
+static void bundle_add_conn(struct connectbundle *bundle,
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 = cb_ptr;
- cb_ptr->num_connections++;
+ conn->bundle = bundle;
+ bundle->num_connections++;
}
/* 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 curl_llist_element *curr;
- curr = cb_ptr->conn_list.head;
+ curr = bundle->conn_list.head;
while(curr) {
if(curr->ptr == conn) {
- Curl_llist_remove(&cb_ptr->conn_list, curr, NULL);
- cb_ptr->num_connections--;
+ Curl_llist_remove(&bundle->conn_list, curr, NULL);
+ bundle->num_connections--;
conn->bundle = NULL;
return 1; /* we removed a handle */
}
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.
Locks/unlocks the cache itself!
*/
size_t Curl_conncache_size(struct Curl_easy *data)
{
size_t num;
- CONN_LOCK(data);
+ CONNCACHE_LOCK(data);
num = data->state.conn_cache->num_conn;
- CONN_UNLOCK(data);
+ CONNCACHE_UNLOCK(data);
return num;
}
const char **hostp)
{
struct connectbundle *bundle = NULL;
- CONN_LOCK(conn->data);
+ CONNCACHE_LOCK(conn->data);
if(connc) {
char key[HASHKEY_SIZE];
hashkey(conn, key, sizeof(key), hostp);
struct connectdata *conn)
{
CURLcode result = CURLE_OK;
- struct connectbundle *bundle;
- struct connectbundle *new_bundle = NULL;
+ struct connectbundle *bundle = NULL;
struct Curl_easy *data = conn->data;
/* *find_bundle() locks the connection cache */
int rc;
char key[HASHKEY_SIZE];
- result = bundle_create(data, &new_bundle);
+ result = bundle_create(&bundle);
if(result) {
goto unlock;
}
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) {
- bundle_destroy(new_bundle);
+ bundle_destroy(bundle);
result = CURLE_OUT_OF_MEMORY;
goto unlock;
}
- bundle = new_bundle;
}
bundle_add_conn(bundle, conn);
conn->connection_id, connc->num_conn));
unlock:
- CONN_UNLOCK(data);
+ CONNCACHE_UNLOCK(data);
return result;
}
/*
- * Removes the connectdata object from the connection cache *and* clears the
- * ->data pointer association. Pass TRUE/FALSE in the 'lock' argument
- * depending on if the parent function already holds the lock or not.
+ * Removes the connectdata object from the connection cache, but does *not*
+ * clear the conn->data association. The transfer still owns this connection.
+ *
+ * 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,
struct connectdata *conn, bool lock)
due to a failed connection attempt, before being added to a bundle */
if(bundle) {
if(lock) {
- CONN_LOCK(data);
+ CONNCACHE_LOCK(data);
}
bundle_remove_conn(bundle, conn);
if(bundle->num_connections == 0)
DEBUGF(infof(data, "The cache now contains %zu members\n",
connc->num_conn));
}
- conn->data = NULL; /* clear the association */
if(lock) {
- CONN_UNLOCK(data);
+ CONNCACHE_UNLOCK(data);
}
}
}
if(!connc)
return FALSE;
- CONN_LOCK(data);
+ CONNCACHE_LOCK(data);
Curl_hash_start_iterate(&connc->hash, &iter);
he = Curl_hash_next_element(&iter);
curr = curr->next;
if(1 == func(conn, param)) {
- CONN_UNLOCK(data);
+ CONNCACHE_UNLOCK(data);
return TRUE;
}
}
}
- CONN_UNLOCK(data);
+ CONNCACHE_UNLOCK(data);
return FALSE;
}
now = Curl_now();
- CONN_LOCK(data);
+ CONNCACHE_LOCK(data);
Curl_hash_start_iterate(&connc->hash, &iter);
he = Curl_hash_next_element(&iter);
connc->num_conn));
conn_candidate->data = data; /* associate! */
}
- CONN_UNLOCK(data);
+ CONNCACHE_UNLOCK(data);
return conn_candidate;
}
sigpipe_ignore(conn->data, &pipe_st);
/* This will remove the connection from the cache */
connclose(conn, "kill all");
+ Curl_conncache_remove_conn(conn->data, conn, TRUE);
(void)Curl_disconnect(connc->closure_handle, conn, FALSE);
sigpipe_restore(&pipe_st);
#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) { \
+#define CONNCACHE_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) { \
+#define CONNCACHE_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) \
+#define CONNCACHE_LOCK(x) if((x)->share) \
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)
#endif
struct connectbundle *Curl_conncache_find_bundle(struct connectdata *conn,
struct conncache *connc,
const char **hostp);
-void Curl_conncache_unlock(struct Curl_easy *data);
/* returns number of connections currently held in the connection cache */
size_t Curl_conncache_size(struct Curl_easy *data);
result = Curl_setup_conn(conn, protocol_done);
- if(result)
- /* We're not allowed to return failure with memory left allocated
- in the connectdata struct, free those here */
- Curl_disconnect(conn->data, conn, TRUE); /* close the connection */
-
+ if(result) {
+ struct Curl_easy *data = conn->data;
+ DEBUGASSERT(data);
+ Curl_detach_connnection(data);
+ Curl_conncache_remove_conn(data, conn, TRUE);
+ Curl_disconnect(data, conn, TRUE);
+ }
return result;
}
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
- * 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
* you should have received as part of this distribution. The terms
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 */
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
- * 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
* you should have received as part of this distribution. The terms
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 */
static CURLMcode multi_timeout(struct Curl_multi *multi,
long *timeout_ms);
static void process_pending_handles(struct Curl_multi *multi);
-static void detach_connnection(struct Curl_easy *data);
#ifdef DEBUGBUILD
static const char * const statename[]={
/* Important: reset the conn pointer so that we don't point to memory
that could be freed anytime */
- detach_connnection(data);
+ Curl_detach_connnection(data);
Curl_expire_clear(data); /* stop all timers */
}
easy handle is added */
memset(&multi->timer_lastcall, 0, sizeof(multi->timer_lastcall));
+ CONNCACHE_LOCK(data);
/* 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
closure handle always has the same timeouts as the most recently added
data->set.server_response_timeout;
data->state.conn_cache->closure_handle->set.no_signal =
data->set.no_signal;
+ CONNCACHE_UNLOCK(data);
Curl_update_timer(multi);
return CURLM_OK;
process_pending_handles(data->multi); /* connection / multiplex */
- CONN_LOCK(data);
- detach_connnection(data);
+ CONNCACHE_LOCK(data);
+ Curl_detach_connnection(data);
if(CONN_INUSE(conn)) {
/* Stop if still used. */
/* conn->data must not remain pointing to this transfer since it is going
away! Find another to own it! */
conn->data = conn->easyq.head->ptr;
- CONN_UNLOCK(data);
+ CONNCACHE_UNLOCK(data);
DEBUGF(infof(data, "Connection still in use %zu, "
"no more multi_done now!\n",
conn->easyq.size));
|| (premature && !(conn->handler->flags & PROTOPT_STREAM))) {
CURLcode res2;
connclose(conn, "disconnecting");
- CONN_UNLOCK(data);
+ Curl_conncache_remove_conn(data, conn, FALSE);
+ CONNCACHE_UNLOCK(data);
res2 = Curl_disconnect(data, conn, premature);
/* If we had an error already, make sure we return that one. But
conn->bits.conn_to_host ? conn->conn_to_host.dispname :
conn->host.dispname);
/* the connection is no longer in use by this transfer */
- CONN_UNLOCK(data);
+ CONNCACHE_UNLOCK(data);
if(Curl_conncache_return_conn(data, conn)) {
/* remember the most recently used connection */
data->state.lastconnect = conn;
vanish with this handle */
/* Remove the association between the connection and the handle */
- if(data->conn)
- detach_connnection(data);
+ Curl_detach_connnection(data);
#ifdef USE_LIBPSL
/* Remove the PSL association. */
return (multi && (multi->multiplexing));
}
-/* This is the only function that should clear data->conn. This will
- occasionally be called with the pointer already cleared. */
-static void detach_connnection(struct Curl_easy *data)
+/*
+ * Curl_detach_connnection() removes the given transfer from the connection.
+ *
+ * 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;
if(conn)
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,
struct connectdata *conn)
{
bool stream_error = FALSE;
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)) {
DEBUGF(infof(data, "multi changed, check CONNECT_PEND queue!\n"));
process_pending_handles(multi); /* multiplexed */
* access free'd data, if the connection is free'd and the handle
* removed before we perform the processing in CURLM_STATE_COMPLETED
*/
- if(data->conn)
- detach_connnection(data);
+ Curl_detach_connnection(data);
}
#ifndef CURL_DISABLE_FTP
/* 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
failure is detected */
- detach_connnection(data);
+ Curl_detach_connnection(data);
+
+ /* remove connection from cache */
+ Curl_conncache_remove_conn(data, conn, TRUE);
/* disconnect properly */
Curl_disconnect(data, conn, dead_connection);
void Curl_update_timer(struct Curl_multi *multi);
void Curl_attach_connnection(struct Curl_easy *data,
struct connectdata *conn);
+void Curl_detach_connnection(struct Curl_easy *data);
bool Curl_multiplex_wanted(const struct Curl_multi *multi);
void Curl_set_in_callback(struct Curl_easy *data, bool value);
bool Curl_is_in_callback(struct Curl_easy *easy);
static void conn_shutdown(struct connectdata *conn)
{
- if(!conn)
- return;
-
+ DEBUGASSERT(conn);
infof(conn->data, "Closing connection %ld\n", conn->connection_id);
DEBUGASSERT(conn->data);
Curl_closesocket(conn, conn->tempsock[0]);
if(CURL_SOCKET_BAD != 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)
{
- if(!conn)
- return;
+ DEBUGASSERT(conn);
Curl_free_idnconverted_hostname(&conn->host);
Curl_free_idnconverted_hostname(&conn->conn_to_host);
CURLcode Curl_disconnect(struct Curl_easy *data,
struct connectdata *conn, bool dead_connection)
{
- if(!conn)
- return CURLE_OK; /* this is closed and fine already */
+ /* there must be a connection to close */
+ DEBUGASSERT(conn);
- if(!data) {
- DEBUGF(infof(data, "DISCONNECT without easy handle, ignoring\n"));
- return CURLE_OK;
- }
+ /* it must be removed from the connection cache */
+ DEBUGASSERT(!conn->bundle);
+
+ /* 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
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 */
Curl_http_auth_cleanup_ntlm(conn);
-#endif
-#if !defined(CURL_DISABLE_HTTP) && defined(USE_SPNEGO)
+
/* Cleanup NEGOTIATE connection-related data */
Curl_http_auth_cleanup_negotiate(conn);
-#endif
/* the protocol specific disconnect handler and conn_shutdown need a transfer
for the connection! */
static void prune_dead_connections(struct Curl_easy *data)
{
struct curltime now = Curl_now();
- timediff_t elapsed =
+ timediff_t elapsed;
+
+ CONNCACHE_LOCK(data);
+ elapsed =
Curl_timediff(now, data->state.conn_cache->last_cleanup);
+ CONNCACHE_UNLOCK(data);
if(elapsed >= 1000L) {
struct prunedead prune;
prune.extracted = NULL;
while(Curl_conncache_foreach(data, data->state.conn_cache, &prune,
call_extract_if_dead)) {
+ /* unlocked */
+
+ /* remove connection from cache */
+ Curl_conncache_remove_conn(data, prune.extracted, TRUE);
+
/* disconnect it */
(void)Curl_disconnect(data, prune.extracted, /* dead_connection */TRUE);
}
+ CONNCACHE_LOCK(data);
data->state.conn_cache->last_cleanup = now;
+ CONNCACHE_UNLOCK(data);
}
}
if(data->set.pipewait) {
infof(data, "Server doesn't support multiplex yet, wait\n");
*waitpipe = TRUE;
- Curl_conncache_unlock(data);
+ CONNCACHE_UNLOCK(data);
return FALSE; /* no re-use */
}
if(chosen) {
/* mark it as used before releasing the lock */
chosen->data = data; /* own it! */
- Curl_conncache_unlock(data);
+ Curl_attach_connnection(data, chosen);
+ CONNCACHE_UNLOCK(data);
*usethis = chosen;
return TRUE; /* yes, we found one to use! */
}
- Curl_conncache_unlock(data);
+ CONNCACHE_UNLOCK(data);
if(foundPendingCandidate && data->set.pipewait) {
infof(data,
if(!result) {
conn->bits.tcpconnect[FIRSTSOCKET] = TRUE; /* we are "connected */
+ Curl_attach_connnection(data, conn);
result = Curl_conncache_add_conn(data->state.conn_cache, conn);
if(result)
goto out;
(void)conn->handler->done(conn, result, FALSE);
goto out;
}
- Curl_attach_connnection(data, conn);
Curl_setup_transfer(data, -1, -1, FALSE, -1);
}
/* The bundle is full. Extract the oldest connection. */
conn_candidate = Curl_conncache_extract_bundle(data, bundle);
- Curl_conncache_unlock(data);
+ CONNCACHE_UNLOCK(data);
if(conn_candidate)
(void)Curl_disconnect(data, conn_candidate,
}
}
else
- Curl_conncache_unlock(data);
+ CONNCACHE_UNLOCK(data);
}
* This is a brand new connection, so let's store it in the connection
* cache of ours!
*/
+ Curl_attach_connnection(data, conn);
+
result = Curl_conncache_add_conn(data->state.conn_cache, conn);
if(result)
goto out;
result = create_conn(data, &conn, asyncp);
if(!result) {
- if(CONN_INUSE(conn))
+ if(CONN_INUSE(conn) > 1)
/* multiplexed */
*protocol_done = TRUE;
else if(!*asyncp) {
else if(result && conn) {
/* We're not allowed to return failure with memory left allocated in the
connectdata struct, free those here */
+ Curl_detach_connnection(data);
+ Curl_conncache_remove_conn(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;
}
<- Mutex unlock
-> Mutex lock
<- 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 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 unlock
-> Mutex lock
<- Mutex unlock
+-> Mutex lock
+<- Mutex unlock
+-> Mutex lock
+<- Mutex unlock
run 1: foobar and so on fun!
-> Mutex lock
<- Mutex unlock
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
- * 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
* you should have received as part of this distribution. The terms
fail_unless(rc == CURLE_OK,
"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);
for(i = (enum dupstring)0; i < STRING_LAST; i++) {
fail_unless(empty->set.str[i] == NULL,