timers: fix timer regression involving redirects / reconnects

In commit 0b3750b5c23c25f (released in 7.36.0) we fixed a timeout issue
but instead broke the timings.

To fix this, I introduce a new timestamp to use for the timeouts and
restored the previous timestamp and timestamp position so that the old
timer functionality is restored.

In addition to that, that change also broke connection timeouts for when
more than one connect was used (as it would then count the total time
from the first connect and not for the most recent one). Now
Curl_timeleft() has been modified so that it checks against different
start times depending on which timeout it checks.

Test 1303 is updated accordingly.

Bug: http://curl.haxx.se/mail/lib-2014-05/0147.html
Reported-by: Ryan Braud

diff --git a/lib/connect.c b/lib/connect.c
index b35c36c002dac7589cf5ed4505eb641f5e7be4a4..ca6e3466c275f1eeeb24412e2f553ff4edbdd703 100644 (file)
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -224,7 +224,12 @@ long Curl_timeleft(struct SessionHandle *data,
   }
 
   /* subtract elapsed time */
-  timeout_ms -= Curl_tvdiff(*nowp, data->progress.t_startsingle);
+  if(duringconnect)
+    /* since this most recent connect started */
+    timeout_ms -= Curl_tvdiff(*nowp, data->progress.t_startsingle);
+  else
+    /* since the entire operation started */
+    timeout_ms -= Curl_tvdiff(*nowp, data->progress.t_startop);
   if(!timeout_ms)
     /* avoid returning 0 as that means no timeout! */
     return -1;

diff --git a/lib/multi.c b/lib/multi.c
index c52db96258e3993e9f654e3c691a7292bdae78b4..72fde7439e15f92a7fb038acc5abdd42e7a9ff37 100644 (file)
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -1026,7 +1026,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
       if(CURLE_OK == data->result) {
         /* after init, go CONNECT */
         multistate(data, CURLM_STATE_CONNECT);
-        Curl_pgrsTime(data, TIMER_STARTSINGLE);
+        Curl_pgrsTime(data, TIMER_STARTOP);
         result = CURLM_CALL_MULTI_PERFORM;
       }
       break;
@@ -1038,6 +1038,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
 
     case CURLM_STATE_CONNECT:
       /* Connect. We want to get a connection identifier filled in. */
+      Curl_pgrsTime(data, TIMER_STARTSINGLE);
       data->result = Curl_connect(data, &data->easy_conn,
                                   &async, &protocol_connect);
       if(CURLE_NO_CONNECTION_AVAILABLE == data->result) {

diff --git a/lib/progress.c b/lib/progress.c
index a242f8bc23e6f75624b17d9f41c1e155b47fffa0..e6a8d825a663398e00e397bc405361905e75a0ba 100644 (file)
--- a/lib/progress.c
+++ b/lib/progress.c
@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2013, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2014, 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
@@ -172,8 +172,12 @@ void Curl_pgrsTime(struct SessionHandle *data, timerid timer)
   case TIMER_NONE:
     /* mistake filter */
     break;
+  case TIMER_STARTOP:
+    /* This is set at the start of a transfer */
+    data->progress.t_startop = now;
+    break;
   case TIMER_STARTSINGLE:
-    /* This is set at the start of a single fetch */
+    /* This is set at the start of each single fetch */
     data->progress.t_startsingle = now;
     break;
 

diff --git a/lib/progress.h b/lib/progress.h
index f97fef99614e00f8d950bdf4ef85a566ad880dc9..a1e6f1a234cd8430570ba5a10e02d69550a8e987 100644 (file)
--- a/lib/progress.h
+++ b/lib/progress.h
@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2012, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2014, 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
@@ -27,13 +27,14 @@
 
 typedef enum {
   TIMER_NONE,
+  TIMER_STARTOP,
+  TIMER_STARTSINGLE,
   TIMER_NAMELOOKUP,
   TIMER_CONNECT,
   TIMER_APPCONNECT,
   TIMER_PRETRANSFER,
   TIMER_STARTTRANSFER,
   TIMER_POSTRANSFER,
-  TIMER_STARTSINGLE,
   TIMER_STARTACCEPT,
   TIMER_REDIRECT,
   TIMER_LAST /* must be last */

diff --git a/lib/urldata.h b/lib/urldata.h
index e62ea19304296b165cce38638ca9ccd5bddb1b24..640cbb1c626fb1eddca8ecfa3ef79a0132ba91b8 100644 (file)
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -1135,6 +1135,7 @@ struct Progress {
 
   struct timeval start;
   struct timeval t_startsingle;
+  struct timeval t_startop;
   struct timeval t_acceptdata;
 #define CURR_TIME (5+1) /* 6 entries for 5 seconds */
 

diff --git a/tests/unit/unit1303.c b/tests/unit/unit1303.c
index 2e4f23096b43d7b7b7e383fb166431355e5142b3..b48a6250cbea9f36a843cd0ee4c1b889c5bad2c1 100644 (file)
--- a/tests/unit/unit1303.c
+++ b/tests/unit/unit1303.c
@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2011, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2014, 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
@@ -131,6 +131,8 @@ const struct timetest run[] = {
 /* this is the pretended start time of the transfer */
 data->progress.t_startsingle.tv_sec = BASE;
 data->progress.t_startsingle.tv_usec = 0;
+data->progress.t_startop.tv_sec = BASE;
+data->progress.t_startop.tv_usec = 0;
 
 for(i=0; i < sizeof(run)/sizeof(run[0]); i++) {
   NOW(run[i].now_s, run[i].now_us);