On Tuesday 24 August 2010 22:25:10 Daniel Stenberg wrote:
> expire 0 removes all timeouts associated to that particular handle. If there
> are more handles, the other handles may of course have timeouts and they will
> then be affecting the single timeout timer libcurl exposes to the app. All
> involved easy handles' timeout values exist in a sorted (splay) tree and the
> closest one in time is what is signalled to the app.
>
> But imagine only one handle added that sets CURLOPT_TIMEOUT to something, then
> we set a timeout to limit the resolve or the connect phase, and after each of
> those phases we kill the previous timeout with an expire 0 (I'm not sure we
> do), then the initial CURLOPT_TIMEOUT is gone.
I didn't got any expire 0 before DONE state is reached. But I'm using
cares. And with --enable-threaded-resolver then there are many
expire 0 followed by a expire (1,2,4,8...), the resolver is polling.

While in resolving state these expire 0 are harmless, because the other
timeouts are not set yet. The connect timeout will be set later.
And this time out works.

But after curl got the connection no new timeout is set. Curl_readwrite
checks if the transfer timed-out. But if the server is silent, then this
function will never be called and the transfer will live forever.

My test cgi sleeps for 15 seconds before it returns a single line.
And the timeout in the multi client is set to 10 seconds, then I get
bit confusing message:
(28) Operation timed out after 15007 milliseconds with 21 bytes received

It happend 5s too late and also transfer is done.

My patch fixes that. But indeed it's a bad patch. The operation
timeout should be set only one time. But I'm not sure where the
correct place is. I think it must be added to many different
states which would be ugly c&p, so I've put it into
Curl_pgrsTime's TIMER_CONNECT case block:

  case TIMER_CONNECT:
    now = Curl_tvnow();
    data->progress.t_connect =
      Curl_tvdiff_secs(now, data->progress.t_startsingle);
    /* remove old connect timeout, we don't need it any more */
    Curl_expire(data, 0);
    if(data->set.timeout) {
      /* add new tranfer timeout */
      long timeout_ms = data->set.timeout - Curl_tvdiff(now, data->progress.t_startsingle);
      Curl_expire(data, timeout_ms>0 ? timeout_ms : 1 );
    }

Looks really misplaced... but I couldn't find a better place.

Now a transfer timeout is set. It works for the first transfer,
but not for the next. But after some time a new transfer
will time out again...strange. After some iterations
I found the timeout will not happen when curl said me

* 0x21f1468 is at send pipe head!
* expire [0x21f1468|0x21c4c98] 1ms
* update
* Expire at 21332 / 401983 (1ms) 0x21f1468

(the middle two lines are debug msg I've added to Curl_expire)

And this replaced the transfer timeout. So Curl_expire(..., 1)
and not Curl_expire(..., 0) is the trouble maker.

I cannot spend more time with digging into that problem.
So I went back to my previous patch with some modification,
until somebody else has done a better fix.

diff --git a/lib/multi.c b/lib/multi.c
index 4bf74a7..ace2a46 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -1505,6 +1505,11 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
         }
       }
 
+ if(easy->state != CURLM_STATE_DONE && data->set.timeout) {
+ /* don't finished, set transfer timeout */
+ long timeout_ms = data->set.timeout - Curl_tvdiff(Curl_tvnow(), data->progress.t_startsingle);
+ Curl_expire(data, timeout_ms>0 ? timeout_ms : 1 );
+ }
       break;
 
     case CURLM_STATE_DONE:
@@ -2519,12 +2524,15 @@ void Curl_expire(struct SessionHandle *data, long milli)
          Compare if the new time is earlier, and only remove-old/add-new if it
          is. */
       long diff = curlx_tvdiff(set, *nowp);
- if(diff > 0) {
+ if(diff > 1) {
         /* the new expire time was later so just add it to the queue
            and get out */
         multi_addtimeout(data->state.timeoutlist, &set);
         return;
       }
+ else if(diff >= 0)
+ /* equal or 1ms difference, do nothing */
+ return;
 
       /* the new time is newer than the presently set one, so add the current
          to the queue and update the head */

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2010-08-25