Daniel, I read your message 3 times and still can't quite grasp all the logic. Could you perhaps rephrase it a bit?
The hardest to digest was "application says TIMEOUT and libcurl ...", what does it mean "to say" here, some API call where timeout value is included? or a separate call? If it is the earlier, there should not be any side-effects of passing a value as apparently described. Side-effects tend to break any other paradigm than original developer had in mind. the overall proposal seems sound, but it's really hard to understand. d. On 5 January 2014 17:02, Daniel Stenberg <[email protected]> wrote: > Hi friends, > > I realize this is deep down into multi socket details, but I'd like to do > this slightly changed behavior and would appreciate feedback. > > If you use event-based libcurl this may be interesting to you. > > ----------- > > BACKGROUND: > > We have learned that one some systems timeout timers are inaccurate and > might occasionally fire off too early. To make the multi_socket API work > with this, we made libcurl run timeout actions a bit early too if they > are within our MULTI_TIMEOUT_INACCURACY. (added in commit 2c72732ebf, > present since 7.21.0) > > Switching everything to the multi API made this inaccuracy problem > slightly more notable as now everyone can be affected. > > Recently we've tweaked that inaccuracy value to make timeouts more > accurate and it is now platform specific. We also noted that we have > code at places that check for fixed timeout values so they MUST NOT run > too early as then they will not trigger at all - so there are > definitately problems with running timeouts before they're supposed to > run. (We've handled that so far by adding the inaccuracy margin to those > specific timeouts.) > > The libcurl multi_socket API tells the application that a timeout > expires in N milliseconds (and it explicitly will not tell it again for > the same timeout), and the application is then supposed to call libcurl > when that timeout expires. When libcurl subsequently gets called with > TIMEOUT set, it knows that the application thinks a timeout did expire - > and alas, if it is within the inaccuracy level libcurl will run code > handling that handle. > > If the application says TIMEOUT to libcurl and _isn't_ within the > inaccuracy level, libcurl will not consider the timeout expired and it > will not tell the application again since the timeout value is still the > same. > > NOW: > > This change introduce a modified behavior here. If the application says > TIMEOUT and libcurl sees no timeout code to run, it will tell the > application about the timeout value - *again* even if it is the same > timeout that it already told about before. This way, we will not risk > that the application believes it has done its job and libcurl thinks the > time hasn't come yet to run any code and both just sit waiting. This > also allows us to decrease the MULTI_TIMEOUT_INACCURACY margin. > > A repeated timeout update to the application risk that the timeout will > then fire again immediately and we have what basically is a busy-loop > until the time is fine even for libcurl. If that becomes a problem, we > need to address it. > --- > lib/multi.c | 34 ++++++++++++++-------------------- > lib/multiif.h | 8 ++------ > 2 files changed, 16 insertions(+), 26 deletions(-) > > diff --git a/lib/multi.c b/lib/multi.c > index ebee674..636259b 100644 > --- a/lib/multi.c > +++ b/lib/multi.c > @@ -2229,31 +2229,25 @@ static CURLMcode multi_socket(struct Curl_multi > *multi, > > data = NULL; /* set data to NULL again to avoid calling > multi_runsingle() in case there's no need to */ > + now = Curl_tvnow(); /* get a newer time since the multi_runsingle() > loop > + may have taken some time */ > } > } > + else { > + /* Asked to run due to time-out. Clear the 'lastcall' variable to force > + update_timer() to trigger a callback to the app again even if the > same > + timeout is still the one to run after this call. That handles the > case > + when the application asks libcurl to run the timeout prematurely. */ > > - /* Compensate for bad precision timers that might've triggered too early. > - > - This precaution was added in commit 2c72732ebf3da5e as a result of bad > - resolution in the windows function use(d). > + memset(&multi->timer_lastcall, 0, sizeof(multi->timer_lastcall)); > > - The problematic case here is when using the multi_socket API and > libcurl > - has told the application about a timeout, and that timeout is what > fires > - off a bit early. As we don't have any IDs associated with the timeout > we > - can't tell which timeout that fired off but we only have the times to > use > - to check what to do. If it fires off too early, we don't run the > correct > - actions and we don't tell the application again about the same timeout > as > - was already first in the queue... > + /* Compensate for bad precision timers that might've triggered too > early */ > > - Originally we made the timeouts run 40 milliseconds early on all > systems, > - but now we have an #ifdef setup to provide a decent precaution > inaccuracy > - margin. > - */ > - > - now.tv_usec += MULTI_TIMEOUT_INACCURACY; > - if(now.tv_usec >= 1000000) { > - now.tv_sec++; > - now.tv_usec -= 1000000; > + now.tv_usec += MULTI_TIMEOUT_INACCURACY; > + if(now.tv_usec >= 1000000) { > + now.tv_sec++; > + now.tv_usec -= 1000000; > + } > } > > /* > diff --git a/lib/multiif.h b/lib/multiif.h > index 15163da..a7f6c6c 100644 > --- a/lib/multiif.h > +++ b/lib/multiif.h > @@ -7,7 +7,7 @@ > * | (__| |_| | _ <| |___ > * \___|\___/|_| \_\_____| > * > - * Copyright (C) 1998 - 2013, Daniel Stenberg, <[email protected]>, et al. > + * Copyright (C) 1998 - 2014, Daniel Stenberg, <[email protected]>, et al. > * > * This software is licensed as described in the file COPYING, which > * you should have received as part of this distribution. The terms > @@ -24,11 +24,7 @@ > > /* See multi_socket() for the explanation of this constant. Counted in > number > of microseconds. */ > -#ifdef WIN32 > -#define MULTI_TIMEOUT_INACCURACY 40000 > -#else > -#define MULTI_TIMEOUT_INACCURACY 3000 > -#endif > +#define MULTI_TIMEOUT_INACCURACY 990 > > #define MULTI_TIMEOUT_INACCURACY_MS (MULTI_TIMEOUT_INACCURACY / 1000) > > -- > 1.8.5.2 > > > -- > > / daniel.haxx.se > ------------------------------------------------------------------- > List admin: http://cool.haxx.se/list/listinfo/curl-library > Etiquette: http://curl.haxx.se/mail/etiquette.html ------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
