From 960753287368e4dee768dd480de90dc07c62a380 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Sun, 5 Jan 2020 10:34:19 +0100 Subject: ConnectionExists: respect the max_concurrent_streams limits A regression made the code use 'multiplexed' as a boolean instead of the counter it is intended to be. This made curl try to "over-populate" connections with new streams. This regression came with 41fcdf71a1, shipped in curl 7.65.0. Also, respect the CURLMOPT_MAX_CONCURRENT_STREAMS value in the same check. Reported-by: Kunal Ekawde Fixes #4779 Closes #4784 --- lib/http2.c | 4 ++-- lib/multi.c | 11 ++++++----- lib/multihandle.h | 2 +- lib/multiif.h | 8 +++----- lib/url.c | 13 ++++++++++--- 5 files changed, 22 insertions(+), 16 deletions(-) diff --git a/lib/http2.c b/lib/http2.c index 65f3513ee..690a537bf 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2019, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2020, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -1158,7 +1158,7 @@ static void populate_settings(struct connectdata *conn, nghttp2_settings_entry *iv = httpc->local_settings; iv[0].settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS; - iv[0].value = (uint32_t)Curl_multi_max_concurrent_streams(conn->data->multi); + iv[0].value = Curl_multi_max_concurrent_streams(conn->data->multi); iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE; iv[1].value = HTTP2_HUGE_WINDOW_SIZE; diff --git a/lib/multi.c b/lib/multi.c index d0f9b8340..1b79d42a4 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -369,6 +369,7 @@ struct Curl_multi *Curl_multi_handle(int hashsize, /* socket hash */ /* -1 means it not set by user, use the default value */ multi->maxconnects = -1; + multi->max_concurrent_streams = 100; #ifdef ENABLE_WAKEUP if(Curl_socketpair(AF_UNIX, SOCK_STREAM, 0, multi->wakeup_pair) < 0) { @@ -2900,8 +2901,8 @@ CURLMcode curl_multi_setopt(struct Curl_multi *multi, if(streams < 1) streams = 100; multi->max_concurrent_streams = - (streams > (long)INITIAL_MAX_CONCURRENT_STREAMS)? - (long)INITIAL_MAX_CONCURRENT_STREAMS : streams; + (streams > (long)INITIAL_MAX_CONCURRENT_STREAMS)? + INITIAL_MAX_CONCURRENT_STREAMS : (unsigned int)streams; } break; default: @@ -3343,8 +3344,8 @@ void Curl_multi_dump(struct Curl_multi *multi) } #endif -size_t Curl_multi_max_concurrent_streams(struct Curl_multi *multi) +unsigned int Curl_multi_max_concurrent_streams(struct Curl_multi *multi) { - return multi ? ((size_t)multi->max_concurrent_streams ? - (size_t)multi->max_concurrent_streams : 100) : 0; + DEBUGASSERT(multi); + return multi->max_concurrent_streams; } diff --git a/lib/multihandle.h b/lib/multihandle.h index 0bf09e6bb..b0cd0b821 100644 --- a/lib/multihandle.h +++ b/lib/multihandle.h @@ -142,7 +142,7 @@ struct Curl_multi { struct curltime timer_lastcall; /* the fixed time for the timeout for the previous callback */ bool in_callback; /* true while executing a callback */ - long max_concurrent_streams; /* max concurrent streams client to support */ + unsigned int max_concurrent_streams; #ifdef ENABLE_WAKEUP curl_socket_t wakeup_pair[2]; /* socketpair() used for wakeup diff --git a/lib/multiif.h b/lib/multiif.h index 75025232c..bde755ee0 100644 --- a/lib/multiif.h +++ b/lib/multiif.h @@ -7,7 +7,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2019, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2020, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -90,9 +90,7 @@ CURLMcode Curl_multi_add_perform(struct Curl_multi *multi, struct connectdata *conn); -/* Return the value of the CURLMOPT_MAX_CONCURRENT_STREAMS option - * If not specified or 0, default would be 100 - */ -size_t Curl_multi_max_concurrent_streams(struct Curl_multi *multi); +/* Return the value of the CURLMOPT_MAX_CONCURRENT_STREAMS option */ +unsigned int Curl_multi_max_concurrent_streams(struct Curl_multi *multi); #endif /* HEADER_CURL_MULTIIF_H */ diff --git a/lib/url.c b/lib/url.c index 4797b5182..b001e87f8 100644 --- a/lib/url.c +++ b/lib/url.c @@ -1073,7 +1073,7 @@ ConnectionExists(struct Curl_easy *data, curr = bundle->conn_list.head; while(curr) { bool match = FALSE; - size_t multiplexed; + size_t multiplexed = 0; /* * Note that if we use a HTTP proxy in normal mode (no tunneling), we @@ -1086,8 +1086,8 @@ ConnectionExists(struct Curl_easy *data, /* connect-only or to-be-closed connections will not be reused */ continue; - multiplexed = CONN_INUSE(check) && - (bundle->multiuse == BUNDLE_MULTIPLEX); + if(bundle->multiuse == BUNDLE_MULTIPLEX) + multiplexed = CONN_INUSE(check); if(canmultiplex) { ; @@ -1347,6 +1347,13 @@ ConnectionExists(struct Curl_easy *data, multiplexed); continue; } + else if(multiplexed >= + Curl_multi_max_concurrent_streams(needle->data->multi)) { + infof(data, "client side MAX_CONCURRENT_STREAMS reached" + ", skip (%zu)\n", + multiplexed); + continue; + } } #endif /* When not multiplexed, we have a match here! */ -- cgit v1.2.3