Fix incorrect exec optimisation with monitor/pipefail on

Reproducer script:
    tempfile=/tmp/out2.$$.$RANDOM
    bintrue=$(whence -p true)
    for opt in monitor pipefail
    do
            (
                    set +x -o "$opt"
                    (
                            sleep .05
                            echo "ok $opt" >&2
                    ) 2>$tempfile | "$bintrue"
            ) &
            wait
            cat "$tempfile"
            rm -f "$tempfile"
    done

Expected output:
    ok monitor
    ok pipefail

Actual output:
    (none)

The 'monitor' and 'pipefail' options are supposed to make the shell
wait for the all commands in the pipeline to terminate and not only
the last component, regardless of whether the pipe between the
component commands is still open. In the failing reproducer, the
dummy external true command is subject to an exec optimization, so
it replaces the subshell instead of forking a new process. This is
incorrect, as the shell is no longer around to wait for the
left-hand part of the pipeline, so it continues in the background
without being waited for. Since it writes to standard error after
.05 seconds (after the pipe is closed), the 'cat' command reliably
finds the temp file empty. Without the sleep this would be a race
condition with unpredictable results.

Interestingly, this bug is only triggered for a (background
subshell)& and not for a forked (regular subshell). Which means the
exec optimization is not done for a forked regular subshell, though
there is no reason not to. That will be fixed in the next commit.

src/cmd/ksh93/sh/xec.c: sh_exec():
- case TFORK: Never allow an exec optimization if we're running a
  command in a multi-command pipeline (pipejob is set) and the
  shell needs to wait for all pipeline commands, i.e.: either the
  time keyword is in use, the SH_MONITOR state is active, or the
  SH_PIPEFAIL option is on.
- case TFIL: Fix the logic for setting job.waitall for the
  non-SH_PIPEFAIL case. Do not 'or' in the boolean value but assign
  it, and include the SH_TIMING (time keyword in use) state too.
- case TTIME: After that fix in case TFIL, we don't need to bother
  setting job.waitall explicitly here.

src/cmd/ksh93/sh.1:
- Add missing documentation for the conditions where the shell
  waits for all pipeline components (time, -o monitor/pipefail).

Resolves: https://github.com/ksh93/ksh/issues/449

NEWS

@@ -3,6 +3,13 @@ For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0
 
 Any uppercase BUG_* names are modernish shell bug IDs.
 
+2022-06-18:
+
+- Fixed a bug where, with the monitor or pipefail option on, the shell
+  failed to wait for all component commands in a pipeline to terminate if
+  the last component command was an external command and the pipeline was
+  the last command in a background subshell.
+
 2022-06-15:
 
 - Fixed a bug where converting an indexed array into an associative array in

src/cmd/ksh93/include/version.h

@@ -23,7 +23,7 @@
 
 #define SH_RELEASE_FORK	"93u+m"		/* only change if you develop a new ksh93 fork */
 #define SH_RELEASE_SVER	"1.0.0-beta.2"	/* semantic version number: https://semver.org */
-#define SH_RELEASE_DATE	"2022-06-15"	/* must be in this format for $((.sh.version)) */
+#define SH_RELEASE_DATE	"2022-06-18"	/* must be in this format for $((.sh.version)) */
 #define SH_RELEASE_CPYR	"(c) 2020-2022 Contributors to ksh " SH_RELEASE_FORK
 
 /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */

src/cmd/ksh93/sh.1

@@ -179,7 +179,7 @@ built-in utility).
 A
 .I pipeline\^
 is a sequence of one or more
-.I commands\^
+.IR command\^ s
 separated by
 .BR | .
 The standard output of each command but the last
@@ -194,10 +194,19 @@ by a
 to the standard input of the next command.
 Each command,
 except possibly the last,
-is run as a separate process;
-the shell waits for the last command to terminate.
+is run as a separate process.
+If the
+.B monitor
+or
+.B pipefail
+option is on,
+or the pipeline is preceded by the
+.I "reserved word"
+.BR time ,
+then the shell waits for all component commands in the pipeline to terminate;
+otherwise, the shell only waits for the last component command.
 The exit status of a pipeline is the exit
-status of the last command unless the
+status of its last component command, unless the
 .B pipefail
 option is enabled.
 Each pipeline can be preceded by the
@@ -7524,6 +7533,8 @@ not just those that precede the command name.
 Background jobs will run in a separate process group
 and a line will print upon completion.
 The exit status of background jobs is reported in a completion message.
+A pipeline will not terminate until all component commands of the pipeline
+have terminated.
 On systems with job control,
 this option is turned on automatically for
 interactive shells.
@@ -7684,10 +7695,11 @@ Same as
 .BR \-u .
 .TP 8
 .B pipefail
-A pipeline will not complete until all components
-of the pipeline have completed, and the return value
-will be the value of the last non-zero command
-to fail or zero if no command has failed.
+The exit status of the entire pipeline will be that of the last component
+command that exited with a non-zero exit status, or zero if no command
+exited with a non-zero exit status.
+The shell will wait for all component commands of the pipeline to terminate,
+instead of only waiting for the last component command.
 .TP 8
 .B posix
 Enables the POSIX standard mode for maximum compatibility with other compliant

src/cmd/ksh93/sh/xec.c

@@ -1506,7 +1506,8 @@ int sh_exec(register const Shnode_t *t, int flags)
 			&& !sh.st.trapcom[0]
 			&& !sh.st.trap[SH_ERRTRAP]
 			&& ((struct checkpt*)sh.jmplist)->mode!=SH_JMPEVAL
-			&& (execflg2 || (execflg && sh.fn_depth==0 && !(pipejob && sh_isoption(SH_PIPEFAIL))));
+			&& (execflg && sh.fn_depth==0 || execflg2)
+			&& !(pipejob && (sh_isstate(SH_MONITOR) || sh_isoption(SH_PIPEFAIL) || sh_isstate(SH_TIMING)));
 			if(no_fork)
 				job.parent=parent=0;
 			else
@@ -1892,6 +1893,10 @@ int sh_exec(register const Shnode_t *t, int flags)
 			sh.inpipe = pvo;
 			sh.outpipe = pvn;
 			pvo[1] = -1;
+			/*
+			 * If the pipefail or monitor options are on or if the time keyword is in use, then wait
+			 * for all commands in the pipeline to complete; otherwise, wait for the last one only
+			 */
 			if(sh_isoption(SH_PIPEFAIL))
 			{
 				const Shnode_t* tn=t;
@@ -1903,7 +1908,7 @@ int sh_exec(register const Shnode_t *t, int flags)
 				memset(exitval,0,job.waitall*sizeof(int));
 			}
 			else
-				job.waitall |= (!pipejob && sh_isstate(SH_MONITOR));
+				job.waitall = !pipejob && (sh_isstate(SH_MONITOR) || sh_isstate(SH_TIMING));
 			job_lock();
 			nlock++;
 			do
@@ -2323,7 +2328,7 @@ int sh_exec(register const Shnode_t *t, int flags)
 			}
 			if(t->par.partre)
 			{
-				long timer_on = sh_isstate(SH_TIMING);
+				int timer_on = sh_isstate(SH_TIMING);
 #ifdef timeofday
 				/* must be run after forking a subshell */
 				timeofday(&tb);
@@ -2336,12 +2341,10 @@ int sh_exec(register const Shnode_t *t, int flags)
 					UNREACHABLE();
 				}
 #endif
-				job.waitall = 1;
 				sh_onstate(SH_TIMING);
 				sh_exec(t->par.partre,sh_isstate(SH_ERREXIT)|OPTIMIZE);
 				if(!timer_on)
 					sh_offstate(SH_TIMING);
-				job.waitall = 0;
 			}
 			else
 			{

src/cmd/ksh93/tests/options.sh

@@ -605,5 +605,25 @@ EOF
 	exit
 EOF
 
+# ======
+# https://github.com/ksh93/ksh/issues/449
+set +o monitor +o pipefail
+for opt in monitor pipefail
+do
+	outfile=out$opt
+	exp="ok $opt"
+	(
+		set +x -o "$opt"
+		(
+			sleep .01
+			print "$exp" >&2
+		) 2>$outfile | "${ whence -p true; }"  # the external 'true' should not be exec-optimized
+	) &
+	wait "$!"
+	got=$(<$outfile)
+	[[ $got == "$exp" ]] || err_exit "shell did not wait for entire pipeline with -o $opt" \
+		"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
+done
+
 # ======
 exit $((Errors<125?Errors:125))