On 10/09/2023 11:05, Oswald Buddenhagen wrote:
The child not dying is tricky, if it is in the same process group as
git then even if git dies the I think the shell will wait for the
child to exit before showing the prompt again so it is not clear to me
that the user is disadvantaged by git ignoring SIGINT in that case.
there is no such thing as waiting for grandchildren. the grandchild is
reparented to init when the child exits.
there is a situation were one can be deadlocked by a non-exiting
grandchild: when doing a blocking read of the child's output past its
exit, when the grandchild has inherited stdout. but that's a
implementation bug in the parent. and not relevant here.
Yes I got carried away and thought that the shell waited for all the
processes in the foreground process group, but it can only wait on those
processes that it created.
On Fri, Sep 08, 2023 at 02:11:51PM +0100, Phillip Wood wrote:
On 08/09/2023 10:59, Phillip Wood wrote:
I've never done it before, but from my reading we basically want to do
(in the forked process before we exec):
setsid();
open("/dev/tty");
Do we want a whole new session? As I understand it to launch a
foreground job shells put the child in its own process group and then
call tcsetpgrp() to change the foreground process group of the
controlling terminal to that of the child.
this would indeed be the right way if we wanted to isolate the children
more, but ...
It is better for handling SIGINT and SIGQUIT when we don't want git to
be killed but in complicates the handling of SIGTSTP and friends. [...]
... this shows that we really don't want that; we don't want to
replicate interactive shell behavior. that is even before the divergence
on windows.
Yeah, I'm not enthusiastic about emulating the shell's job control in git.
so i think your patch is approaching things the right way.
though blocking signals doesn't appear right - to ensure git's own clean
exit while it has no children, it must catch sigint anyway, and
temporarily ignoring it around spawning children sounds racy.
There is an inevitable race between wait() returning and calling
signal() to restore the handlers for SIGINT and SIGQUIT, it is such a
small window I'm not sure it is a problem in practice. There is also a
race when creating the child but if we block signals before calling
fork, then ignore SIGINT and SIGQUIT in the parent before unblocking
them we're OK because the child will be killed as soon as it unblocks
signal by any signal received while the signals were blocks and we'll
detect that in the parent and exit. Currently editor.c just ignores the
signals after fork has returned in the parent which means it is
theoretically possible to kill git with SIGINT while the child is running.
Best Wishes
Phillip
