From: Felipe Contreras Date: Tue, 02 Apr 2013 04:51:20 GMT Subject: Re: [PATCH 2/4] transport-helper: check if remote helper is alive Message-ID: In-Reply-To: <20130402023024.GB719@sigill.intra.peff.net> On Mon, Apr 1, 2013 at 8:30 PM, Jeff King wrote: > 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? The problem is not with import, since fast-import would generally wait properly for a 'done' status, the problem is with export. Also, the design is such that the remote-helper stays alive, even after fast-export has finished. -- Felipe Contreras