[PATCH] Fix users/26150 from Etc/BUGS

[PATCH] Fix users/26150 from Etc/BUGS

[Bart Schaefer]

[-- Attachment #1: Type: text/plain, Size: 112 bytes --]

This fixes multios when used with nullexec.

The fix for
 :|: =(nosuchcommand)
is probably going to be similar.

[-- Attachment #2: u26150.txt --]
[-- Type: text/plain, Size: 1200 bytes --]

diff --git a/Etc/BUGS b/Etc/BUGS
index 3121fc9fa..b511ec6d6 100644
--- a/Etc/BUGS
+++ b/Etc/BUGS
@@ -46,12 +46,6 @@ interactive and the subshell is the foreground job.  The USEZLE option is
 always turned off in subshells, for reasons lost to history.  There is a
 related, probably obsolete, vared special case for $TERM set to "emacs".
 ------------------------------------------------------------------------
-users/26150: MULTIOS does not work with "exec":
-
-exec 3>/tmp/test1 3>/tmp/test2
-
-causes a script to hang.
-------------------------------------------------------------------------
 47561: [PATCH v4] vcs_info: choose backend by basedir
 ------------------------------------------------------------------------
 39319: () { exit } =(:) doesn't clean up the tempfile
diff --git a/Src/exec.c b/Src/exec.c
index 47753da48..c31089bb5 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3898,6 +3898,10 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 	    for (i = 0; i < 10; i++)
 		if (save[i] != -2)
 		    zclose(save[i]);
+	    /*
+	     * We're done with this job, no need to wait for it.
+	     */
+	    jobtab[thisjob].stat |= STAT_DONE;
 	    goto done;
 	}
 	if (isset(XTRACE)) {

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

[Bart Schaefer]

On Wed, Apr 27, 2022 at 10:24 PM Bart Schaefer
<schaefer@brasslantern.com> 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;
     }