From c06902713998d68202c5a764de910ba8d0e8f54d Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Mon, 27 Apr 2020 00:33:21 +0200 Subject: conncache: various concept cleanups 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 --- lib/url.c | 69 ++++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 37 insertions(+), 32 deletions(-) (limited to 'lib/url.c') diff --git a/lib/url.c b/lib/url.c index 03c274438..1df3dfd3a 100644 --- a/lib/url.c +++ b/lib/url.c @@ -684,9 +684,7 @@ static void conn_reset_all_postponed_data(struct connectdata *conn) 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); @@ -707,16 +705,11 @@ static void conn_shutdown(struct connectdata *conn) 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); @@ -783,13 +776,17 @@ static void conn_free(struct connectdata *conn) 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 @@ -805,16 +802,11 @@ CURLcode Curl_disconnect(struct Curl_easy *data, 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! */ @@ -1011,8 +1003,12 @@ static int call_extract_if_dead(struct connectdata *conn, void *param) 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; @@ -1020,10 +1016,17 @@ static void prune_dead_connections(struct Curl_easy *data) 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); } } @@ -1083,7 +1086,7 @@ ConnectionExists(struct Curl_easy *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 */ } @@ -1407,11 +1410,12 @@ ConnectionExists(struct Curl_easy *data, 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, @@ -3529,6 +3533,7 @@ static CURLcode create_conn(struct Curl_easy *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; @@ -3543,7 +3548,6 @@ static CURLcode create_conn(struct Curl_easy *data, (void)conn->handler->done(conn, result, FALSE); goto out; } - Curl_attach_connnection(data, conn); 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. */ conn_candidate = Curl_conncache_extract_bundle(data, bundle); - Curl_conncache_unlock(data); + CONNCACHE_UNLOCK(data); if(conn_candidate) (void)Curl_disconnect(data, conn_candidate, @@ -3705,7 +3709,7 @@ static CURLcode create_conn(struct Curl_easy *data, } } 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 * cache of ours! */ + Curl_attach_connnection(data, conn); + result = Curl_conncache_add_conn(data->state.conn_cache, conn); if(result) goto out; @@ -3893,7 +3899,7 @@ CURLcode Curl_connect(struct Curl_easy *data, result = create_conn(data, &conn, asyncp); if(!result) { - if(CONN_INUSE(conn)) + if(CONN_INUSE(conn) > 1) /* multiplexed */ *protocol_done = TRUE; else if(!*asyncp) { @@ -3910,11 +3916,10 @@ CURLcode Curl_connect(struct Curl_easy *data, 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; } -- cgit v1.2.3