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 <[email protected]> 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
