Tarantool development patches archive
 help / color / mirror / Atom feed
* [PATCH] httpc: fix zero timeout handling
@ 2019-04-28  0:53 Alexander Turenko
  2019-04-29 17:23 ` Vladimir Davydov
  0 siblings, 1 reply; 2+ messages in thread
From: Alexander Turenko @ 2019-04-28  0:53 UTC (permalink / raw)
  To: Vladimir Davydov; +Cc: Alexander Turenko, tarantool-patches

When libcurl is built with --enable-threaded-resolver (which is default)
and the version of the library is 7.60 or above, libcurl calls a timer
callback with exponentially increasing timeout_ms value during DNS
resolving.

This behaviour was introduced in curl-7_59_0-36-g67636222f (see [1],
[2]). During first ten milliseconds the library sets a timer to a passed
time divided by three (see Curl_resolver_getsock()). It is possible that
passed time is zero during at least several thousands of iterations.

Before this commit we didn't set a libev timer in curl_multi_timer_cb()
when a timeout_ms value is zero, but call curl_multi_process()
immediately. Libcurl however can call curl_multi_timer_cb() again and
here we're going into a recursion that stops only when timeous_ms
becomes positive. Often we generate several thousands of stack frames
within this recursion and exceed 512KiB of a fiber stack size.

The fix is easy: set a libev timer to call curl_multi_process() even
when a timeout_ms value is zero.

The reason why we did the call to curl_multi_process() immediately is
the unclear wording in the CURLMOPT_TIMERFUNCTION option documentation.
This documentation page was fixed in curl-7_64_0-88-g47e540df8 (see [3],
[4], [5]).

There is also the related change in curl-7_60_0-121-g3ef67c686 (see [6],
[7]): after this commit libcurl calls a timer callback with zero
timeout_ms during a first three milliseconds of asynchronous DNS
resolving.

Fixes #4179.

[1]: https://github.com/curl/curl/pull/2419
[2]: https://github.com/curl/curl/commit/67636222f42b7db146b963deb577a981b4fcdfa2
[3]: https://github.com/curl/curl/issues/3537
[4]: https://github.com/curl/curl/pull/3601
[5]: https://github.com/curl/curl/commit/47e540df8f32c8f7298ab1bc96b0087b5738c257
[6]: https://github.com/curl/curl/pull/2685
[7]: https://github.com/curl/curl/commit/3ef67c6861c9d6236a4339d3446a444767598a58
---

https://github.com/tarantool/tarantool/issues/4179
https://github.com/tarantool/tarantool/tree/Totktonada/gh-4179-libcurl-explodes-stack

 src/curl.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/curl.c b/src/curl.c
index b85a4553b..788fdade1 100644
--- a/src/curl.c
+++ b/src/curl.c
@@ -121,7 +121,7 @@ curl_multi_timer_cb(CURLM *multi, long timeout_ms, void *envp)
 
 	say_debug("curl %p: wait timeout=%ldms", env, timeout_ms);
 	ev_timer_stop(loop(), &env->timer_event);
-	if (timeout_ms > 0) {
+	if (timeout_ms >= 0) {
 		/*
 		 * From CURLMOPT_TIMERFUNCTION manual:
 		 * Your callback function should install a non-repeating timer
@@ -132,15 +132,6 @@ curl_multi_timer_cb(CURLM *multi, long timeout_ms, void *envp)
 		ev_timer_init(&env->timer_event, curl_timer_cb, timeout, 0);
 		ev_timer_start(loop(), &env->timer_event);
 		return 0;
-	} else if (timeout_ms == 0) {
-		/*
-		 * From CURLMOPT_TIMERFUNCTION manual:
-		 * A timeout_ms value of 0 means you should call
-		 * curl_multi_socket_action or curl_multi_perform (once) as
-		 * soon as possible.
-		 */
-		curl_timer_cb(loop(), &env->timer_event, 0);
-		return 0;
 	} else {
 		assert(timeout_ms == -1);
 		/*
-- 
2.21.0

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-04-29 17:23 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-28  0:53 [PATCH] httpc: fix zero timeout handling Alexander Turenko
2019-04-29 17:23 ` Vladimir Davydov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox