Re: interrupt handling bug (again?)

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

Backgrounded shell code with subproceses:

On Sat, 24 Jun 2017 12:03:10 -0700
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> If you
> instead disown before the foreground job is done, the job table entry
> for the foreground job is deleted, so the parent never notices that
> it has exited and never resumes the loop at all.

I agree with this.

However, I think we can do slightly better by delaying the disown to the
point where we send SIGCONT to the superjob (the background copy of the
shell process) after finding it has no associated processes in the
current shell. At that point it will start its own processes, so we
never need to interact with it again.

This seems to work, and I've documented the effect.  Is it weird enough
that we should print a warning from "disown", or is that just going to
cause more confusion than it removes (warnings of a technical nature
often do, in my experience)?  I reckon simply pointing people at
the documentation if they ask is probably good enough.

BTW, I think I don't need to check if there are actually processes
associated with the superjob, since if there aren't (in which case it's
marked was WASSUPER instead of SUPERJOB) disowning immediately has no
bad effects.

pws

diff --git a/Doc/Zsh/jobs.yo b/Doc/Zsh/jobs.yo
index 6262dd2..44e0a44 100644
--- a/Doc/Zsh/jobs.yo
+++ b/Doc/Zsh/jobs.yo
@@ -49,6 +49,12 @@ in the parent shell.  Thus the behaviour is different from the case
 where the function was never suspended.  Zsh is different from many
 other shells in this regard.
 
+One additional side effect is that use of tt(disown) with a job
+created by suspending shell code in this fashion is delayed: the
+job can only be disowned once any process started from the parent
+shell has terminated.  At that point, the job disappears silently
+from the process list.
+
 The same behaviour is found when the shell is executing code as the
 right hand side of a pipeline or any complex shell construct such as
 tt(if), tt(for), etc., in order that the entire block of code
diff --git a/Src/jobs.c b/Src/jobs.c
index 32f7daa..66dfb5a 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -277,6 +277,10 @@ handle_sub(int job, int fg)
 		(!jn->procs->next || cp || jn->procs->pid != jn->gleader))
 		attachtty(jn->gleader);
 	    kill(sj->other, SIGCONT);
+	    if (jn->stat & STAT_DISOWN)
+	    {
+		deletejob(jn, 1);
+	    }
 	}
 	curjob = jn - jobtab;
     } else if (sj->stat & STAT_STOPPED) {
@@ -2375,6 +2379,10 @@ bin_fg(char *name, char **argv, Options ops, int func)
 	    printjob(job + (oldjobtab ? oldjobtab : jobtab), lng, 2);
 	    break;
 	case BIN_DISOWN:
+	    if (jobtab[job].stat & STAT_SUPERJOB) {
+		jobtab[job].stat |= STAT_DISOWN;
+		continue;
+	    }
 	    if (jobtab[job].stat & STAT_STOPPED) {
 		char buf[20], *pids = "";
 
diff --git a/Src/zsh.h b/Src/zsh.h
index 137b2a5..a5b4d8f 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1029,6 +1029,7 @@ struct job {
 #define STAT_BUILTIN    (0x4000) /* job at tail of pipeline is a builtin */
 #define STAT_SUBJOB_ORPHANED (0x8000)
                                  /* STAT_SUBJOB with STAT_SUPERJOB exited */
+#define STAT_DISOWN     (0x10000) /* STAT_SUPERJOB with disown pending */
 
 #define SP_RUNNING -1		/* fake status for jobs currently running */