Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

> On 28 March 2022 at 16:09 Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> On Mon, Mar 28, 2022 at 7:49 AM Peter Stephenson
> <p.w.stephenson@xxxxxxxxxxxx> wrote:
> >
> > Just thinking the pukka way to do this is actually zero the PN fields
> > in the saved job structure when we enter the subshell (unless we decide
> > to preserve them properly, which is more work but should be possible).
> 
> Looking at the way zargs was using subshells and $jobstates, I think
> it's important that there be some way to distinguish jobs that the
> subshell can't manage.
> 
> % % sleep 5 & ( sleep 2 & print -aC2 ${(kv)jobstates} )
> [1] 68859
> 1  running:+
> 3  running:

So this is a different issue, not directly related to the immediate
cause of the crash though under the same general heading.

Here we're overloading the meaning of "jobs" so instead of meaning
"a real live job started with job control", which only applies to
the top level shell, we're using it to mean "something we just
started at the current process level which may or may not be
a job in the Unix sense but which we can do processy stuff with".
I don't think we can back out of doing the former with job states,
that's the one people notice --- "jobs | where_my_job_gone" being
the classic case, but if we're going to handle that we should be
consistent.  But the other case is a perfectly valid one.

We have two options here: either keep jobstates for top level jobs
and introduce a separate one for the last thing started in the current
shell, or decide on the fly that we're no longer interested in the
parent shell.  In the former case we're going to end up with some
combination of inconsistency and incompatibility however we do it.
We can make the latter work something like the following by noticing
we've just put something into the background.  That's about as far as
I think we can get relying on shell magic, though we should probably
document it if we do it.

This includes the previous fix (in the original simple form) with
the test fix.

pws

diff --git a/Doc/Zsh/mod_parameter.yo b/Doc/Zsh/mod_parameter.yo
index 2e3011e44..28c19f797 100644
--- a/Doc/Zsh/mod_parameter.yo
+++ b/Doc/Zsh/mod_parameter.yo
@@ -189,6 +189,10 @@ the var(state) describes the state of that process.
 
 Handling of the keys of the associative array is as described for
 tt(jobdirs) above.
+
+This parameter is available in a subshell, but only shows overall job
+information, not process information.  The information is current at the
+point the subshell forks from the parent shell.
 )
 vindex(nameddirs)
 item(tt(nameddirs))(
diff --git a/Src/Modules/parameter.c b/Src/Modules/parameter.c
index dbb61e474..2692766eb 100644
--- a/Src/Modules/parameter.c
+++ b/Src/Modules/parameter.c
@@ -1348,6 +1348,15 @@ pmjobstate(Job jtab, int job)
     else
 	ret = dyncat("running", cp);
 
+    if (jtab != jobtab)
+    {
+	/*
+	 * This is a saved job table so doesn't have current
+	 * process information --- bail out now.
+	 */
+	return ret;
+    }
+
     for (pn = jtab[job].procs; pn; pn = pn->next) {
 
 	if (pn->status == SP_RUNNING)
diff --git a/Src/exec.c b/Src/exec.c
index f67074846..19463e7c6 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1689,6 +1689,7 @@ execpline(Estate state, wordcode slcode, int how, int last1)
     execpline2(state, code, how, opipe[0], ipipe[1], last1);
     pline_level--;
     if (how & Z_ASYNC) {
+	clearoldjobtab();
 	lastwj = newjob;
 
         if (thisjob == list_pipe_job)
diff --git a/Src/jobs.c b/Src/jobs.c
index 18e43f03c..ef097dde8 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -1733,6 +1733,18 @@ clearjobtab(int monitor)
     thisjob = initjob();
 }
 
+/* In a subshell, decide we want our own job table after all. */
+
+/**/
+mod_export void
+clearoldjobtab(void)
+{
+    if (oldjobtab)
+	free(oldjobtab);
+    oldjobtab = NULL;
+    oldmaxjob = 0;
+}
+
 static int initnewjob(int i)
 {
     jobtab[i].stat = STAT_INUSE;
@@ -2449,6 +2461,7 @@ bin_fg(char *name, char **argv, Options ops, int func)
 	case BIN_BG:
 	case BIN_WAIT:
 	    if (func == BIN_BG) {
+		clearoldjobtab();
 		jobtab[job].stat |= STAT_NOSTTY;
 		jobtab[job].stat &= ~STAT_CURSH;
 	    }
diff --git a/Test/W03jobparameters.ztst b/Test/W03jobparameters.ztst
index af889c6d5..21de47395 100644
--- a/Test/W03jobparameters.ztst
+++ b/Test/W03jobparameters.ztst
@@ -46,5 +46,5 @@
 0:$jobstate for running job in main shell and subshell
 *>\[1] [0-9]##
 *>running:+:*=running
-*>running:+:*=running
+*>running:+
 *>zsh:*SIGHUPed*