From 2c72732ebf3da5eba414f08507a28a90a1ec1f2c Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 1 Jun 2010 23:18:34 +0200 Subject: [PATCH] multi_socket: handles timer inaccuracy better for timeouts Igor Novoseltsev reported a problem with the multi socket API and using timeouts and timers. It boiled down to a problem with libcurl's use of GetTickCount() interally to figure out the current time, while Igor's own application code used another function call. It made his app call the socket API timeout function a bit _before_ libcurl would consider the timeout to trigger, and that could easily lead to timeouts or stalls in the app. It seems GetTickCount() in general often has no better resolution than 16ms and switching to the alternative function QueryPerformanceCounter has its share of problems: http://www.virtualdub.org/blog/pivot/entry.php?id=106 We address this problem by simply having libcurl treat timers that already has occured or will occur within 40ms subject for treatment. I'm confident that there are other implementations and operating systems with similarly in accurate timer functions so it makes sense to have applied generically and I don't believe we sacrifice much by adding a 40ms inaccuracy on these timeouts. --- CHANGES | 20 ++++++++++++++++++++ RELEASE-NOTES | 4 +++- lib/multi.c | 8 +++++--- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index f8d9b3653..914b2f9ee 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,26 @@ Changelog +Daniel Stenberg (1 June 2010) +- Igor Novoseltsev reported a problem with the multi socket API and using + timeouts and timers. It boiled down to a problem with libcurl's use of + GetTickCount() interally to figure out the current time, while Igor's own + application code used another function call. + + It made his app call the socket API timeout function a bit _before_ libcurl + would consider the timeout to trigger, and that could easily lead to + timeouts or stalls in the app. It seems GetTickCount() in general often has + no better resolution than 16ms and switching to the alternative function + QueryPerformanceCounter has its share of problems: + http://www.virtualdub.org/blog/pivot/entry.php?id=106 + + We address this problem by simply having libcurl treat timers that already + has occured or will occur within 40ms subject for treatment. I'm confident + that there are other implementations and operating systems with similarly in + accurate timer functions so it makes sense to have applied generically and I + don't believe we sacrifice much by adding a 40ms inaccuracy on these + timeouts. + Kamil Dudka (27 May 2010) - added a new test for CRL support (test313) diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 86260d14f..7639afe28 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -33,6 +33,7 @@ This release includes the following bugfixes: o OpenSSL handshake state-machine for multi interface o TFTP timeout option sent correctly o TFTP block id wrap + o curl_multi_socket_action() timeout handles inaccuracy in timers better This release includes the following known bugs: @@ -43,6 +44,7 @@ advice from friends like these: Rainer Canavan, Paul Howarth, Jerome Vouillon, Ruslan Gazizov, Yang Tse, Kamil Dudka, Alex Bligh, Ben Greear, Hoi-Ho Chan, Howard Chu, Dirk Manske, - Pavel Raiskup, John-Mark Bell, Eric Mertens, Tor Arntsen, Douglas Kilpatrick + Pavel Raiskup, John-Mark Bell, Eric Mertens, Tor Arntsen, Douglas Kilpatrick, + Igor Novoseltsev Thanks! (and sorry if I forgot to mention someone) diff --git a/lib/multi.c b/lib/multi.c index 44771a309..06ffbd7b1 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -1994,9 +1994,11 @@ static CURLMcode multi_socket(struct Curl_multi *multi, extracts a matching node if there is one */ now = Curl_tvnow(); - now.tv_usec += 1000; /* to compensate for the truncating of 999us to 0ms, - we always add time here to make the comparison - below better */ + now.tv_usec += 40000; /* compensate for bad precision timers */ + if(now.tv_usec > 1000000) { + now.tv_sec++; + now.tv_usec -= 1000000; + } multi->timetree = Curl_splaygetbest(now, multi->timetree, &t); if(t) {