zsh-workers
 help / color / mirror / code / Atom feed
* Crash on jobstates since workers/49783 (commit 6a8aa2a)
@ 2022-03-25 22:32 Bart Schaefer
  2022-03-27  5:34 ` Bart Schaefer
  0 siblings, 1 reply; 15+ messages in thread
From: Bart Schaefer @ 2022-03-25 22:32 UTC (permalink / raw)
  To: Zsh hackers list

I was fooling around with "zargs" as follows:

run1 () {
    print -r $1 $sysparams[pid]
    sleep 2
}
runN () {
    zargs -n 1 -P $# -- "$@" -- run1
}
zargs -n 5 -- {1..20} -- runN

The nested calls to zargs from runN all die with a SEGV on when running:

wait ${${jobstates[(R)running:*]/#*:/}/%=*/}

Obviously in this case we're counting on the subshell NOT having
access to the parent shell's jobstates, it only wants to know about
its own jobs.  I think the pre-49783 was better.

Top of the stack:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x0000560869c32a10 in pmjobstate (jtab=0x56086ae54130, job=3)
    at parameter.c:1353
1353        if (pn->status == SP_RUNNING)
(gdb) where
#0  0x0000560869c32a10 in pmjobstate (jtab=0x56086ae54130, job=3)
    at parameter.c:1353
#1  0x0000560869c32f0d in scanpmjobstates (ht=0x56086aec9200,
    func=0x560869bd696d <scanparamvals>, flags=561) at parameter.c:1428
#2  0x0000560869ba4939 in scanmatchtable (ht=0x56086aec9200, pprog=0x0,
    sorted=0, flags1=0, flags2=16777216,
    scanfunc=0x560869bd696d <scanparamvals>, scanflags=561) at hashtable.c:386
#3  0x0000560869ba4c20 in scanhashtable (ht=0x56086aec9200, sorted=0,
    flags1=0, flags2=16777216, scanfunc=0x560869bd696d <scanparamvals>,
    scanflags=561) at hashtable.c:449
#4  0x0000560869bd6c89 in paramvalarr (ht=0x56086aec9200, flags=561)
    at params.c:662
#5  0x0000560869bd6d5a in getvaluearr (v=0x7ffe223f0750) at params.c:680
#6  0x0000560869bd945d in getarg (str=0x7ffe223f0258, inv=0x7ffe223f023c,
    v=0x7ffe223f0750, a2=0, w=0x7ffe223f0260, prevcharlen=0x7ffe223f0240,
    nextcharlen=0x7ffe223f0244, flags=0) at params.c:1592
#7  0x0000560869bda3bc in getindex (pptr=0x7ffe223f02f8, v=0x7ffe223f0750,
    flags=0) at params.c:1902
#8  0x0000560869bdab7e in fetchvalue (v=0x7ffe223f0750, pptr=0x7ffe223f04c8,
    bracks=1, flags=0) at params.c:2122
#9  0x0000560869c07b8b in paramsubst (l=0x7ffe223f0a70, n=0x7ffe223f0a90,
    str=0x7ffe223f0820, qt=0, pf_flags=64, ret_flags=0x7ffe223f0b14)
    at subst.c:2553
#10 0x0000560869c030c8 in stringsubst (list=0x7ffe223f0a70,
    node=0x7ffe223f0a90, pf_flags=64, ret_flags=0x7ffe223f0b14, asssub=0)
    at subst.c:322


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-25 22:32 Crash on jobstates since workers/49783 (commit 6a8aa2a) Bart Schaefer
@ 2022-03-27  5:34 ` Bart Schaefer
  2022-03-28  8:47   ` Peter Stephenson
  0 siblings, 1 reply; 15+ messages in thread
From: Bart Schaefer @ 2022-03-27  5:34 UTC (permalink / raw)
  To: Zsh hackers list

On Fri, Mar 25, 2022 at 3:32 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> The nested calls to zargs from runN all die with a SEGV on when running:
>
> wait ${${jobstates[(R)running:*]/#*:/}/%=*/}

Minimal reproducer:

% sleep 5 &
[1] 171363
% ( wait ${${jobstates[(R)running:*]/#*:/}/%=*/} )
zsh: segmentation fault (core dumped)  ( wait
${${jobstates[(R)running:*]/#*:/}/%=*/}; )
%


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-27  5:34 ` Bart Schaefer
@ 2022-03-28  8:47   ` Peter Stephenson
  2022-03-28 14:49     ` Peter Stephenson
  2022-03-28 15:13     ` Bart Schaefer
  0 siblings, 2 replies; 15+ messages in thread
From: Peter Stephenson @ 2022-03-28  8:47 UTC (permalink / raw)
  To: Bart Schaefer, Zsh hackers list

> On 27 March 2022 at 06:34 Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Fri, Mar 25, 2022 at 3:32 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > The nested calls to zargs from runN all die with a SEGV on when running:
> >
> > wait ${${jobstates[(R)running:*]/#*:/}/%=*/}
> 
> Minimal reproducer:
> 
> % sleep 5 &
> [1] 171363
> % ( wait ${${jobstates[(R)running:*]/#*:/}/%=*/} )
> zsh: segmentation fault (core dumped)  ( wait
> ${${jobstates[(R)running:*]/#*:/}/%=*/}; )
> %

$jobstates was still looking at the process structures which doesn't
work in the subshell.  Previously, it didn't even look at the 
main job state in a subshell.   Obviously, the subshell doesn't know
the up-to-date state anyway, and can't actually manipulate it.
So probably the best we can do is report the last we knew about the
job state and ignore the process structures.  This probably warrants
a bit of documentation.

The code now reports "wait: can't manipulate jobs in subshell",
which I believe is a correct statement of affairs.

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)


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-28  8:47   ` Peter Stephenson
@ 2022-03-28 14:49     ` Peter Stephenson
  2022-03-28 15:09       ` Bart Schaefer
  2022-03-28 15:13     ` Bart Schaefer
  1 sibling, 1 reply; 15+ messages in thread
From: Peter Stephenson @ 2022-03-28 14:49 UTC (permalink / raw)
  To: Zsh hackers list

> On 28 March 2022 at 09:47 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > On 27 March 2022 at 06:34 Bart Schaefer <schaefer@brasslantern.com> wrote:
> > On Fri, Mar 25, 2022 at 3:32 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> > >
> > > The nested calls to zargs from runN all die with a SEGV on when running:
> > >
> > > wait ${${jobstates[(R)running:*]/#*:/}/%=*/}
> > 
> > Minimal reproducer:
> > 
> > % sleep 5 &
> > [1] 171363
> > % ( wait ${${jobstates[(R)running:*]/#*:/}/%=*/} )
> > zsh: segmentation fault (core dumped)  ( wait
> > ${${jobstates[(R)running:*]/#*:/}/%=*/}; )
> > %
> 
> $jobstates was still looking at the process structures which doesn't
> work in the subshell.  Previously, it didn't even look at the 
> main job state in a subshell.   Obviously, the subshell doesn't know
> the up-to-date state anyway, and can't actually manipulate it.
> So probably the best we can do is report the last we knew about the
> job state and ignore the process structures.  This probably warrants
> a bit of documentation.

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).

There's also a one-line test change unless we do go the extra mile
with the process structures.

pws


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-28 14:49     ` Peter Stephenson
@ 2022-03-28 15:09       ` Bart Schaefer
  2022-03-28 16:14         ` Peter Stephenson
  0 siblings, 1 reply; 15+ messages in thread
