Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: Alexander Turenko <alexander.turenko@tarantool.org>,
	tarantool-patches@freelists.org
Subject: [PATCH] httpc: fix zero timeout handling
Date: Sun, 28 Apr 2019 03:53:58 +0300	[thread overview]
Message-ID: <306168d6016a149f02524033bce37177fde6267c.1556411469.git.alexander.turenko@tarantool.org> (raw)

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

             reply	other threads:[~2019-04-28  0:53 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-28  0:53 Alexander Turenko [this message]
2019-04-29 17:23 ` Vladimir Davydov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=306168d6016a149f02524033bce37177fde6267c.1556411469.git.alexander.turenko@tarantool.org \
    --to=alexander.turenko@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH] httpc: fix zero timeout handling' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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