* Re: Not quite out of the pipestatus woods yet ...
2013-10-26 16:57 ` Bart Schaefer
@ 2013-10-26 22:53 ` Peter Stephenson
2013-10-26 22:55 ` Bart Schaefer
1 sibling, 0 replies; 5+ messages in thread
From: Peter Stephenson @ 2013-10-26 22:53 UTC (permalink / raw)
To: zsh-workers
On Sat, 26 Oct 2013 09:57:46 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> HOWEVER, with the "setopt MONITOR" added before the stress test, I have
> had the shell go into a fast busy in acquire_pgrp() -- the while loop
> condition never becomes false but the shell never gets SIGT* either.
(Could perhaps be the same problem as the "i'm sorry, I can't do that
without a terminal, dave" race with a function at the end of the
pipeline?)
> I think that's a separate problem, but: PWS, do you remember enough
> about why there is an assumption that oldjobtab never needs to be set
> back to NULL once it has been created? Or is this just a case of not
> realizing that printjob() has multiple responsibilities, including the
> reaping of old jobs?
Orignally, I'm pretty sure (because for once I can remember doing it;
perhaps the coffee machine was broken) that oldjobtab was created to
allow you to look at the main shell's jobs from a subshell; they
wouldn't be updated any more, but users weren't too worried about the
races as they just wanted e.g. to be able to pipe the output of jobs in
a one-off fashion. In that case, of course, there's no question of
doing anything with it; it remains the same ancient history of the
origignal shell until the subshell dies. There was also no question of
job control in a subshell in those days --- that came significantly
later --- so copying it and clearing the main jobtab, rather than using
the latter in situ and assuming it was frozen, may just have been
caution.
If oldjobtab has suffered mission creep since then, there's a good
chance that any additional circumstance has been overlooked.
pws
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Not quite out of the pipestatus woods yet ...
2013-10-26 16:57 ` Bart Schaefer
2013-10-26 22:53 ` Peter Stephenson
@ 2013-10-26 22:55 ` Bart Schaefer
1 sibling, 0 replies; 5+ messages in thread
From: Bart Schaefer @ 2013-10-26 22:55 UTC (permalink / raw)
To: zsh-workers
On Oct 26, 9:57am, Bart Schaefer wrote:
} Subject: Re: Not quite out of the pipestatus woods yet ...
}
} On Oct 26, 12:45am, Bart Schaefer wrote:
} }
} } I think it has something to do with the oldjobtab / jobtab switching, but
} } have not yet confirmed.
}
} Yes, this [is] definitely it.
}
} HOWEVER, with the "setopt MONITOR" added before the stress test, I have
} had the shell go into a fast busy in acquire_pgrp() -- the while loop
} condition never becomes false but the shell never gets SIGT* either.
OK, I still don't recall why oldjobtab is hanging around all the time,
but the following seems to cover everything. I updated the stress
test to use three different exit values to be sure those values were
assigned to pipestats in the correct order.
There remains a question of what happens if (jn->stat & STAT_DONE) for
an entry in oldjobtab. I'm not sure what side effects fiddling with
curjob and prevjob may have in that case. Maybe it never happens.
diff --git a/Src/jobs.c b/Src/jobs.c
index c218743..336c5d4 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -941,10 +941,13 @@ printjob(Job jn, int lng, int synch)
int doneprint = 0, skip_print = 0;
FILE *fout = (synch == 2 || !shout) ? stdout : shout;
- if (oldjobtab != NULL)
+ if (synch > 1 && oldjobtab != NULL)
job = jn - oldjobtab;
else
job = jn - jobtab;
+ DPUTS3(job < 0 || job > (synch > 1 ? oldmaxjob : maxjob),
+ "bogus job number, jn = %L, jobtab = %L, oldjobtab = %L",
+ (long)jn, (long)jobtab, (long)oldjobtab);
if (jn->stat & STAT_NOPRINT) {
skip_print = 1;
@@ -995,7 +998,8 @@ printjob(Job jn, int lng, int synch)
if (skip_print) {
if (jn->stat & STAT_DONE) {
/* This looks silly, but see update_job() */
- storepipestats(jn, job == thisjob, job == thisjob);
+ if (synch <= 1)
+ storepipestats(jn, job == thisjob, job == thisjob);
if (should_report_time(jn))
dumptime(jn);
deletejob(jn, 0);
@@ -1128,7 +1132,8 @@ printjob(Job jn, int lng, int synch)
if (jn->stat & STAT_DONE) {
/* This looks silly, but see update_job() */
- storepipestats(jn, job == thisjob, job == thisjob);
+ if (synch <= 1)
+ storepipestats(jn, job == thisjob, job == thisjob);
if (should_report_time(jn))
dumptime(jn);
deletejob(jn, 0);
@@ -2610,6 +2615,8 @@ acquire_pgrp(void)
while ((ttpgrp = gettygrp()) != -1 && ttpgrp != mypgrp) {
mypgrp = GETPGRP();
if (mypgrp == mypid) {
+ if (!interact)
+ break; /* attachtty() will be a no-op, give up */
signal_setmask(oldset);
attachtty(mypgrp); /* Might generate SIGT* */
signal_block(blockset);
diff --git a/Test/A05execution.ztst b/Test/A05execution.ztst
index 8578016..ba7e02c 100644
--- a/Test/A05execution.ztst
+++ b/Test/A05execution.ztst
@@ -179,17 +179,23 @@
0:Status reset by starting a backgrounded command
>0
- repeat 2048; do (: | : | while false; do
+ setopt MONITOR
+ [[ -o MONITOR ]] || print -u $ZTST_fd 'Unable to change MONITOR option'
+ repeat 2048; do (return 2 |
+ return 1 |
+ while true; do
+ false
break
done;
print "${pipestatus[@]}")
ZTST_hashmark
done | sort | uniq -c | sed 's/^ *//'
0:Check whether `$pipestatus[]' behaves.
->2048 0 0 0
+>2048 2 1 0
F:This test checks for a bug in `$pipestatus[]' handling. If it breaks then
F:the bug is still there or it reappeared. See workers-29973 for details.
+ setopt MONITOR
externFunc() { awk >/dev/null 2>&1; true; }
false | true | false | true | externFunc
echo $pipestatus
^ permalink raw reply [flat|nested] 5+ messages in thread