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 06:12:45PM -0600, Felipe Contreras wrote:

> > Checking asynchronously for death like this is subject to a rac
> > 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.
> 
> The pipe is between fast-export and the remote-helper, "we"
> (transport-helper) are not part of the pipe any more. That's the
> problem.

So in fetch_with_import, we have a remote-helper, and we have a
bidirectional pipe to it. We then call get_importer, which starts
fast-import, whose stdin is connected to the stdout of the remote
helper. We tell the remote-helper to run the import, then we wait for
fast-import to finish (and complain if it fails).

Then what? We seem to do some more work, which I think is what causes
the errors you see; but should we instead be reaping the helper at this
point unconditionally? Its stdout has presumably been flushed out to
fast-import; is there anything else for us to get from it besides its
exit code?

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