Re: Fix potential hang in https handshake.

[Jeff King <peff@peff.net>]

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...

-Peff