diff options
| -rw-r--r-- | lib/conncache.c | 30 | ||||
| -rw-r--r-- | lib/conncache.h | 24 | ||||
| -rw-r--r-- | lib/http.c | 2 | ||||
| -rw-r--r-- | lib/http2.c | 5 | ||||
| -rw-r--r-- | lib/http2.h | 2 | ||||
| -rwxr-xr-x | lib/multi.c | 20 | ||||
| -rw-r--r-- | lib/url.c | 21 | ||||
| -rw-r--r-- | tests/data/test1554 | 6 |
8 files changed, 58 insertions, 52 deletions
diff --git a/lib/conncache.c b/lib/conncache.c index 57d6061fd..a23244cf6 100644 --- a/lib/conncache.c +++ b/lib/conncache.c @@ -40,27 +40,6 @@ #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 - #define HASHKEY_SIZE 128 static void conn_llist_dtor(void *user, void *element) @@ -122,6 +101,7 @@ static int bundle_remove_conn(struct connectbundle *cb_ptr, } curr = curr->next; } + DEBUGASSERT(0); return 0; } @@ -428,17 +408,15 @@ conncache_find_first_connection(struct conncache *connc) * * Return TRUE if stored, FALSE if closed. */ -bool Curl_conncache_return_conn(struct connectdata *conn) +bool Curl_conncache_return_conn(struct Curl_easy *data, + 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; - conn->data = NULL; /* no owner anymore */ conn->lastused = Curl_now(); /* it was used up until now */ if(maxconnects > 0 && Curl_conncache_size(data) > maxconnects) { @@ -541,7 +519,7 @@ Curl_conncache_extract_oldest(struct Curl_easy *data) while(curr) { conn = curr->ptr; - if(!CONN_INUSE(conn) && !conn->data) { + if(!CONN_INUSE(conn) && !conn->data && !conn->bits.close) { /* Set higher score for the age passed since the connection was used */ score = Curl_timediff(now, conn->lastused); diff --git a/lib/conncache.h b/lib/conncache.h index 58f902409..5fe80b4c8 100644 --- a/lib/conncache.h +++ b/lib/conncache.h @@ -42,6 +42,27 @@ struct conncache { #define BUNDLE_UNKNOWN 0 /* initial value */ #define BUNDLE_MULTIPLEX 2 +#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 + struct connectbundle { int multiuse; /* supports multi-use */ size_t num_connections; /* Number of connections in the bundle */ @@ -61,7 +82,8 @@ void Curl_conncache_unlock(struct Curl_easy *data); 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); +bool Curl_conncache_return_conn(struct Curl_easy *data, + struct connectdata *conn); CURLcode Curl_conncache_add_conn(struct conncache *connc, struct connectdata *conn) WARN_UNUSED_RESULT; void Curl_conncache_remove_conn(struct Curl_easy *data, diff --git a/lib/http.c b/lib/http.c index 885704560..35d2e9fb0 100644 --- a/lib/http.c +++ b/lib/http.c @@ -1617,7 +1617,7 @@ CURLcode Curl_http_done(struct connectdata *conn, Curl_add_buffer_free(&http->send_buffer); } - Curl_http2_done(conn, premature); + Curl_http2_done(data, premature); Curl_quic_done(data, premature); Curl_mime_cleanpart(&http->form); diff --git a/lib/http2.c b/lib/http2.c index b741aed48..65f3513ee 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -1169,11 +1169,10 @@ static void populate_settings(struct connectdata *conn, httpc->local_settings_num = 3; } -void Curl_http2_done(struct connectdata *conn, bool premature) +void Curl_http2_done(struct Curl_easy *data, bool premature) { - struct Curl_easy *data = conn->data; struct HTTP *http = data->req.protop; - struct http_conn *httpc = &conn->proto.httpc; + struct http_conn *httpc = &data->conn->proto.httpc; /* there might be allocated resources done before this got the 'h2' pointer setup */ diff --git a/lib/http2.h b/lib/http2.h index 93058ccb3..12d36eef9 100644 --- a/lib/http2.h +++ b/lib/http2.h @@ -50,7 +50,7 @@ CURLcode Curl_http2_switched(struct connectdata *conn, /* called from http_setup_conn */ void Curl_http2_setup_conn(struct connectdata *conn); void Curl_http2_setup_req(struct Curl_easy *data); -void Curl_http2_done(struct connectdata *conn, bool premature); +void Curl_http2_done(struct Curl_easy *data, bool premature); CURLcode Curl_http2_done_sending(struct connectdata *conn); CURLcode Curl_http2_add_child(struct Curl_easy *parent, struct Curl_easy *child, diff --git a/lib/multi.c b/lib/multi.c index f30e41a65..1fa6b092b 100755 --- a/lib/multi.c +++ b/lib/multi.c @@ -547,6 +547,8 @@ static CURLcode multi_done(struct Curl_easy *data, /* Stop if multi_done() has already been called */ return CURLE_OK; + conn->data = data; /* ensure the connection uses this transfer now */ + /* Stop the resolver and free its own resources (but not dns_entry yet). */ Curl_resolver_kill(conn); @@ -583,15 +585,17 @@ static CURLcode multi_done(struct Curl_easy *data, process_pending_handles(data->multi); /* connection / multiplex */ + CONN_LOCK(data); detach_connnection(data); if(CONN_INUSE(conn)) { /* Stop if still used. */ + CONN_UNLOCK(data); DEBUGF(infof(data, "Connection still in use %zu, " "no more multi_done now!\n", conn->easyq.size)); return CURLE_OK; } - + conn->data = NULL; /* the connection now has no owner */ data->state.done = TRUE; /* called just now! */ if(conn->dns_entry) { @@ -634,7 +638,10 @@ static CURLcode multi_done(struct Curl_easy *data, #endif ) || conn->bits.close || (premature && !(conn->handler->flags & PROTOPT_STREAM))) { - CURLcode res2 = Curl_disconnect(data, conn, premature); + CURLcode res2; + conn->bits.close = TRUE; /* forcibly prevents reuse */ + CONN_UNLOCK(data); + res2 = Curl_disconnect(data, conn, premature); /* If we had an error already, make sure we return that one. But if we got a new error, return that. */ @@ -651,9 +658,9 @@ static CURLcode multi_done(struct Curl_easy *data, 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 by this transfer */ - if(Curl_conncache_return_conn(conn)) { + CONN_UNLOCK(data); + if(Curl_conncache_return_conn(data, conn)) { /* remember the most recently used connection */ data->state.lastconnect = conn; infof(data, "%s\n", buffer); @@ -760,10 +767,8 @@ CURLMcode curl_multi_remove_handle(struct Curl_multi *multi, vanish with this handle */ /* Remove the association between the connection and the handle */ - if(data->conn) { - data->conn->data = NULL; + if(data->conn) detach_connnection(data); - } #ifdef USE_LIBPSL /* Remove the PSL association. */ @@ -1349,6 +1354,7 @@ static CURLcode multi_do(struct Curl_easy *data, bool *done) DEBUGASSERT(conn); DEBUGASSERT(conn->handler); + DEBUGASSERT(conn->data == data); if(conn->handler->do_it) { /* generic protocol-specific function pointer set in curl_connect() */ @@ -1082,16 +1082,15 @@ ConnectionExists(struct Curl_easy *data, check = curr->ptr; curr = curr->next; - if(check->bits.connect_only) - /* connect-only connections will not be reused */ + if(check->bits.connect_only || check->bits.close) + /* connect-only or to-be-closed connections will not be reused */ continue; multiplexed = CONN_INUSE(check) && (bundle->multiuse == BUNDLE_MULTIPLEX); if(canmultiplex) { - if(check->bits.protoconnstart && check->bits.close) - continue; + ; } else { if(multiplexed) { @@ -1111,12 +1110,9 @@ ConnectionExists(struct Curl_easy *data, } } - if((check->sock[FIRSTSOCKET] == CURL_SOCKET_BAD) || - check->bits.close) { - if(!check->bits.close) - foundPendingCandidate = TRUE; - /* Don't pick a connection that hasn't connected yet or that is going - to get closed. */ + if(check->sock[FIRSTSOCKET] == CURL_SOCKET_BAD) { + foundPendingCandidate = TRUE; + /* Don't pick a connection that hasn't connected yet */ infof(data, "Connection #%ld isn't open enough, can't reuse\n", check->connection_id); continue; @@ -1194,8 +1190,7 @@ ConnectionExists(struct Curl_easy *data, already in use so we skip it */ continue; - if(CONN_INUSE(check) && check->data && - (check->data->multi != needle->data->multi)) + if(check->data && (check->data->multi != needle->data->multi)) /* this could be subject for multiplex use, but only if they belong to * the same multi handle */ continue; @@ -1643,6 +1638,7 @@ static struct connectdata *allocate_conn(struct Curl_easy *data) it may live on without (this specific) Curl_easy */ conn->fclosesocket = data->set.fclosesocket; conn->closesocket_client = data->set.closesocket_client; + conn->lastused = Curl_now(); /* used now */ return conn; error: @@ -3612,7 +3608,6 @@ static CURLcode create_conn(struct Curl_easy *data, reuse = FALSE; infof(data, "We can reuse, but we want a new connection anyway\n"); - Curl_conncache_return_conn(conn_temp); } } } diff --git a/tests/data/test1554 b/tests/data/test1554 index be48e02eb..06f189724 100644 --- a/tests/data/test1554 +++ b/tests/data/test1554 @@ -38,6 +38,8 @@ run 1: foobar and so on fun! <- Mutex unlock -> Mutex lock <- Mutex unlock +-> Mutex lock +<- Mutex unlock run 1: foobar and so on fun! -> Mutex lock <- Mutex unlock @@ -47,6 +49,8 @@ run 1: foobar and so on fun! <- Mutex unlock -> Mutex lock <- Mutex unlock +-> Mutex lock +<- Mutex unlock run 1: foobar and so on fun! -> Mutex lock <- Mutex unlock @@ -54,6 +58,8 @@ run 1: foobar and so on fun! <- Mutex unlock -> Mutex lock <- Mutex unlock +-> Mutex lock +<- Mutex unlock </datacheck> </reply> |