From: Peter Stephenson <p.stephenson@samsung.com>
To: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: PATCH: job control debug
Date: Thu, 6 Sep 2018 10:22:49 +0100 [thread overview]
Message-ID: <20180906092250eucas1p13d651e07ae627d179dd0701e65f912d6~RxTLztJrP2263722637eucas1p12@eucas1p1.samsung.com> (raw)
In-Reply-To: <20180906090902.1f344e9f@camnpupstephen.cam.scsc.local>
Typo fixed, now with setpgrp debugging.
Starting to think the key difference may be set_gleader(11) --- transfer
of job ownership on exit, which happens too early for that to be
possible in the bad case.
pws
Good:
% echo | { sleep 0.01 | more }
attachtty(7233728): pgrp = 15378, mypgrp = 15378
set_gleader(10): 1 = 0
set_gleader(3): 1 = 15379
setpgrp(4): pid 0, pgid 15379, current pid 15379, ret 0
attachtty(3): pgrp = 15379, mypgrp = 15378
set_gleader(10): 1 = 0
attachtty(7233728): pgrp = 15378, mypgrp = 15378
set_gleader(10): 1 = 0
set_gleader(9): 1 = 15379
set_gleader(10): 2 = 0
setpgrp(1): pid 0, pgid 15379, current pid 15380, ret 0
set_gleader(10): 1 = 0
set_gleader(10): 2 = 0
set_gleader(9): 2 = 15380
setpgrp(1): pid 0, pgid 15379, current pid 15381, ret 0
set_gleader(10): 1 = 0
set_gleader(10): 2 = 0
set_gleader(9): 2 = 15380
set_gleader(11): 2 = 0
attachtty(14): pgrp = 15378, mypgrp = 15378
set_gleader(8): 2 = 0
attachtty(8): pgrp = 15378, mypgrp = 15378
set_gleader(8): 1 = 0
attachtty(4): pgrp = 15378, mypgrp = 15378
Bad ("sleep 0" is the same, should perhaps use that for consistency):
camnpupstephen% echo | { : | more }
attachtty(7233728): pgrp = 15378, mypgrp = 15378
set_gleader(10): 1 = 0
set_gleader(3): 1 = 15384
setpgrp(4): pid 0, pgid 15384, current pid 15384, ret 0
attachtty(3): pgrp = 15384, mypgrp = 15378
set_gleader(10): 1 = 0
attachtty(7233728): pgrp = 15378, mypgrp = 15378
set_gleader(10): 1 = 0
set_gleader(9): 1 = 15384
set_gleader(10): 2 = 0
setpgrp(1): pid 0, pgid 15384, current pid 15385, ret 0
set_gleader(10): 1 = 0
set_gleader(10): 2 = 0
set_gleader(9): 2 = 15385
setpgrp(1): pid 0, pgid 15384, current pid 15386, ret 0
set_gleader(10): 1 = 0
zsh: done echo |
zsh: suspended (tty output) { : | more; }
set_gleader(10): 2 = 0
set_gleader(9): 2 = 15385
set_gleader(11): 2 = 0
attachtty(14): pgrp = 15378, mypgrp = 15378
set_gleader(6): 2 = 15384
attachtty(4): pgrp = 15378, mypgrp = 15378
diff --git a/Src/Modules/zpty.c b/Src/Modules/zpty.c
index 2f83f7c..f8a5a95 100644
--- a/Src/Modules/zpty.c
+++ b/Src/Modules/zpty.c
@@ -356,7 +356,7 @@ newptycmd(char *nam, char *pname, char **args, int echo, int nblock)
if (get_pty(0, &slave))
exit(1);
SHTTY = slave;
- attachtty(mypid);
+ ATTACHTTY(mypid, 17);
#ifdef TIOCGWINSZ
/* Set the window size before associating with the terminal *
* so that we don't get hit with a SIGWINCH. I'm paranoid. */
diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index 3487b5d..770c0c4 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -233,7 +233,7 @@ zsetterm(void)
shttyinfo.lmodes &= ~LFLUSHO;
#endif
- attachtty(mypgrp);
+ ATTACHTTY(mypgrp, 18);
ti = shttyinfo;
#ifdef HAS_TIO
if (unset(FLOWCONTROL))
@@ -917,7 +917,7 @@ getbyte(long do_keytmout, int *timeout, int full)
} else if (errno == EIO && !die) {
ret = opts[MONITOR];
opts[MONITOR] = 1;
- attachtty(mypgrp);
+ ATTACHTTY(mypgrp, 19);
zrefresh(); /* kludge! */
opts[MONITOR] = ret;
die = 1;
diff --git a/Src/builtin.c b/Src/builtin.c
index 93fa911..02bcd85 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -6129,7 +6129,7 @@ bin_read(char *name, char **args, Options ops, UNUSED(int func))
if (unset(INTERACTIVE))
gettyinfo(&shttyinfo);
/* attach to the tty */
- attachtty(mypgrp);
+ ATTACHTTY(mypgrp, 1);
if (!isem)
setcbreak();
readfd = SHTTY;
diff --git a/Src/exec.c b/Src/exec.c
index 09ee132..e00fae0 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1022,17 +1022,18 @@ entersubsh(int flags)
}
} else if (thisjob != -1 && (flags & ESUB_PGRP)) {
if (jobtab[list_pipe_job].gleader && (list_pipe || list_pipe_child)) {
- if (setpgrp(0L, jobtab[list_pipe_job].gleader) == -1 ||
+ if (SETPGRP(0L, jobtab[list_pipe_job].gleader, 1) == -1 ||
killpg(jobtab[list_pipe_job].gleader, 0) == -1) {
- jobtab[list_pipe_job].gleader =
- jobtab[thisjob].gleader = (list_pipe_child ? mypgrp : getpid());
- setpgrp(0L, jobtab[list_pipe_job].gleader);
+ SET_GLEADER(list_pipe_job,
+ (list_pipe_child ? mypgrp : getpid()), 1);
+ SET_GLEADER(thisjob, jobtab[list_pipe_job].gleader, 2);
+ SETPGRP(0L, jobtab[list_pipe_job].gleader, 2);
if (!(flags & ESUB_ASYNC))
- attachtty(jobtab[thisjob].gleader);
+ ATTACHTTY(jobtab[thisjob].gleader, 2);
}
}
else if (!jobtab[thisjob].gleader ||
- setpgrp(0L, jobtab[thisjob].gleader) == -1) {
+ SETPGRP(0L, jobtab[thisjob].gleader, 3) == -1) {
/*
* This is the standard point at which a newly started
* process gets put into the foreground by taking over
@@ -1043,13 +1044,13 @@ entersubsh(int flags)
* the operation is allowed to work (in addition to not
* causing the shell to be suspended).
*/
- jobtab[thisjob].gleader = getpid();
+ SET_GLEADER(thisjob, getpid(), 3);
if (list_pipe_job != thisjob &&
!jobtab[list_pipe_job].gleader)
- jobtab[list_pipe_job].gleader = jobtab[thisjob].gleader;
- setpgrp(0L, jobtab[thisjob].gleader);
+ SET_GLEADER(list_pipe_job, jobtab[thisjob].gleader, 4);
+ SETPGRP(0L, jobtab[thisjob].gleader, 4);
if (!(flags & ESUB_ASYNC))
- attachtty(jobtab[thisjob].gleader);
+ ATTACHTTY(jobtab[thisjob].gleader, 3);
}
}
if (!(flags & ESUB_FAKE))
@@ -1687,7 +1688,7 @@ execpline(Estate state, wordcode slcode, int how, int last1)
/* If the super-job contains only the sub-shell, the
sub-shell is the group leader. */
if (!jn->procs->next || lpforked == 2) {
- jn->gleader = list_pipe_pid;
+ SET_GLEADER(jn->gleader, list_pipe_pid, 5);
jn->stat |= STAT_SUBLEADER;
/*
* Pick up any subjob that's still lying around
@@ -1810,7 +1811,7 @@ execpline(Estate state, wordcode slcode, int how, int last1)
jn->stat |= STAT_SUBJOB | STAT_NOPRINT;
jn->other = list_pipe_pid; /* see zsh.h */
if (hasprocs(list_pipe_job))
- jn->gleader = jobtab[list_pipe_job].gleader;
+ SET_GLEADER(jn-jobtab, jobtab[list_pipe_job].gleader, 6);
}
if ((list_pipe || last1) && hasprocs(list_pipe_job))
killpg(jobtab[list_pipe_job].gleader, SIGSTOP);
@@ -1833,7 +1834,7 @@ execpline(Estate state, wordcode slcode, int how, int last1)
* do anything other than the following, but no
* doubt we'll find out...
*/
- setpgrp(0L, mypgrp = getpid());
+ SETPGRP(0L, mypgrp = getpid(), 5);
close(synch[1]);
kill(getpid(), SIGSTOP);
list_pipe = 0;
diff --git a/Src/hist.c b/Src/hist.c
index dbdc1e4..4e50b75 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -1116,7 +1116,7 @@ hbegin(int dohist)
hist_ring->ftim = time(NULL);
if ((dohist == 2 || (interact && isset(SHINSTDIN))) && !strin) {
histactive = HA_ACTIVE;
- attachtty(mypgrp);
+ ATTACHTTY(mypgrp, 4);
linkcurline();
defev = addhistnum(curhist, -1, HIST_FOREIGN);
} else
diff --git a/Src/jobs.c b/Src/jobs.c
index 38b3d89..1edb065 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -260,7 +260,7 @@ handle_sub(int job, int fg)
killpg(jn->gleader, 0) == -1))) {
Process p;
for (p = jn->procs; p->next; p = p->next);
- jn->gleader = p->pid;
+ SET_GLEADER(jn-jobtab, p->pid, 7);
}
/* This deleted the job too early if the parent
shell waited for a command in a list that will
@@ -275,7 +275,7 @@ handle_sub(int job, int fg)
now. */
if ((fg || thisjob == job) &&
(!jn->procs->next || cp || jn->procs->pid != jn->gleader))
- attachtty(jn->gleader);
+ ATTACHTTY(jn->gleader, 5);
kill(sj->other, SIGCONT);
if (jn->stat & STAT_DISOWN)
{
@@ -495,7 +495,7 @@ update_job(Job jn)
(jn->gleader == pgrp || (pgrp > 1 && kill(-pgrp, 0) == -1))) {
if (list_pipe) {
if (somestopped || (pgrp > 1 && kill(-pgrp, 0) == -1)) {
- attachtty(mypgrp);
+ ATTACHTTY(mypgrp, 6);
/* check window size and adjust if necessary */
adjustwinsize(0);
} else {
@@ -522,7 +522,7 @@ update_job(Job jn)
inerrflush();
}
} else {
- attachtty(mypgrp);
+ ATTACHTTY(mypgrp, 7);
/* check window size and adjust if necessary */
adjustwinsize(0);
}
@@ -1326,7 +1326,8 @@ freejob(Job jn, int deleting)
freejob(jobtab + jn->other, 0);
jn = jobtab + job;
}
- jn->gleader = jn->other = 0;
+ SET_GLEADER(jn-jobtab, 0, 8);
+ jn->other = 0;
jn->stat = jn->stty_in_env = 0;
jn->filelist = NULL;
jn->ty = NULL;
@@ -1353,7 +1354,7 @@ deletejob(Job jn, int disowning)
{
deletefilelist(jn->filelist, disowning);
if (jn->stat & STAT_ATTACH) {
- attachtty(mypgrp);
+ ATTACHTTY(mypgrp, 8);
adjustwinsize(0);
}
if (jn->stat & STAT_SUPERJOB) {
@@ -1395,7 +1396,7 @@ addproc(pid_t pid, char *text, int aux, struct timeval *bgtime)
/* if this is the first process we are adding to *
* the job, then it's the group leader. */
if (!jobtab[thisjob].gleader)
- jobtab[thisjob].gleader = pid;
+ SET_GLEADER(thisjob, pid, 9);
/* attach this process to end of process list of current job */
pnlist = &jobtab[thisjob].procs;
}
@@ -1643,7 +1644,7 @@ static int initnewjob(int i)
zsfree(jobtab[i].pwd);
jobtab[i].pwd = NULL;
}
- jobtab[i].gleader = 0;
+ SET_GLEADER(i, 0, 10);
if (i > maxjob)
maxjob = i;
@@ -2384,9 +2385,9 @@ bin_fg(char *name, char **argv, Options ops, int func)
(jobtab[job].stat & STAT_SUBLEADER) ||
killpg(jobtab[job].gleader, 0) == -1)) &&
jobtab[jobtab[job].other].gleader)
- attachtty(jobtab[jobtab[job].other].gleader);
+ ATTACHTTY(jobtab[jobtab[job].other].gleader, 9);
else
- attachtty(jobtab[job].gleader);
+ ATTACHTTY(jobtab[job].gleader, 10);
}
}
if (stopped) {
@@ -2848,11 +2849,15 @@ acquire_pgrp(void)
oldset = signal_block(blockset);
while ((ttpgrp = gettygrp()) != -1 && ttpgrp != mypgrp) {
mypgrp = GETPGRP();
+#ifdef DEBUG_JOB_CONTROL
+ fprintf(stderr, "mypgrp(1) %ld -> %d, pid %d, mypid %ld\n",
+ lastpgrp, mypgrp, getpid(), mypid);
+#endif
if (mypgrp == mypid) {
if (!interact)
break; /* attachtty() will be a no-op, give up */
signal_setmask(oldset);
- attachtty(mypgrp); /* Might generate SIGT* */
+ ATTACHTTY(mypgrp, 11); /* Might generate SIGT* */
signal_block(blockset);
}
if (mypgrp == gettygrp())
@@ -2861,14 +2866,22 @@ acquire_pgrp(void)
if (read(0, NULL, 0) != 0) {} /* Might generate SIGT* */
signal_block(blockset);
mypgrp = GETPGRP();
+#ifdef DEBUG_JOB_CONTROL
+ fprintf(stderr, "mypgrp(2) %ld -> %d, pid %d, mypid %ld\n",
+ lastpgrp, mypgrp, getpid(), mypid);
+#endif
if (mypgrp == lastpgrp && !interact)
break; /* Unlikely that pgrp will ever change */
lastpgrp = mypgrp;
}
if (mypgrp != mypid) {
- if (setpgrp(0, 0) == 0) {
+ if (SETPGRP(0, 0, 6) == 0) {
mypgrp = mypid;
- attachtty(mypgrp);
+#ifdef DEBUG_JOB_CONTROL
+ fprintf(stderr, "mypgrp(3) %ld -> %d, pid %d, mypid %ld\n",
+ lastpgrp, mypgrp, getpid(), mypid);
+#endif
+ ATTACHTTY(mypgrp, 12);
} else
opts[MONITOR] = 0;
}
@@ -2886,9 +2899,13 @@ release_pgrp(void)
if (origpgrp != mypgrp) {
/* in linux pid namespaces, origpgrp may never have been set */
if (origpgrp) {
- attachtty(origpgrp);
- setpgrp(0, origpgrp);
+ ATTACHTTY(origpgrp, 13);
+ SETPGRP(0, origpgrp, 7);
}
+#ifdef DEBUG_JOB_CONTROL
+ fprintf(stderr, "mypgrp(4) %d -> %d, pid %d, mypid %ld\n",
+ mypgrp, origpgrp, getpid(), mypid);
+#endif
mypgrp = origpgrp;
}
}
diff --git a/Src/params.c b/Src/params.c
index a1c299f..66687fb 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2586,7 +2586,7 @@ assignstrvalue(Value v, char *val, int flags)
Param pm = v->pm;
/* Size doesn't change, can limit actions to only
* overwriting bytes in already allocated string */
- strncpy(z + v->start, val, vlen);
+ memcpy(z + v->start, val, vlen);
/* Implement remainder of strsetfn */
if (!(pm->node.flags & PM_HASHELEM) &&
((pm->node.flags & PM_NAMEDDIR) ||
diff --git a/Src/signals.c b/Src/signals.c
index 20c6fdf..868764d 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -540,7 +540,7 @@ wait_for_processes(void)
if (WIFEXITED(status) &&
pn->pid == jn->gleader &&
killpg(pn->pid, 0) == -1) {
- jn->gleader = 0;
+ SET_GLEADER(jn-jobtab, 0, 11);
if (!(jn->stat & STAT_NOSTTY)) {
/*
* This PID was in control of the terminal;
@@ -549,7 +549,7 @@ wait_for_processes(void)
* process of this job will become group
* leader, however.
*/
- attachtty(mypgrp);
+ ATTACHTTY(mypgrp, 14);
}
}
}
diff --git a/Src/utils.c b/Src/utils.c
index 075d272..42f6ab7 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -2952,7 +2952,7 @@ getquery(char *valid_chars, int purge)
int isem = !strcmp(term, "emacs");
struct ttyinfo ti;
- attachtty(mypgrp);
+ ATTACHTTY(mypgrp, 15);
gettyinfo(&ti);
#ifdef HAS_TIO
@@ -4639,13 +4639,21 @@ setcbreak(void)
/* give the tty to some process */
-/**/
+/* note: deliberately not automatically prototyped */
mod_export void
-attachtty(pid_t pgrp)
+attachtty(pid_t pgrp
+#ifdef DEBUG_JOB_CONTROL
+ , int index
+#endif
+ )
{
static int ep = 0;
if (jobbing && interact) {
+#ifdef DEBUG_JOB_CONTROL
+ fprintf(stderr, "attachtty(%d): pgrp = %d, mypgrp = %d\n",
+ index, pgrp, mypgrp);
+#endif
#ifdef HAVE_TCSETPGRP
if (SHTTY != -1 && tcsetpgrp(SHTTY, pgrp) == -1 && !ep)
#else
@@ -4658,8 +4666,11 @@ attachtty(pid_t pgrp)
# endif
#endif
{
+#ifdef DEBUG_JOB_CONTROL
+ fprintf(stderr, "attachtty failed\n");
+#endif
if (pgrp != mypgrp && kill(-pgrp, 0) == -1)
- attachtty(mypgrp);
+ ATTACHTTY(mypgrp, 16);
else {
if (errno != ENOTTY)
{
@@ -4673,6 +4684,26 @@ attachtty(pid_t pgrp)
}
}
+/**/
+#ifdef DEBUG_JOB_CONTROL
+
+void set_gleader(int job, int pid, int index)
+{
+ jobtab[job].gleader = pid;
+ fprintf(stderr, "set_gleader(%d): %d = %d\n", index, job, pid);
+}
+
+int setpgrp_debug(int pid, int pgid, int index)
+{
+ int ret = setpgrp(pid, pgid);
+ fprintf(stderr, "setpgrp(%d): pid %d, pgid %d, current pid %d, ret %d\n",
+ index, pid, pgid, getpid(), ret);
+ return ret;
+}
+
+/**/
+#endif /* DEBUG_JOB_CONTROL */
+
/* get the process group associated with the tty */
/**/
diff --git a/Src/zsh.h b/Src/zsh.h
index 8e7f20b..bcad47b 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -3292,7 +3292,7 @@ typedef int convchar_t;
#define MB_METASTRLEN2(str, widthp) ztrlen(str)
#define MB_METASTRLEN2END(str, widthp, eptr) ztrlenend(str, eptr)
-#define MB_CHARINIT()
+#define MBT_CHARINIT()
#define MB_CHARLENCONV(str, len, cp) charlenconv((str), (len), (cp))
#define MB_CHARLEN(str, len) ((len) ? 1 : 0)
@@ -3303,3 +3303,26 @@ typedef int convchar_t;
#define ZWS(s) s
#endif /* MULTIBYTE_SUPPORT */
+
+
+/* Uncomment to debug problems with job control */
+/*#define DEBUG_JOB_CONTROL*/
+
+#ifdef DEBUG_JOB_CONTROL
+#define ATTACHTTY(pgrp, index) attachtty(pgrp, index)
+void attachtty(pid_t pgrp, int index);
+
+#define SET_GLEADER(job, pid, index) set_gleader(job, pid, index)
+void set_gleader(int job, int pid, int index);
+
+#define SETPGRP(pid, pgid, index) setpgrp_debug(pid, pgid, index)
+int setpgrp_debug(int pid, int pgid, int index);
+#else
+
+#define ATTACHTTY(pgrp, index) attachtty(pgrp)
+
+#define SET_GLEADER(job, pid, index) (jobtab[(job)].gleader = (pid))
+void attachtty(pid_t pgrp);
+
+#define SETPGRP(pid, pgid, index) setpgrp(pid, pgid)
+#endif
next prev parent reply other threads:[~2018-09-06 9:23 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20180905200816epcas5p18ce6c49baa637e7d83a769e97c4364fb@epcas5p1.samsung.com>
2018-09-05 20:07 ` Peter Stephenson
2018-09-06 8:09 ` Peter Stephenson
[not found] ` <20180906090902.1f344e9f@camnpupstephen.cam.scsc.local>
2018-09-06 9:22 ` Peter Stephenson [this message]
2018-09-07 3:18 ` Bart Schaefer
2018-09-07 9:18 ` Peter Stephenson
[not found] ` <20180907101852.62415ff9@camnpupstephen.cam.scsc.local>
2018-09-07 11:21 ` Peter Stephenson
[not found] ` <20180907122145.2af5bcba@camnpupstephen.cam.scsc.local>
2018-09-07 14:01 ` PATCH: Rimmerworld pipeline race Peter Stephenson
[not found] ` <20180907150140.46a05880@camnpupstephen.cam.scsc.local>
2018-09-07 14:58 ` Peter Stephenson
2018-09-07 16:46 ` Peter Stephenson
2018-09-07 23:40 ` Bart Schaefer
2018-09-08 17:37 ` Peter Stephenson
2018-09-08 18:40 ` Peter Stephenson
2018-09-10 22:55 ` Axel Beckert
2018-09-11 8:58 ` Peter Stephenson
2018-09-11 14:40 ` Vincent Lefevre
2018-09-11 15:03 ` Peter Stephenson
[not found] ` <20180911160326.33dfd575@camnpupstephen.cam.scsc.local>
2018-09-11 15:32 ` Peter Stephenson
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='20180906092250eucas1p13d651e07ae627d179dd0701e65f912d6~RxTLztJrP2263722637eucas1p12@eucas1p1.samsung.com' \
--to=p.stephenson@samsung.com \
--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).