git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Fix potential hang in https handshake.

From
Shawn Pearce <spearce@spearce.org>
Date
Oct 19, 2012, 14:10 UTC
Message-ID
<CAJo=hJvWV0WPN5rCYK-JxfaEPWp7syUM1H0w4=Eb27=50+pXjg@mail.gmail.com>
In-Reply-To
<20121019103627.GA29366@sigill.intra.peff.net>
On Fri, Oct 19, 2012 at 3:36 AM, Jeff King <peff@peff.net> wrote:
Show 47 quoted lines
> On Thu, Oct 18, 2012 at 03:59:41PM -0700, Junio C Hamano wrote:
>
>> > 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);
>
> But wouldn't that override a potentially shorter timeout that curl gave
> us via curl_multi_timeout, making us unnecessarily slow to hand control
> back to curl?
>
> The current logic is:
>
>   - if curl says there is something to do now (timeout == 0), do it
>     immediately
>
>   - if curl gives us a timeout, use it with select
>
>   - otherwise, feed 50ms to selection
>
> It should not matter what we get from curl_multi_fdset. If there are
> fds, great, we will feed them to select with the timeout, and we may
> break out early if there is work to do. If not, then we are already
> doing this wait.
>
> IOW, it seems like we are _already_ following the advice referenced in
> curl's manpage. Is there some case I am missing? Confused...

The issue with the current code is sometimes when libcurl is opening a CONNECT style connection through an HTTP proxy it returns a crazy high timeout (>240 seconds) and no fds. In this case Git waits forever. Stefan observed that using a timeout of 50 ms in this situation to poll libcurl is better, as it figures out a lot more quickly that it is connected to the proxy and can issue the request.

Previous: Jeff KingNext: Daniel Stenberg
Message 4 of 10 in “Fix potential hang in https handshake.”
  1. szager@google.comOct 18, 2012
  2. Junio C HamanoOct 18, 2012
  3. Jeff KingOct 19, 2012
  4. Shawn PearceOct 19, 2012
  5. Daniel StenbergOct 19, 2012
  6. Jeff KingOct 19, 2012
  7. Stefan ZagerOct 19, 2012
  8. Jeff KingOct 19, 2012
  9. Junio C HamanoOct 19, 2012
  10. Junio C HamanoOct 19, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.