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