[PATCH] The thing about ":|: =(hang)" (Re: [PATCH] Fix users/26150 from Etc/BUGS)

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Wed, Apr 27, 2022 at 10:24 PM Bart Schaefer
<schaefer@xxxxxxxxxxxxxxxx> wrote:
>
> The fix for
>  :|: =(nosuchcommand)
> is probably going to be similar.

As you might expect, this is a race condition.

In getoutputfile() we have:

    os = jobtab[thisjob].stat;
    waitforpid(pid, 0);
    cmdoutval = 0;
    jobtab[thisjob].stat = os;

In ":|: =(hang)", waitforpid() is looking for the PID of "=(hang)",
but both the left side of the pipe and the command substitution exit
during that wait.  This means all jobs of the right-hand-side are done
(because the right-hand side is in the current shell) so
jobtab[thisjob] should be marked STAT_DONE, but that status is
clobbered by restoring the saved state after waitforpid().

Saving and restoring the job state here dates from before we used git,
but doesn't appear to be needed any longer -- all tests still pass
with it removed.  Any ideas how to prove this wrong?

diff --git a/Src/exec.c b/Src/exec.c
index c31089bb5..f2911807c 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -4883,13 +4883,9 @@ getoutputfile(char *cmd, char **eptr)
        child_unblock();
        return nam;
     } else if (pid) {
-       int os;
-
        close(fd);
-       os = jobtab[thisjob].stat;
        waitforpid(pid, 0);
        cmdoutval = 0;
-       jobtab[thisjob].stat = os;
        return nam;
     }