I guess I understood you correctly the first time.

I think it's a good change.

In fact, I think it's a good change irrespective of time accuracy.
That is to say the old behaviour where libcurl inferred application's
intention was very error-prone. As in, IMO there should be no
difference if an application calls
curl_multi_socket_action(...CURL_SOCKET_TIMEOUT...) an extra time.

my 2c,
d.

On 8 January 2014 08:35, Daniel Stenberg <daniel_at_haxx.se> wrote:
> On Tue, 7 Jan 2014, Dima Tisnek wrote:
>
>> 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.
>
>
> Thank you. I took a step back and I decided to *only* do the "reminding"
> part as the first commit as this is the big thing that I'm talking about and
> I shouldn't mix different things into the same change. (The other changes
> were minor in comparison and I'll address them separately.)
>
> I then went through and tried to clarify the description by using the
> correct function names and defines from curl headers to allow readers to
> easier check the documentation for details on on them. I also decided to
> include a bunch of commit references to changes I'm referring to in the
> text.
>
> Does this make more sense or is still too vague? It will still assume that
> you're somewhat familiar with the multi_socket API/paradigm.
>
> -------------
>
> PATCH v2] multi_socket: remind app if timeout didn't run
>
> BACKGROUND:
>
> We have learned that on 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 execute 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 (commit 21091549c02) we tweaked that inaccuracy value to make
> timeouts more accurate and made it platform specific. We also figured
> out 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 (see
> commit be28223f35 and a691e044705) - 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 with a callback 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 curl_multi_socket_action(...CURL_SOCKET_TIMEOUT...), it
> knows that the application thinks the timeout expired - and alas, if it
>
> is within the inaccuracy level libcurl will run code handling that
> handle.
>
> If the application says CURL_SOCKET_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 introduces a modified behavior here. If the application says
> CURL_SOCKET_TIMEOUT and libcurl finds no timeout code to run, it will
> inform the application about the timeout value - *again* even if it is
> the same timeout that it already told about before (although libcurl
> will of course tell it the updated time so that it'll still get the
> correct remaining time). 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, but that will be handled
> in a separate commit.
>
>
> 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 | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/lib/multi.c b/lib/multi.c
> index 698a99e..3af460d 100644
> --- a/lib/multi.c
> +++ b/lib/multi.c
> @@ -2233,6 +2233,13 @@ static CURLMcode multi_socket(struct Curl_multi
> *multi,
>
> multi_runsingle() in case there's no need to */
> }
> }
> + 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. */
> + memset(&multi->timer_lastcall, 0, sizeof(multi->timer_lastcall));
> + }
>
> /* Compensate for bad precision timers that might've triggered too early.
>
>
> --
> 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-08