Interactive: Avoid losing the job after suspending a subshell

Reproducer: run vi in a subshell:

	$ (vi)

vi opens; now press Ctrl+Z to suspend. The output is as expected:

	[2] + Stopped                  (vi)

…but the exit status is 18 (SIGTSTP's signal number) instead of 0.

Now do:

	$ fg
	(vi)
	$

The exit status is 18 again, vi is not resumed, and the job is
lost. You have to find vi's pid manually using ps and kill it.

Forking all non-command substitution subshells invoked from the
interactive main shell is the only reliable and effective fix I've
found. I've tried to fork the subshell conditionally in every other
remotely plausible place I can think of in fault.c and xec.c, but I
can't get anything to work properly. If anyone can get this to work
without forking as much (or at all), please do submit a patch or PR
that supersedes this fix.

At least subshells of subshells don't need to fork, so the
performance impact can be limited. Plus, it's not as if most people
need maximum speed on the interactive command line. Scripts
(including login/profile scripts) are not affected at all.

Command substitutions can be handled differently. My testing shows
that all shells except ksh93 simply block SIGTSTP (the ^Z signal)
while they run. We should do the same, so they don't need to fork.

NOTE for any backporters: the subshell.c and fault.c changes depend
on commits 35b0262 and 48ba696 to work correctly.

src/cmd/ksh93/sh/subshell.c: sh_subshell():
- If the interactive shell state bit is on, then before executing
  the subshell's code:
  - for command substitutions, block SIGTSTP;
  - for other subshells, fork.
- For command substitutions, release SIGTSTP if the interactive
  shell state bit was on upon invoking the subshell.

src/cmd/ksh93/sh/fault.c:
- Instead of checking for a virtual subshell, check the shell's
  interactive state bit to decide whether to handle SIGTSTP, as
  that is only turned on in the interactive main shell.

src/cmd/ksh93/sh/main.c: sh_main():
- To avoid bugs, ignore SIGTSTP while running profile scripts.
  Blocking it doesn't work because delaying it until after
  sigrelease() will cause a crash. Thanks to @JohnoKing for this.
- While we're here, prevent a possible overflow of the 'beenhere'
  static char variable by only incrementing it once.

Co-authored-by: Johnothan King <[email protected]>
Resolves: #390

NEWS

@@ -12,6 +12,15 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 	[1]	30909      <--- incorrect job control output from subshell
 	done
 
+- Fixed: after suspending (Ctrl+Z) a subshell that is running an external
+  command, resuming the subshell with 'fg' failed and the job was lost.
+	$ (vi)    <--- press Ctrl+Z
+	[2] + Stopped                  (vi)
+	$ fg
+	(vi)      <--- vi failed to resume; immediate return to command line
+	$ fg
+	ksh: no such job
+
 2021-12-20:
 
 - Ported performance optimizations from illumos to improve the performance

src/cmd/ksh93/sh/fault.c

@@ -547,7 +547,7 @@ void sh_exit(register int xno)
 		sh_offstate(SH_STOPOK);
 		shp->trapnote = 0;
 		shp->forked = 1;
-		if(!shp->subshell && (sig=sh_fork(shp,0,NIL(int*))))
+		if(sh_isstate(SH_INTERACTIVE) && (sig=sh_fork(shp,0,NIL(int*))))
 		{
 			job.curpgid = 0;
 			job.parent = (pid_t)-1;
@@ -564,7 +564,7 @@ void sh_exit(register int xno)
 		{
 			if(shp->subshell)
 				sh_subfork();
-			/* child process, put to sleep */
+			/* script or child process; put to sleep */
 			sh_offstate(SH_STOPOK);
 			sh_offstate(SH_MONITOR);
 			shp->sigflag[SIGTSTP] = 0;

src/cmd/ksh93/sh/main.c

@@ -160,9 +160,11 @@ int sh_main(int ac, char *av[], Shinit_f userinit)
 	else
 		sh_onoption(SH_BRACEEXPAND);
 #endif
-	if((beenhere++)==0)
+	if(!beenhere)
 	{
+		beenhere++;
 		sh_onstate(SH_PROFILE);
+		shp->sigflag[SIGTSTP] |= SH_SIGIGNORE;
 		if(shp->gd->ppid==1)
 			shp->login_sh++;
 		if(shp->login_sh >= 2)
@@ -236,6 +238,7 @@ int sh_main(int ac, char *av[], Shinit_f userinit)
 				sh_source(shp, iop, e_suidprofile);
 		}
 		shp->st.cmdname = error_info.id = command;
+		shp->sigflag[SIGTSTP] &= ~(SH_SIGIGNORE);
 		sh_offstate(SH_PROFILE);
 		if(rshflag)
 			sh_onoption(SH_RESTRICTED);

src/cmd/ksh93/sh/subshell.c

@@ -660,6 +660,16 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 					sh_subfork();
 			}
 #endif /* _lib_fchdir */
+#ifdef SIGTSTP
+			/* Virtual subshells are not safe to suspend (^Z, SIGTSTP) in the interactive main shell. */
+			if(sh_isstate(SH_INTERACTIVE))
+			{
+				if(comsub)
+					sigblock(SIGTSTP);
+				else if(!sh_isstate(SH_PROFILE))
+					sh_subfork();
+			}
+#endif
 			sh_offstate(SH_INTERACTIVE);
 			sh_exec(t,flags);
 		}
@@ -686,6 +696,10 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 	nv_restore(sp);
 	if(comsub)
 	{
+#ifdef SIGTSTP
+		if(savst.states & sh_state(SH_INTERACTIVE))
+			sigrelease(SIGTSTP);
+#endif
 		/* re-enable job control */
 		if(!sp->nofork)
 			sh_offstate(SH_NOFORK);