Re: Fix potential hang in https handshake.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 18, 2012, 22:59 UTC
- Message-ID
- <7vd30fl736.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <5080761e.IEDySUuQzlCwL6fM%szager@google.com>
szager@google.com writes:
Show 14 quoted lines
> From 700b8075c578941c8f951711825c390ac68b190f Mon Sep 17 00:00:00 2001 > From: Stefan Zager <szager@google.com> > Date: Thu, 18 Oct 2012 14:03:59 -0700 > Subject: [PATCH] Fix potential hang in https handshake. > > It will sometimes happen that curl_multi_fdset() doesn't > return any file descriptors. In that case, it's recommended > that the application sleep for a short time before running > curl_multi_perform() again. > > http://curl.haxx.se/libcurl/c/curl_multi_fdset.html > > Signed-off-by: Stefan Zager <szager@google.com> > ---
Thanks. Would it be a better idea to "patch up" in problematic case, instead of making this logic too deeply nested, like this instead, I have to wonder...
... all the existing code above unchanged ...
curl_multi_fdset(..., &max_fd);
+ if (max_fd < 0) {
+ /* nothing actionable??? */
+ select_timeout.tv_sec = 0;
+ select_timeout.tv_usec = 50000;
+ }select(max_fd+1, ..., &select_timeout);
Show 69 quoted lines
> http.c | 40 ++++++++++++++++++++++++++--------------
> 1 files changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/http.c b/http.c
> index df9bb71..a6f66c0 100644
> --- a/http.c
> +++ b/http.c
> @@ -602,35 +602,47 @@ void run_active_slot(struct active_request_slot *slot)
> int max_fd;
> struct timeval select_timeout;
> int finished = 0;
> + long curl_timeout;
>
> slot->finished = &finished;
> while (!finished) {
> step_active_slots();
>
> if (slot->in_use) {
> + max_fd = -1;
> + FD_ZERO(&readfds);
> + FD_ZERO(&writefds);
> + FD_ZERO(&excfds);
> + curl_multi_fdset(curlm, &readfds, &writefds, &excfds, &max_fd);
> +
> #if LIBCURL_VERSION_NUM >= 0x070f04
> - long curl_timeout;
> - curl_multi_timeout(curlm, &curl_timeout);
> - if (curl_timeout == 0) {
> - continue;
> - } else if (curl_timeout == -1) {
> + /* It will sometimes happen that curl_multi_fdset() doesn't
> + return any file descriptors. In that case, it's recommended
> + that the application sleep for a short time before running
> + curl_multi_perform() again.
> +
> + http://curl.haxx.se/libcurl/c/curl_multi_fdset.html
> + */
> + if (max_fd == -1) {
> select_timeout.tv_sec = 0;
> select_timeout.tv_usec = 50000;
> } else {
> - select_timeout.tv_sec = curl_timeout / 1000;
> - select_timeout.tv_usec = (curl_timeout % 1000) * 1000;
> + curl_timeout = 0;
> + curl_multi_timeout(curlm, &curl_timeout);
> + if (curl_timeout == 0) {
> + continue;
> + } else if (curl_timeout == -1) {
> + select_timeout.tv_sec = 0;
> + select_timeout.tv_usec = 50000;
> + } else {
> + select_timeout.tv_sec = curl_timeout / 1000;
> + select_timeout.tv_usec = (curl_timeout % 1000) * 1000;
> + }
> }
> #else
> select_timeout.tv_sec = 0;
> select_timeout.tv_usec = 50000;
> #endif
> -
> - max_fd = -1;
> - FD_ZERO(&readfds);
> - FD_ZERO(&writefds);
> - FD_ZERO(&excfds);
> - curl_multi_fdset(curlm, &readfds, &writefds, &excfds, &max_fd);
> -
> select(max_fd+1, &readfds, &writefds, &excfds, &select_timeout);
> }
> }