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

diff --git a/lib/http2.c b/lib/http2.c
index 65f3513ee51d5e371dc80f670df9881ad0af6ff9..690a537bf7e5a4de06859fe8e23340a573d73f7f 100644 (file)
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -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
  * 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 d0f9b83400551608cec6b1b09c294277a4ffa864..1b79d42a4379b5025e56f8417fbc08df0f974c14 100644 (file)
--- 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 0bf09e6bb55c478d5b4ee8364e67a9537ae8935b..b0cd0b821bb999e83aa037b0973b5d197c3de9f7 100644 (file)
--- 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 75025232c42998228fa1d7aaebf5387f667cc48d..bde755ee0f7617132107dada5bf5616c91f71f5f 100644 (file)
--- a/lib/multiif.h
+++ b/lib/multiif.h
@@ -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
  * 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 4797b518219d1f6ab499d5ba3f751e0df65dd6bb..b001e87f8c12215f196a79fcbbff0d1d49688033 100644 (file)
--- 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! */