Re: [PATCH v2 0/6] forking and threading

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Apr 13, 2017, 20:50 UTC
Message-ID: <20170413205008.GB10084@aiede.mtv.corp.google.com>
In-Reply-To: <20170413183252.4713-1-bmwill@google.com>

Brandon Williams wrote:

> From what I can see, there are now no calls in the child process (after fork
> and before exec/_exit) which are not Async-Signal-Safe.  This means that
> fork/exec in a threaded context should work without deadlock

I don't see why the former implies the latter. Can you explain further?

You already know my opinions about fork+threads by now. I continue to think this is heading in a direction of decreased maintainability that I dread.

That's not to say that this is wasted work. I would prefer an approach like the following:

 1. First, make grep work without running fork() from threads.
    Jonathan Tan's approach would be one way to do this.  Another way
    would be to simply disable threads in --recurse-submodules mode.

    This would be the first thing to do because it would make tests
    reliable again, without having to wait for deeper changes.

 2. Then, teaching run_command to prepare the environment and do $PATH
    lookup before forking.  This might make it possible for run_command
    to use vfork or might not.

 3. Teaching run_command to delegate chdir to the child, using -C for
    git commands and 'cd' for shell commands, and using a shell where
    necessary where it didn't before.

 4. Switching run_command to use posix_spawn on platforms where it is
    available, which would make it possible to use in a threaded
    context on those platforms.

Thoughts? Jonathan