Subject: Re: [PATCH 5/5] run-command: add note about forking and threading

From: Brandon Williams <[email-masked]>
Date: Tue, 11 Apr 2017 10:34:45 -0700

On 04/11, Eric Wong wrote:
> Jonathan Nieder <[email-masked]> wrote:
> > Why can't git use e.g. posix_spawn to avoid this?
> 
> posix_spawn does not support chdir, and it seems we run non-git
> commands so no using "git -C" for those.

This is actually the biggest reason why I didn't go down that route from
the start.  I didn't want to dig through each and every user of
run-command and verify that removing chdir wouldn't break them (or add
in some other way to do it).

> 
> > fork()-ing in a threaded context is very painful for maintainability.
> > Any library function you are using could start taking a lock, and then
> > you have a deadlock.  So you have to make use of a very small
> > whitelisted list of library functions for this to work.
> 
> Completely agreed.

Yes it is difficult to get right, but it seems very doable to just do
all of the heavy-lifting prior to fork/exec making it easier to just use
async-safe between the fork/exec in the child.

> 
> On the other hand, I believe we should make run-command
> vfork-compatible (and Brandon's series is a big (but incomplete)
> step in the (IMHO) right direction); as anything which is
> vfork-safe would also be safe in the presence of threads+(plain) fork.
> With vfork; the two processes share heap until execve.

I haven't looked to much into vfork, one of the benefits of vfork is
that it is slightly more preferment than vanilla fork correct?  What are
some of the other benefits of using vfork over fork?

> 
> I posted some notes about it last year:
> 
>   https://public-inbox.org/git/[email-masked]/
> 
> > The function calls you have to audit are not only between fork() and
> > exec() in the normal control flow.  You have to worry about signal
> > handlers, too.
> 
> Yes, all that auditing is necessary for vfork; too, but totally
> doable.  The mainline Ruby implementation has been using vfork
> for spawning subprocesses for several years, now; and I think the
> ruby-core developers (myself included) have fixed all the
> problems with it; even in multi-threaded code which calls malloc.

-- 
Brandon Williams