From: Bart Schaefer @ 2022-03-28 15:09 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Mon, Mar 28, 2022 at 7:49 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> 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:

How is it helpful that this returned anything?  Previously the
subshell could get information about the jobs it started itself, but
now it gets nothing.  I'm not even sure what job 3 is, there, as it
only shows up in the table at all a small fraction of the time when I
try this (usually it's just job 1).


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-28  8:47   ` Peter Stephenson
  2022-03-28 14:49     ` Peter Stephenson
@ 2022-03-28 15:13     ` Bart Schaefer
  1 sibling, 0 replies; 15+ messages in thread
From: Bart Schaefer @ 2022-03-28 15:13 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Mon, Mar 28, 2022 at 1:47 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> The code now reports "wait: can't manipulate jobs in subshell",
> which I believe is a correct statement of affairs.

Well, no, it's not, or at least not entirely.

% ret27() { sleep 5; return 27 }
% ( ret27 & wait $!; print $? )
27

A subhell can spawn and wait for processes, it just can't move them
between background and foreground.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-28 15:09       ` Bart Schaefer
@ 2022-03-28 16:14         ` Peter Stephenson
  2022-03-28 16:44           ` Bart Schaefer
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Stephenson @ 2022-03-28 16:14 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

> On 28 March 2022 at 16:09 Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Mon, Mar 28, 2022 at 7:49 AM Peter Stephenson
> <p.w.stephenson@ntlworld.com> 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*


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-28 16:14         ` Peter Stephenson
@ 2022-03-28 16:44           ` Bart Schaefer
  2022-03-28 19:52             ` Peter Stephenson
  0 siblings, 1 reply; 15+ messages in thread
From: Bart Schaefer @ 2022-03-28 16:44 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Mon, Mar 28, 2022 at 9:14 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> 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.

It's not just "the last thing started" that's wanted, it's a list of
every process that was started.  So although it's a bit magical that
backgrounding a job clears the job table, that seems the better of
those two alternatives.

Another possibility that occurs to me is for the values in $jobstates to change.

     ... The keys are the job numbers and the values
     are strings of the form 'JOB-STATE:MARK:PID=STATE...'.  The
     JOB-STATE gives the state the whole job is currently in, one of
     'running', 'suspended', or 'done'.

What if JOB-STATE became "parent" in the subshell?  Or something along
those lines.  I guess that goes back to having to make a deep copy of
the saved job structure.

Incidentally:

% sleep 10 & print -aC2 ${(kv)jobstates}; (jobs -l; print -aC2 ${(kv)jobstates})
[1] 69151
1  running:+:69151=running
[1]  + 69151 running    sleep 10
1  running:+

How is it that "jobs -l" still knows the PID from the parent job, but
$jobstates cannot report it?


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-28 16:44           ` Bart Schaefer
@ 2022-03-28 19:52             ` Peter Stephenson
  2022-03-29  3:49               ` Bart Schaefer
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Stephenson @ 2022-03-28 19:52 UTC (permalink / raw)
  To: zsh-workers

On Mon, 2022-03-28 at 09:44 -0700, Bart Schaefer wrote:
> Another possibility that occurs to me is for the values in $jobstates to change.
> 
>      ... The keys are the job numbers and the values
>      are strings of the form 'JOB-STATE:MARK:PID=STATE...'.  The
>      JOB-STATE gives the state the whole job is currently in, one of
>      'running', 'suspended', or 'done'.
> 
> What if JOB-STATE became "parent" in the subshell?  Or something along
> those lines.  I guess that goes back to having to make a deep copy of
> the saved job structure.

I think if can get something that "just works" we'll save quite a lot of
aggravation for ourselves both as users and as zsh-workers.  If there's
nothing too obviously wrong with magically pusthing the rabbit back into
the hat again so it looks like a normal top hat, I'd prefer to try that.

> Incidentally:
> 
> % sleep 10 & print -aC2 ${(kv)jobstates}; (jobs -l; print -aC2 ${(kv)jobstates})
> [1] 69151
> 1  running:+:69151=running
> [1]  + 69151 running    sleep 10
> 1  running:+
> 
> How is it that "jobs -l" still knows the PID from the parent job, but
> $jobstates cannot report it?

Hmm... on my home machine the crash is a bit more sporadic than it was
on the NUC at work, so it's something to do with races rather than a
simple invalid pointer as I originally assumed.  When I simply print
jobstates and it does crash I'm seeing what looks like the process
number of the subshell process appear right before the crash (i.e. it's
typically one more than the sleep that was just started).  But I can't
see what's different here from jobs.  In both cases, when oldjobtab is
non-NULL, we're scanning from job 1 to the maximum for jobs with stat
and procs non-zero and no STAT_NOPRINT.  Also, signals should be queued
at this point.  So I'm stuck at the moment.

pws



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-28 19:52             ` Peter Stephenson
@ 2022-03-29  3:49               ` Bart Schaefer
  2022-03-29  9:08                 ` Peter Stephenson
  0 siblings, 1 reply; 15+ messages in thread
From: Bart Schaefer @ 2022-03-29  3:49 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Mon, Mar 28, 2022 at 12:53 PM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> When I simply print
> jobstates and it does crash I'm seeing what looks like the process
> number of the subshell process appear right before the crash (i.e. it's
> typically one more than the sleep that was just started).

This?

diff --git a/Src/jobs.c b/Src/jobs.c
index 18e43f03c..83ffdbde5 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -1718,6 +1718,12 @@ clearjobtab(int monitor)
        /* Don't report any job we're part of */
        if (thisjob != -1 && thisjob < oldmaxjob)
            memset(oldjobtab+thisjob, 0, sizeof(struct job));
+
+       /* oldmaxjob is now the size of the table, but outside
+        * this function, it's used as a job number, which must
+        * be the largest index available in the table.
+        */
+       --oldmaxjob;
     }


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-29  3:49               ` Bart Schaefer
@ 2022-03-29  9:08                 ` Peter Stephenson
  2022-03-29 13:31                   ` Peter Stephenson
  2022-03-29 15:36                   ` Bart Schaefer
  0 siblings, 2 replies; 15+ messages in thread
From: Peter Stephenson @ 2022-03-29  9:08 UTC (permalink / raw)
  To: Zsh hackers list

> On 29 March 2022 at 04:49 Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Mon, Mar 28, 2022 at 12:53 PM Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> >
> > When I simply print
> > jobstates and it does crash I'm seeing what looks like the process
> > number of the subshell process appear right before the crash (i.e. it's
> > typically one more than the sleep that was just started).
> 
> This?
> 
> diff --git a/Src/jobs.c b/Src/jobs.c
> index 18e43f03c..83ffdbde5 100644
> --- a/Src/jobs.c
> +++ b/Src/jobs.c
> @@ -1718,6 +1718,12 @@ clearjobtab(int monitor)
>         /* Don't report any job we're part of */
>         if (thisjob != -1 && thisjob < oldmaxjob)
>             memset(oldjobtab+thisjob, 0, sizeof(struct job));
> +
> +       /* oldmaxjob is now the size of the table, but outside
> +        * this function, it's used as a job number, which must
> +        * be the largest index available in the table.
> +        */
> +       --oldmaxjob;
>      }

Yes, that seems like it's doing the right thing.

I'll add documentation and tests to the patch for the other half and remove
changes to job state querying.

pws


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-29  9:08                 ` Peter Stephenson
@ 2022-03-29 13:31                   ` Peter Stephenson
  2022-03-29 15:39                     ` Bart Schaefer
  2022-03-29 15:36                   ` Bart Schaefer
  1 sibling, 1 reply; 15+ messages in thread
From: Peter Stephenson @ 2022-03-29 13:31 UTC (permalink / raw)
  To: Zsh hackers list

> On 29 March 2022 at 10:08 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> I'll add documentation and tests to the patch for the other half and remove
> changes to job state querying.

See if the documentation in the first hunk makes any sense.

pws

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 5649e00d4..1d74f0c17 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -1114,6 +1114,24 @@ The tt(-Z) option replaces the shell's argument and environment space with
 the given string, truncated if necessary to fit.  This will normally be
 visible in tt(ps) (manref(ps)(1)) listings.  This feature is typically
 used by daemons, to indicate their state.
+
+Full job control is only available in the top-level interactive shell,
+not in commands run in the left hand side of pipelines or within
+the tt(LPAR())var(...)tt(RPAR()) construct.  However, a snapshot
+of the job state at that point is taken, so it is still possible
+to use the tt(jobs) builtin, or any parameter providing job information.
+This gives information about the state of jobs at the point the subshell
+was created.  If background processes are created within the subshell,
+then instead information about those processes is provided.
+
+For example,
+
+example(sleep 10 &    # Job in background
+LPAR()             # Shell forks
+jobs          # Shows information about "sleep 10 &"
+sleep 5 &     # Process in background (no job control)
+jobs          # Shows information about "sleep 5 &"
+RPAR())
 )
 findex(kill)
 cindex(killing jobs)
diff --git a/Doc/Zsh/mod_parameter.yo b/Doc/Zsh/mod_parameter.yo
index 2e3011e44..f3bcd7957 100644
--- a/Doc/Zsh/mod_parameter.yo
+++ b/Doc/Zsh/mod_parameter.yo
@@ -165,6 +165,8 @@ The keys of the associative arrays are usually valid job numbers,
 and these are the values output with, for example, tt(${(k)jobdirs}).
 Non-numeric job references may be used when looking up a value;
 for example, tt(${jobdirs[%+]}) refers to the current job.
+
+See the tt(jobs) builtin for how job information is provided in a subshell.
 )
 vindex(jobtexts)
 item(tt(jobtexts))(
@@ -173,6 +175,8 @@ that were used to start the jobs.
 
 Handling of the keys of the associative array is as described for
 tt(jobdirs) above.
+
+See the tt(jobs) builtin for how job information is provided in a subshell.
 )
 vindex(jobstates)
 item(tt(jobstates))(
@@ -189,6 +193,8 @@ the var(state) describes the state of that process.
 
 Handling of the keys of the associative array is as described for
 tt(jobdirs) above.
+
+See the tt(jobs) builtin for how job information is provided in a subshell.
 )
 vindex(nameddirs)
 item(tt(nameddirs))(
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..a6f7a09b1 100644
--- a/Test/W03jobparameters.ztst
+++ b/Test/W03jobparameters.ztst
@@ -48,3 +48,31 @@
 *>running:+:*=running
 *>running:+:*=running
 *>zsh:*SIGHUPed*
+
+# $jobstates refers to a job started in the main shell unless
+# one has been started in the subshell.  In the latter case,
+# the subshell has no job control so the job is not marked as current.
+  zpty_start
+  zpty_input "MODULE_PATH=${(q)MODULE_PATH}"
+  zpty_input 'sleep 3 &'
+  zpty_input '(print main; print $jobstates; sleep 2& print sub; print $jobstates)'
+  zpty_input 'jobs -s'
+  zpty_stop
+0:$jobstate shows one job started in main shell or one started in subshell
+*>\[1] [0-9]##
+>main
+*>running:+:*=running
+>sub
+*>running::*=running
+*>zsh:*SIGHUPed*
+
+# output from zpty removes empty lines
+  zpty_start
+  zpty_input "MODULE_PATH=${(q)MODULE_PATH}"
+  zpty_input '(print main; print $jobstates; sleep 2& print sub; print $jobstates)'
+  zpty_input 'jobs -s'
+  zpty_stop
+0:$jobstate shows no job started in main shell but one started in subshell
+>main
+>sub
+*>running::*=running


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-29  9:08                 ` Peter Stephenson
  2022-03-29 13:31                   ` Peter Stephenson
@ 2022-03-29 15:36                   ` Bart Schaefer
  2022-03-29 15:41                     ` Peter Stephenson
  1 sibling, 1 reply; 15+ messages in thread
From: Bart Schaefer @ 2022-03-29 15:36 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Tue, Mar 29, 2022 at 2:09 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> Yes, that seems like it's doing the right thing.

Can I rely on you to commit all these related patches, including any
relevant ones of mine?  I've sort of lost track.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-29 13:31                   ` Peter Stephenson
@ 2022-03-29 15:39                     ` Bart Schaefer
  0 siblings, 0 replies; 15+ messages in thread
From: Bart Schaefer @ 2022-03-29 15:39 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Tue, Mar 29, 2022 at 6:33 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> See if the documentation in the first hunk makes any sense.

Reads well for me.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Crash on jobstates since workers/49783 (commit 6a8aa2a)
  2022-03-29 15:36                   ` Bart Schaefer
@ 2022-03-29 15:41                     ` Peter Stephenson
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Stephenson @ 2022-03-29 15:41 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

> On 29 March 2022 at 16:36 Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Tue, Mar 29, 2022 at 2:09 AM Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> >
> > Yes, that seems like it's doing the right thing.
> 
> Can I rely on you to commit all these related patches, including any
> relevant ones of mine?  I've sort of lost track.

Sure, I think that last short one of yours is all that's needed for
the bit I didn't fix so I'll commit it with the rest.

pws


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-03-29 15:42 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-25 22:32 Crash on jobstates since workers/49783 (commit 6a8aa2a) Bart Schaefer
2022-03-27  5:34 ` Bart Schaefer
2022-03-28  8:47   ` Peter Stephenson
2022-03-28 14:49     ` Peter Stephenson
2022-03-28 15:09       ` Bart Schaefer
2022-03-28 16:14         ` Peter Stephenson
2022-03-28 16:44           ` Bart Schaefer
2022-03-28 19:52             ` Peter Stephenson
2022-03-29  3:49               ` Bart Schaefer
2022-03-29  9:08                 ` Peter Stephenson
2022-03-29 13:31                   ` Peter Stephenson
2022-03-29 15:39                     ` Bart Schaefer
2022-03-29 15:36                   ` Bart Schaefer
2022-03-29 15:41                     ` Peter Stephenson
2022-03-28 15:13     ` Bart Schaefer

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).