unposted (branch fork_early): Fork early for pipelines.

If executing a command not at the end of a pipeline, and
not optimised in the calling code, we can fork before
"prefork" substitutions, at the same point as
for background commands.

Move fork before our preliminary scan of arguments to find
a command: in the cases of early fork
we don't need this information before forking.

Ensure we _exit if forked in execcmd_exec().

Rationalise use of forks and pipes.

Ensure we _exit instead of returning from execcmd_exec() if we
have forked.  Before the optimisation code after the fork always ran
to the check at the end, but that code is overkill for the logic
between the early fork and the existing one.

Remove old workaround to fork in caller of execcmd for current shell
constructs as no longer needed with early fork below.

Close input of newly created pipe on fork (destined for RHS of pipe
which we never execute): this replaces a workaround from
zsh-workers/32171, commit 9887fc3d7b.

Set last1 on early fork as needed by some instances of shell
constructs on LHS of pipeline to know they are exiting.

1 files changed, 63 insertions, 82 deletions

diff --git a/Src/exec.c b/Src/exec.c
index 64cf5ae46..08c4ea95a 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1878,8 +1878,6 @@ static void
 execpline2(Estate state, wordcode pcode,
 	   int how, int input, int output, int last1)
 {
-    pid_t pid;
-    int pipes[2];
     struct execcmd_params eparams;
 
     if (breaks || retflag)
@@ -1900,65 +1898,21 @@ execpline2(Estate state, wordcode pcode,
     }
     if (WC_PIPE_TYPE(pcode) == WC_PIPE_END) {
 	execcmd_analyse(state, &eparams);
-	execcmd_exec(state, &eparams, input, output, how, last1 ? 1 : 2);
+	execcmd_exec(state, &eparams, input, output, how, last1 ? 1 : 2, -1);
     } else {
+	int pipes[2];
 	int old_list_pipe = list_pipe;
-	int subsh_close = -1;
-	Wordcode next = state->pc + (*state->pc), start_pc;
+	Wordcode next = state->pc + (*state->pc);
 
-	start_pc = ++state->pc;
+	++state->pc;
 	execcmd_analyse(state, &eparams);
 
 	if (mpipe(pipes) < 0) {
 	    /* FIXME */
 	}
 
-	/* if we are doing "foo | bar" where foo is a current *
-	 * shell command, do foo in a subshell and do the     *
-	 * rest of the pipeline in the current shell.         */
-	if ((eparams.type >= WC_CURSH || !eparams.args)
-	    && (how & Z_SYNC)) {
-	    int synch[2];
-	    struct timeval bgtime;
-
-	    if (pipe(synch) < 0) {
-		zerr("pipe failed: %e", errno);
-		lastval = 1;
-		errflag |= ERRFLAG_ERROR;
-		return;
-	    } else if ((pid = zfork(&bgtime)) == -1) {
-		close(synch[0]);
-		close(synch[1]);
-		lastval = 1;
-		errflag |= ERRFLAG_ERROR;
-		return;
-	    } else if (pid) {
-		char dummy, *text;
-
-		text = getjobtext(state->prog, start_pc);
-		addproc(pid, text, 0, &bgtime);
-		close(synch[1]);
-		read_loop(synch[0], &dummy, 1);
-		close(synch[0]);
-	    } else {
-		zclose(pipes[0]);
-		close(synch[0]);
-		entersubsh(((how & Z_ASYNC) ? ESUB_ASYNC : 0)
-			   | ESUB_PGRP | ESUB_KEEPTRAP);
-		close(synch[1]);
-		if (sigtrapped[SIGEXIT])
-		{
-		    unsettrap(SIGEXIT);
-		}
-		execcmd_exec(state, &eparams, input, pipes[1], how, 1);
-		_exit(lastval);
-	    }
-	} else {
-	    /* otherwise just do the pipeline normally. */
-	    addfilelist(NULL, pipes[0]);
-	    subsh_close = pipes[0];
-	    execcmd_exec(state, &eparams, input, pipes[1], how, 0);
-	}
+	addfilelist(NULL, pipes[0]);
+	execcmd_exec(state, &eparams, input, pipes[1], how, 0, pipes[0]);
 	zclose(pipes[1]);
 	state->pc = next;
 
@@ -1969,8 +1923,6 @@ execpline2(Estate state, wordcode pcode,
 	execpline2(state, *state->pc++, how, pipes[0], output, last1);
 	list_pipe = old_list_pipe;
 	cmdpop();
-	if (subsh_close != pipes[0])
-	    zclose(pipes[0]);
     }
 }
 
@@ -2710,7 +2662,8 @@ static void execcmd_getargs(LinkList preargs, LinkList args, int expand)
 /**/
 static int
 execcmd_fork(Estate state, int how, int type, Wordcode varspc,
-	     LinkList *filelistp, char *text, int oautocont)
+	     LinkList *filelistp, char *text, int oautocont,
+	     int close_if_forked)
 {
     pid_t pid;
     int synch[2], flags;
@@ -2766,6 +2719,7 @@ execcmd_fork(Estate state, int how, int type, Wordcode varspc,
     *filelistp = jobtab[thisjob].filelist;
     entersubsh(flags);
     close(synch[1]);
+    zclose(close_if_forked);
 
     if (sigtrapped[SIGINT] & ZSIG_IGNORED)
 	holdintr();
@@ -2786,7 +2740,7 @@ execcmd_fork(Estate state, int how, int type, Wordcode varspc,
 /**/
 static void
 execcmd_exec(Estate state, Execcmd_params eparams,
-	     int input, int output, int how, int last1)
+	     int input, int output, int how, int last1, int close_if_forked)
 {
     HashNode hn = NULL;
     LinkList filelist = NULL;
@@ -2863,6 +2817,29 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 	    pushnode(args, dupstring("fg"));
     }
 
+    if ((how & Z_ASYNC) || output) {
+	/*
+	 * If running in the background, or not the last command in a
+	 * pipeline, we don't need any of the rest of this function to
+	 * affect the state in the main shell, so fork immediately.
+	 *
+	 * In other cases we may need to process the command line
+	 * a bit further before we make the decision.
+	 */
+	text = getjobtext(state->prog, eparams->beg);
+	switch (execcmd_fork(state, how, type, varspc, &filelist,
+			     text, oautocont, close_if_forked)) {
+	case -1:
+	    goto fatal;
+	case 0:
+	    break;
+	default:
+	    return;
+	}
+	last1 = forked = 1;
+    } else
+	text = NULL;
+
     /* Check if it's a builtin needing automatic MAGIC_EQUALS_SUBST      *
      * handling.  Things like typeset need this.  We can't detect the    *
      * command if it contains some tokens (e.g. x=ex; ${x}port), so this *
@@ -2871,7 +2848,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
     if ((type == WC_SIMPLE || type == WC_TYPESET) && args) {
 	/*
 	 * preargs contains args that have been expanded by prefork.
-	 * Running execcmd_getargs() causes the any argument available
+	 * Running execcmd_getargs() causes any argument available
 	 * in args to be exanded where necessary and transferred to
 	 * preargs.  We call execcmd_getargs() every time we need to
 	 * analyse an argument not available in preargs, though there is
@@ -2918,8 +2895,11 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 		is_builtin = 1;
 
 		/* autoload the builtin if necessary */
-		if (!(hn = resolvebuiltin(cmdarg, hn)))
+		if (!(hn = resolvebuiltin(cmdarg, hn))) {
+		    if (forked)
+			_exit(lastval);
 		    return;
+		}
 		if (type != WC_TYPESET)
 		    magic_assign = (hn->flags & BINF_MAGICEQUALS);
 		break;
@@ -3097,6 +3077,8 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 			    zerr("unknown exec flag -%c", *cmdopt);
 			    lastval = 1;
 			    errflag |= ERRFLAG_ERROR;
+			    if (forked)
+				_exit(lastval);
 			    return;
 			}
 		    }
