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 <daniel_at_haxx.se> 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, <daniel_at_haxx.se>, et al.
> + * Copyright (C) 1998 - 2014, Daniel Stenberg, <daniel_at_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
> @@ -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
Received on 2014-01-07