On Thu, 24 Jan 2019, Zhao Yisha via curl-library wrote:

I did some investigations and think there is discrepancy between expire time and Curl_timeleft calculation. EXPIRE_TIMEOUT/EXPIRE_CONNECTTIMEOUT timeouts are smaller than (t_startsingle/t_startop + configured timeout). My proposed fix is to set corresponding expire time after t_startsingle and t_startop are set.

Have you seen a similar problem?

I have not seen this happen, but I do appreciate your analysis and I understand what you're saying happens in your case: The EXPIRE_* timeout triggers but Curl_timeleft() doesn't think there's a timeout yet so it doesn't cancel the transfer, but since the expire timeout already triggered there is no more timer in the timetree.

Attach is my attempt at fixing this based on your suggestion and description. Is this what you had in mind?

I also created PR #3501 where this patch is being run through the CI tests: https://github.com/curl/curl/pull/3501

--

 / daniel.haxx.se
From 6aa964cefb13633cb6b0fd0218f1932933dd0261 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <dan...@haxx.se>
Date: Sun, 27 Jan 2019 23:45:42 +0100
Subject: [PATCH] multi: set the EXPIRE_*TIMEOUT timers at TIMER_STARTSINGLE
 time

To make sure Curl_timeleft() also thinks the timeout has been reached
when one of the EXPIRE_*TIMEOUTs expires.

Reported-by: Zhao Yisha
---
 lib/multi.c    | 6 ++++++
 lib/transfer.c | 6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/multi.c b/lib/multi.c
index 75ece15ca..130226f56 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -1452,10 +1452,16 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
       break;
 
     case CURLM_STATE_CONNECT:
       /* Connect. We want to get a connection identifier filled in. */
       Curl_pgrsTime(data, TIMER_STARTSINGLE);
+      if(data->set.timeout)
+        Curl_expire(data, data->set.timeout, EXPIRE_TIMEOUT);
+
+      if(data->set.connecttimeout)
+        Curl_expire(data, data->set.connecttimeout, EXPIRE_CONNECTTIMEOUT);
+
       result = Curl_connect(data, &async, &protocol_connect);
       if(CURLE_NO_CONNECTION_AVAILABLE == result) {
         /* There was no connection available. We will go to the pending
            state and wait for an available connection. */
         multistate(data, CURLM_STATE_CONNECT_PEND);
diff --git a/lib/transfer.c b/lib/transfer.c
index e687e6018..3a18c7bdd 100644
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -1556,16 +1556,10 @@ CURLcode Curl_pretransfer(struct Curl_easy *data)
 
     Curl_initinfo(data); /* reset session-specific information "variables" */
     Curl_pgrsResetTransferSizes(data);
     Curl_pgrsStartNow(data);
 
-    if(data->set.timeout)
-      Curl_expire(data, data->set.timeout, EXPIRE_TIMEOUT);
-
-    if(data->set.connecttimeout)
-      Curl_expire(data, data->set.connecttimeout, EXPIRE_CONNECTTIMEOUT);
-
     /* In case the handle is re-used and an authentication method was picked
        in the session we need to make sure we only use the one(s) we now
        consider to be fine */
     data->state.authhost.picked &= data->state.authhost.want;
     data->state.authproxy.picked &= data->state.authproxy.want;
-- 
2.20.1

-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Reply via email to