@@ -3146,28 +3128,6 @@ execcmd_exec(Estate state, Execcmd_params eparams,
     esprefork = (magic_assign ||
 		 (isset(MAGICEQUALSUBST) && type != WC_TYPESET)) ?
 		 PREFORK_TYPESET : 0;
-    if (how & Z_ASYNC) {
-	/*
-	 * If running in the background, we don't need any of
-	 * the rest of this functino to affect the state in the
-	 * main shell, so fork immediately.
-	 *
-	 * In other cases we may need to process the command line
-	 * a bit further before we make the decision.
-	 */
-	text = getjobtext(state->prog, eparams->beg);
-	switch (execcmd_fork(state, how, type, varspc, &filelist,
-			     text, oautocont)) {
-	case -1:
-	    goto fatal;
-	case 0:
-	    break;
-	default:
-	    return;
-	}
-	forked = 1;
-    } else
-	text = NULL;
 
     if (args) {
 	if (eparams->htok)
@@ -3212,6 +3172,8 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 			zerr("redirection with no command");
 			lastval = 1;
 			errflag |= ERRFLAG_ERROR;
+			if (forked)
+			    _exit(lastval);
 			return;
 		    } else if (!nullcmd || !*nullcmd || opts[SHNULLCMD]) {
 			if (!args)
@@ -3230,6 +3192,8 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 		    }
 		} else if ((cflags & BINF_PREFIX) && (cflags & BINF_COMMAND)) {
 		    lastval = 0;
+		    if (forked)
+			_exit(lastval);
 		    return;
 		} else {
 		    /*
@@ -3240,6 +3204,8 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 		    if (badcshglob == 1) {
 			zerr("no match");
 			lastval = 1;
+			if (forked)
+			    _exit(lastval);
 			return;
 		    }
 		    cmdoutval = use_cmdoutval ? lastval : 0;
@@ -3253,12 +3219,16 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 			fputc('\n', xtrerr);
 			fflush(xtrerr);
 		    }
+		    if (forked)
+			_exit(lastval);
 		    return;
 		}
 	    } else if (isset(RESTRICTED) && (cflags & BINF_EXEC) && do_exec) {
 		zerrnam("exec", "%s: restricted",
 			(char *) getdata(firstnode(args)));
 		lastval = 1;
+		if (forked)
+		    _exit(lastval);
 		return;
 	    }
 
@@ -3293,6 +3263,8 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 		    lastval = 1;
 		    if (oautocont >= 0)
 			opts[AUTOCONTINUE] = oautocont;
+		    if (forked)
+			_exit(lastval);
 		    return;
 		}
 		break;
@@ -3301,8 +3273,11 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 		is_builtin = 1;
 
 		/* autoload the builtin if necessary */
-		if (!(hn = resolvebuiltin(cmdarg, hn)))
+		if (!(hn = resolvebuiltin(cmdarg, hn))) {
+		    if (forked)
+			_exit(lastval);
 		    return;
+		}
 		break;
 	    }
 	    cflags &= ~BINF_BUILTIN & ~BINF_COMMAND;
@@ -3317,6 +3292,8 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 	    lastval = 1;
 	if (oautocont >= 0)
 	    opts[AUTOCONTINUE] = oautocont;
+	if (forked)
+	    _exit(lastval);
 	return;
     }
 
@@ -3394,6 +3371,8 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 		lastval = 1;
 		if (oautocont >= 0)
 		    opts[AUTOCONTINUE] = oautocont;
+		if (forked)
+		    _exit(lastval);
 		return;
 	    }
 	}
@@ -3422,6 +3401,8 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 	lastval = 1;
 	if (oautocont >= 0)
 	    opts[AUTOCONTINUE] = oautocont;
+	if (forked)
+	    _exit(lastval);
 	return;
     }
 
@@ -3497,7 +3478,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 	     (!is_cursh && (last1 != 1 || nsigtrapped || havefiles() ||
 			    fdtable_flocks)))) {
 	    switch (execcmd_fork(state, how, type, varspc, &filelist,
-				 text, oautocont)) {
+				 text, oautocont, close_if_forked)) {
 	    case -1:
 		goto fatal;
 	    case 0: