On Tuesday 12 May 2009 11:59:13 pm Yang Tse wrote:
> 2009/5/13, Luke Dashjr wrote:
> > [...] Is this patch acceptable as-is, or does anyone have any
> > suggestions/needs for revision before it can be merged into the next
> > version of libcurl?
>
> First comments come directly from the compiler. Better first fix all
> errors and warnings...
Whoops, I guess I forgot to test that last-minute safety check. Didn't realize
libcurl was supposed to be C90 conforming either, that's not the default
compile flag for me... Anyhow, here's a revised patch with those issues fixed.
On Wednesday 13 May 2009 05:03:13 am Daniel Stenberg wrote:
> I think the fix need to be done differently and possibly it needs a
> somewhat larger take. I'll elaborate on the timeout logic...
This version of my code (I had an earlier O(n) one that just checked every
easy-handle at curl_multi_timeout call) does use the splay tree. I noticed
that the multi timeouts were only ever 1ms or disabled, so this new function
is run in the cases where it would have been disabled only. Therefore, the 1ms
timeouts always should override the user-defined timeout which only comes into
action when there would be no other timeout set. Or at least that was what I
tried to do-- did I miss something?
Luke
Index: lib/multi.c
===================================================================
RCS file: /cvsroot/curl/curl/lib/multi.c,v
retrieving revision 1.198
diff -u -r1.198 multi.c
--- lib/multi.c 11 May 2009 07:53:38 -0000 1.198
+++ lib/multi.c 12 May 2009 22:37:02 -0000
@@ -887,6 +887,31 @@
return CURLM_OK;
}
+static void update_timeout(struct SessionHandle *data)
+{
+ struct timeval now = Curl_tvnow();
+ struct timeval start = data->req.start;
+ if (!start.tv_secs && !start.tv_usecs)
+ {
+ // FIXME: Should we recalculate timeout again later?
+ Curl_expire(data, 0);
+ return;
+ }
+
+ long timeout = data->set.timeout;
+ long passed = curlx_tvdiff(now, start);
+ if (passed > timeout) {
+ // Timeout occurs in the past
+ // FIXME: what if the timeout hasn't be processed yet?
+ Curl_expire(data, 0);
+ return;
+ }
+ --passed; // round any sub-ms delay up
+
+ long remaining = timeout - passed;
+ Curl_expire(data, remaining);
+}
+
static CURLMcode multi_runsingle(struct Curl_multi *multi,
struct Curl_one_easy *easy)
{
@@ -936,6 +961,7 @@
easy->result=Curl_pretransfer(easy->easy_handle);
if(CURLE_OK == easy->result) {
+ update_timeout(easy->easy_handle);
/* after init, go CONNECT */
multistate(easy, CURLM_STATE_CONNECT);
result = CURLM_CALL_MULTI_PERFORM;
@@ -1537,6 +1563,8 @@
splay tree */
tv->tv_sec = 0;
tv->tv_usec = 0;
+
+ update_timeout(d);
}
} while(t);
@@ -1909,6 +1937,8 @@
/* get the socket(s) and check if the state has been changed since
last */
singlesocket(multi, data->set.one_easy);
+
+ update_timeout(data);
}
/* Check if there's one (more) expired timer to deal with! This function
