Re: [PATCH 2/4] transport-helper: check if remote helper is alive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 01, 2013 at 03:46:42PM -0600, Felipe Contreras wrote:

> Otherwise transport-helper will continue checking for refs and other
> things what will confuse the user more.
> [...]
> diff --git a/transport-helper.c b/transport-helper.c
> index cb3ef7d..dfdfa7a 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -460,6 +460,10 @@ static int fetch_with_import(struct transport *transport,
>  
>  	if (finish_command(&fastimport))
>  		die("Error while running fast-import");
> +
> +	if (!check_command(data->helper))
> +		die("Error while running helper");
> +
>  	argv_array_free_detached(fastimport.argv);

Can you be more specific about what happens when we miss the death here,
what happens next, etc?

Checking asynchronously for death like this is subject to a race
condition; the helper may be about to die but not have died yet. In
practice we may catch some cases, but this seems like an indication that
the protocol is not well thought-out. Usually we would wait for a
confirmation over the read pipe from a child, and know that the child
failed when either:

  1. It tells us so on the pipe.

  2. The pipe closes (at which point we know it is time to reap the
     child).

Why doesn't that scheme work here? I am not doubting you that it does
not; the import helper protocol is a bit of a mess, and I can easily
believe it has such a problem. But I'm wondering if it's possible to
improve it in a more robust way.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]