From d5bb459ccf1fc5980ae4b95c05b4ecf6454a7599 Mon Sep 17 00:00:00 2001
From: Marc Aldorasi <marc@groundctl.com>
Date: Thu, 30 Jul 2020 14:16:17 -0400
Subject: [PATCH] multi_remove_handle: close unused connect-only connections

Previously any connect-only connections in a multi handle would be kept
alive until the multi handle was closed.  Since these connections cannot
be re-used, they can be marked for closure when the associated easy
handle is removed from the multi handle.

Closes #5749
---
 lib/multi.c         | 34 ++++++++++++++++++++++++++++++----
 tests/data/test1554 |  6 ++++++
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/lib/multi.c b/lib/multi.c
index 6b62ddaf767..1c3be72feed 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -689,6 +689,26 @@ static CURLcode multi_done(struct Curl_easy *data,
   return result;
 }
 
+static int close_connect_only(struct connectdata *conn, void *param)
+{
+  struct Curl_easy *data = param;
+
+  if(data->state.lastconnect != conn)
+    return 0;
+
+  if(conn->data != data)
+    return 1;
+  conn->data = NULL;
+
+  if(!conn->bits.connect_only)
+    return 1;
+
+  connclose(conn, "Removing connect-only easy handle");
+  conn->bits.connect_only = FALSE;
+
+  return 1;
+}
+
 CURLMcode curl_multi_remove_handle(struct Curl_multi *multi,
                                    struct Curl_easy *data)
 {
@@ -776,10 +796,6 @@ CURLMcode curl_multi_remove_handle(struct Curl_multi *multi,
      multi_done() as that may actually call Curl_expire that uses this */
   Curl_llist_destroy(&data->state.timeoutlist, NULL);
 
-  /* as this was using a shared connection cache we clear the pointer to that
-     since we're not part of that multi handle anymore */
-  data->state.conn_cache = NULL;
-
   /* change state without using multistate(), only to make singlesocket() do
      what we want */
   data->mstate = CURLM_STATE_COMPLETED;
@@ -789,12 +805,22 @@ CURLMcode curl_multi_remove_handle(struct Curl_multi *multi,
   /* Remove the association between the connection and the handle */
   Curl_detach_connnection(data);
 
+  if(data->state.lastconnect) {
+    /* Mark any connect-only connection for closure */
+    Curl_conncache_foreach(data, data->state.conn_cache,
+                           data, &close_connect_only);
+  }
+
 #ifdef USE_LIBPSL
   /* Remove the PSL association. */
   if(data->psl == &multi->psl)
     data->psl = NULL;
 #endif
 
+  /* as this was using a shared connection cache we clear the pointer to that
+     since we're not part of that multi handle anymore */
+  data->state.conn_cache = NULL;
+
   data->multi = NULL; /* clear the association to this multi handle */
 
   /* make sure there's no pending message in the queue sent from this easy
diff --git a/tests/data/test1554 b/tests/data/test1554
index d3926d91644..fffa6adb54f 100644
--- a/tests/data/test1554
+++ b/tests/data/test1554
@@ -50,6 +50,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
@@ -65,6 +67,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
@@ -74,6 +78,8 @@ run 1: foobar and so on fun!
 <- Mutex unlock
 -> Mutex lock
 <- Mutex unlock
+-> Mutex lock
+<- Mutex unlock
 </datacheck>
 </reply>
 
