From: Bart Schaefer <schaefer@brasslantern.com>
To: Jun T <takimoto-j@kba.biglobe.ne.jp>
Cc: zsh-workers@zsh.org
Subject: Re: [PATCH] problem with 'ls | less' shell function
Date: Fri, 4 Nov 2022 17:09:36 -0700 [thread overview]
Message-ID: <CAH+w=7bP0kqdKAzHsTmuyUStucuU0eAct35BineP5GFN05o6QA@mail.gmail.com> (raw)
In-Reply-To: <7BB37CE7-16A2-4079-956B-802E3FB4DC82@kba.biglobe.ne.jp>
[-- Attachment #1: Type: text/plain, Size: 1267 bytes --]
On Fri, Nov 4, 2022 at 8:12 AM Jun T <takimoto-j@kba.biglobe.ne.jp> wrote:
>
> 548 /* is this job in the foreground of an interactive shell? */
> 549 if (mypgrp != pgrp && inforeground &&
> 550 (jn->gleader == pgrp ||
> 551 (pgrp > 1 &&
> 552 (kill(-pgrp, 0) == -1 && errno == ESRCH)))) {
>
> But why just "jn->gleader == pgrp" is enough to assume that the foreground
> job has finished?
> [...]
> Another code that looks suspicious to me is (also in jobs.c)
>
> 478 if (pn->pid == jn->gleader) /* if this process is process group leader */
> 479 status = pn->status;
> 480 }
>
> [...]
> If the line 478 is replaced by
>
> 478 if (WIFSIGNALED(pn->status) || pn->pid == jn->gleader)
>
> then [2] can be killed by a single ^C.
Thank you! This is the clue I needed.
On line 476 is
476 signalled = WIFSIGNALED(pn->status);
If we change line 550 to be
550 ((jn->gleader == pgrp && signalled) ||
then all three examples start working.
Second patch attached, replaces the one from workers/50862. Tests
pass, which is not surprising since all of this depends on signaling
interactive shells, but I have no idea how to write a regression test
for that.
[-- Attachment #2: pipejobs2.txt --]
[-- Type: text/plain, Size: 2422 bytes --]
diff --git a/Src/jobs.c b/Src/jobs.c
index 707374297..76c762ee5 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -544,16 +544,14 @@ update_job(Job jn)
if (isset(MONITOR)) {
pid_t pgrp = gettygrp(); /* get process group of tty */
+ int deadpgrp = (mypgrp != pgrp && inforeground && pgrp > 1 &&
+ kill(-pgrp, 0) == -1 && errno == ESRCH);
/* is this job in the foreground of an interactive shell? */
if (mypgrp != pgrp && inforeground &&
- (jn->gleader == pgrp ||
- (pgrp > 1 &&
- (kill(-pgrp, 0) == -1 && errno == ESRCH)))) {
+ ((jn->gleader == pgrp && signalled) || deadpgrp)) {
if (list_pipe) {
- if (somestopped || (pgrp > 1 &&
- kill(-pgrp, 0) == -1 &&
- errno == ESRCH)) {
+ if (somestopped || deadpgrp) {
attachtty(mypgrp);
/* check window size and adjust if necessary */
adjustwinsize(0);
@@ -566,6 +564,12 @@ update_job(Job jn)
* when the job is finally deleted.
*/
jn->stat |= STAT_ATTACH;
+ /*
+ * If we're in shell jobs on the right side of a pipeline
+ * we should treat it like a job in the current shell.
+ */
+ if (inforeground == 2)
+ inforeground = 1;
}
/* If we have `foo|while true; (( x++ )); done', and hit
* ^C, we have to stop the loop, too. */
@@ -1488,10 +1492,7 @@ addproc(pid_t pid, char *text, int aux, struct timeval *bgtime,
* set it for that, too.
*/
if (gleader != -1) {
- if (jobtab[thisjob].stat & STAT_CURSH)
- jobtab[thisjob].gleader = gleader;
- else
- jobtab[thisjob].gleader = pid;
+ jobtab[thisjob].gleader = gleader;
if (list_pipe_job_used != -1)
jobtab[list_pipe_job_used].gleader = gleader;
/*
@@ -1500,7 +1501,7 @@ addproc(pid_t pid, char *text, int aux, struct timeval *bgtime,
*/
last_attached_pgrp = gleader;
} else if (!jobtab[thisjob].gleader)
- jobtab[thisjob].gleader = pid;
+ jobtab[thisjob].gleader = pid;
/* attach this process to end of process list of current job */
pnlist = &jobtab[thisjob].procs;
}
@@ -2506,6 +2507,7 @@ bin_fg(char *name, char **argv, Options ops, int func)
jobtab[job].stat &= ~STAT_CURSH;
}
if ((stopped = (jobtab[job].stat & STAT_STOPPED))) {
+ /* WIFCONTINUED will makerunning() again at killjb() */
makerunning(jobtab + job);
if (func == BIN_BG) {
/* Set $! to indicate this was backgrounded */
next prev parent reply other threads:[~2022-11-05 0:10 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-17 9:17 Thomas Klausner
2022-10-17 14:50 ` Mikael Magnusson
2022-10-17 15:36 ` Thomas Klausner
2022-10-19 8:26 ` zsugabubus
2022-10-19 10:04 ` Jun T
2022-10-19 13:36 ` Jun. T
2022-10-20 0:10 ` Bart Schaefer
2022-10-20 16:26 ` Peter Stephenson
2022-10-21 5:45 ` Jun T
2022-10-21 7:40 ` Jun T
2022-10-21 21:22 ` Bart Schaefer
2022-10-21 21:24 ` Bart Schaefer
2022-10-22 23:22 ` Bart Schaefer
2022-10-22 23:43 ` Bart Schaefer
2022-11-03 23:10 ` Bart Schaefer
2022-11-04 6:09 ` [PATCH] " Bart Schaefer
2022-11-04 15:10 ` [PATCH] " Jun T
2022-11-05 0:09 ` Bart Schaefer [this message]
2022-11-06 19:12 ` Bart Schaefer
2022-11-07 8:43 ` Jun T
2022-11-07 19:33 ` Bart Schaefer
2022-11-07 19:44 ` Roman Perepelitsa
2022-11-08 0:46 ` Bart Schaefer
2022-11-08 0:44 ` Bart Schaefer
2022-11-08 5:03 ` Bart Schaefer
2022-11-09 5:03 ` Bart Schaefer
2022-10-19 9:33 ` Jun T
2022-10-19 10:01 ` Jun T
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAH+w=7bP0kqdKAzHsTmuyUStucuU0eAct35BineP5GFN05o6QA@mail.gmail.com' \
--to=schaefer@brasslantern.com \
--cc=takimoto-j@kba.biglobe.ne.jp \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).