On Fri, Oct 19, 2012 at 02:04:20PM -0700, sza...@google.com wrote:
> From 32e06128dbc97ceb0d060c88ec8db204fa51be5c Mon Sep 17 00:00:00 2001
> From: Stefan Zager <sza...@google.com>
> Date: Thu, 18 Oct 2012 16:23:53 -0700
Drop these lines.
> Subject: [PATCH] Fix potential hang in https handshake.
And make this one your actual email subject.
> It has been observed that curl_multi_timeout may return a very long
> timeout value (e.g., 294 seconds and some usec) just before
> curl_multi_fdset returns no file descriptors for reading. The
> upshot is that select() will hang for a long time -- long enough for
> an https handshake to be dropped. The observed behavior is that
> the git command will hang at the terminal and never transfer any
> data.
>
> This patch is a workaround for a probable bug in libcurl. The bug
> only seems to manifest around a very specific set of circumstances:
>
> - curl version (from curl/curlver.h):
>
> #define LIBCURL_VERSION_NUM 0x071307
>
> - git-remote-https running on an ubuntu-lucid VM.
> - Connecting through squid proxy running on another VM.
>
> Interestingly, the problem doesn't manifest if a host connects
> through squid proxy running on localhost; only if the proxy is on
> a separate VM (not sure if the squid host needs to be on a separate
> physical machine). That would seem to suggest that this issue
> is timing-sensitive.
Thanks, that explanation makes much more sense.
> diff --git a/http.c b/http.c
> index df9bb71..51eef02 100644
> --- a/http.c
> +++ b/http.c
> @@ -631,6 +631,17 @@ void run_active_slot(struct active_request_slot *slot)
> FD_ZERO(&excfds);
> curl_multi_fdset(curlm, &readfds, &writefds, &excfds,
> &max_fd);
>
> + /* It can happen that curl_multi_timeout returns a
> pathologically
> + * long timeout when curl_multi_fdset returns no file
> descriptors
> + * to read. See commit message for more details.
> + */
Minor nit, but our multi-line comment style is:
/*
* blah blah blah
*/
> + if (max_fd < 0 &&
> + select_timeout.tv_sec > 0 ||
> + select_timeout.tv_usec > 50000) {
> + select_timeout.tv_sec = 0;
> + select_timeout.tv_usec = 50000;
> + }
Should there be parentheses separating the || bit from the &&?
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
