Re: Fix potential hang in https handshake.

[Junio C Hamano <gitster@pobox.com>]

szager@google.com writes:

> 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);



>  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);
>  		}
>  	}