Re: [TANGENT] run-command: use vfork instead of fork

From: Brandon Williams <bmwill@google.com>
Date: May 16, 2017, 21:11 UTC
Message-ID: <20170516211125.GA74302@google.com>
In-Reply-To: <CA+55aFygKMJH3BRitQftmtCTx6es93CTR4_y1ULGGE2rSsMyog@mail.gmail.com>

On 05/16, Linus Torvalds wrote:

Show 14 quoted lines

> On Tue, May 16, 2017 at 12:35 PM, Eric Wong <e@80x24.org> wrote:
> >
> > Fwiw, most of the vfork preparation was already done by Brandon
> > and myself a few weeks ago, and cooking in pu.
> 
> Oh, interesting. Was that done for vfork(), or is it for something
> else? Some of the changes seem almost overly careful. Is this for
> prep-work porting to some odd environment that doesn't really have a
> MMU at all? There's nothing fundamentally wrong with allocating memory
> after fork().
> 
> But yes, it looks like it helps the vfork case.
> 
>                Linus

I started working on the run-command code when I ran into a deadlock in 'git grep --recurse-submodules'. When I added support for submodules to grep I just assumed that launching a process (which atm is unfortunately the only way to work on a submodule) would work in a multi-threaded environment. I was naive and wrong!

The deadlock was due to a malloc lock being held by thread 'A' while thread 'b' tried to launch a process. Since that lock was in a locked-state at the time of forking, it remained in a locked-state with no hope of ever being released. So when the child process that thread 'b' spawned tried to malloc a chunk of memory after forking, it deadlocked.

I didn't catch this in initial testing because gclib registers atfork_handelers in order to prevent this sort of thing, while libraries like tcmalloc don't do this.

So to account for this, I worked to make run-command safe to use in the presence of threads, which had the benefit of also preparing it to be vfork() ready.

Ultimately I'd like to drop the requirement to spawn a child process to work on a submodule, but that's going to take a lot more effort.

-- 
Brandon Williams