From: Daniel Stenberg <daniel@haxx.se>
Date: Thu, 4 Apr 2024 09:14:44 +0000 (+0200)
Subject: multi: timeout handles even without connection
X-Git-Tag: curl-8_8_0~233
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a1ec035afc96cd2f8e9fb56c7d21653815875727;p=thirdparty%2Fcurl.git

multi: timeout handles even without connection

When there is a "change" in a multi handle and pending handles are moved
back to the main list to be retested if they can proceed further (for
example a previous transfer completed or a connection has a confirmed
multiplexed state), the timeout check in multi_runsingle() would not
trigger because it required an established connection.

This could make a pending tranfer go back to pending state even though
it had been "in progress" for a longer time than permitted. By removing
the requirement for an associated connection, the timeout check will be
done proper even for transfers that has not yet been assigned one.

Ref #13227
Reported-by: Rahul Krishna M
Closes #13276
---

diff --git a/lib/multi.c b/lib/multi.c
index 02f553b39c..8387c60654 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -1744,47 +1744,48 @@ static bool multi_handle_timeout(struct Curl_easy *data,
                                  CURLcode *result,
                                  bool connect_timeout)
 {
-  timediff_t timeout_ms;
-  timeout_ms = Curl_timeleft(data, now, connect_timeout);
+  timediff_t timeout_ms = Curl_timeleft(data, now, connect_timeout);
 
   if(timeout_ms < 0) {
     /* Handle timed out */
+    struct curltime since;
+    if(connect_timeout)
+      since = data->progress.t_startsingle;
+    else
+      since = data->progress.t_startop;
     if(data->mstate == MSTATE_RESOLVING)
       failf(data, "Resolving timed out after %" CURL_FORMAT_TIMEDIFF_T
-            " milliseconds",
-            Curl_timediff(*now, data->progress.t_startsingle));
+            " milliseconds", Curl_timediff(*now, since));
     else if(data->mstate == MSTATE_CONNECTING)
       failf(data, "Connection timed out after %" CURL_FORMAT_TIMEDIFF_T
-            " milliseconds",
-            Curl_timediff(*now, data->progress.t_startsingle));
+            " milliseconds", Curl_timediff(*now, since));
     else {
       struct SingleRequest *k = &data->req;
       if(k->size != -1) {
         failf(data, "Operation timed out after %" CURL_FORMAT_TIMEDIFF_T
               " milliseconds with %" CURL_FORMAT_CURL_OFF_T " out of %"
               CURL_FORMAT_CURL_OFF_T " bytes received",
-              Curl_timediff(*now, data->progress.t_startsingle),
-              k->bytecount, k->size);
+              Curl_timediff(*now, since), k->bytecount, k->size);
       }
       else {
         failf(data, "Operation timed out after %" CURL_FORMAT_TIMEDIFF_T
               " milliseconds with %" CURL_FORMAT_CURL_OFF_T
-              " bytes received",
-              Curl_timediff(*now, data->progress.t_startsingle),
-              k->bytecount);
+              " bytes received", Curl_timediff(*now, since), k->bytecount);
       }
     }
-
-    /* Force connection closed if the connection has indeed been used */
-    if(data->mstate > MSTATE_DO) {
-      streamclose(data->conn, "Disconnected with pending data");
-      *stream_error = TRUE;
-    }
     *result = CURLE_OPERATION_TIMEDOUT;
-    (void)multi_done(data, *result, TRUE);
+    if(data->conn) {
+      /* Force connection closed if the connection has indeed been used */
+      if(data->mstate > MSTATE_DO) {
+        streamclose(data->conn, "Disconnect due to timeout");
+        *stream_error = TRUE;
+      }
+      (void)multi_done(data, *result, TRUE);
+    }
+    return TRUE;
   }
 
-  return (timeout_ms < 0);
+  return FALSE;
 }
 
 /*
@@ -1942,23 +1943,12 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
         return CURLM_INTERNAL_ERROR;
     }
 
-    if(data->conn &&
-       (data->mstate >= MSTATE_CONNECT) &&
-       (data->mstate < MSTATE_COMPLETED)) {
-      /* Check for overall operation timeout here but defer handling the
-       * connection timeout to later, to allow for a connection to be set up
-       * in the window since we last checked timeout. This prevents us
-       * tearing down a completed connection in the case where we were slow
-       * to check the timeout (e.g. process descheduled during this loop).
-       * We set connect_timeout=FALSE to do this. */
-
-      /* we need to wait for the connect state as only then is the start time
-         stored, but we must not check already completed handles */
-      if(multi_handle_timeout(data, nowp, &stream_error, &result, FALSE)) {
-        /* Skip the statemachine and go directly to error handling section. */
-        goto statemachine_end;
-      }
-    }
+    /* Wait for the connect state as only then is the start time stored, but
+       we must not check already completed handles */
+    if((data->mstate >= MSTATE_CONNECT) && (data->mstate < MSTATE_COMPLETED) &&
+       multi_handle_timeout(data, nowp, &stream_error, &result, FALSE))
+      /* Skip the statemachine and go directly to error handling section. */
+      goto statemachine_end;
 
     switch(data->mstate) {
     case MSTATE_INIT:
@@ -2629,8 +2619,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
       return CURLM_INTERNAL_ERROR;
     }
 
-    if(data->conn &&
-       data->mstate >= MSTATE_CONNECT &&
+    if(data->mstate >= MSTATE_CONNECT &&
        data->mstate < MSTATE_DO &&
        rc != CURLM_CALL_MULTI_PERFORM &&
        !multi_ischanged(multi, false)) {