On Thu, May 16, 2019 at 07:08:34AM +0900, Mike Hommey wrote: > > >> - Except, well, fds being what they are, we in fact just closed a fd > > >> from a packed_git->pack_fd. So, when use_pack is later called, and > > >> tries to mmap data from that pack, it fails because the file > > >> descriptor was closed. > > > > Either dup() the file descriptor, or mmap() before you call the > > consuming start_command(). If I understand your case correctly, the mmap() is not relevant. The issue is that we close a random file descriptor accidentally, and it just happens to be a pack descriptor later on. Right? > You seem to imply this is my code doing something. It's not. The process > that is doing all the things I noted above is an unmodified `git fetch`, > when using a remote-helper transport. The use_pack happens after the > transport is disposed because that's at the end of git fetch, when it > updates refs. Yes, it's a bug in git. > Anyway, it would seem the fix is to dup(out) when passing it as in to > start_command? Generally, yes. It looks like maybe this spot? diff --git a/transport-helper.c b/transport-helper.c index fcd2a58d0e..45cdf891ec 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -433,7 +433,7 @@ static int get_importer(struct transport *transport, struct child_process *fasti struct helper_data *data = transport->data; int cat_blob_fd, code; child_process_init(fastimport); - fastimport->in = helper->out; + fastimport->in = xdup(helper->out); argv_array_push(&fastimport->args, "fast-import"); argv_array_push(&fastimport->args, debug ? "--stats" : "--quiet"); One thing I'd wonder, though: what is the contract between the helper and fast-import here? In the current code, when the helper closes its stdout, fast-import will see EOF. But not if we are holding on to another copy of the descriptor. In that case, the right solution is probably more like: fastimport->in = helper->out; helper->out = -1; /* hand ownership to fast-import */ -Peff
