From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25793 invoked by alias); 13 Aug 2011 18:52:38 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 29677 Received: (qmail 18284 invoked from network); 13 Aug 2011 18:52:36 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received-SPF: none (ns1.primenet.com.au: domain at closedmail.com does not designate permitted sender hosts) From: Bart Schaefer Message-id: <110813115208.ZM20513@torch.brasslantern.com> Date: Sat, 13 Aug 2011 11:52:08 -0700 In-reply-to: <20110809211910.631d6561@pws-pc.ntlworld.com> Comments: In reply to Peter Stephenson "Re: How to misplace an entire pipeline" (Aug 9, 9:19pm) References: <110805203111.ZM32508@torch.brasslantern.com> <20110807185002.6a042cab@pws-pc.ntlworld.com> <110807144359.ZM27903@torch.brasslantern.com> <110807210507.ZM28821@torch.brasslantern.com> <20110808192720.380a3ee7@pws-pc.ntlworld.com> <110808231032.ZM2380@torch.brasslantern.com> <20110809211910.631d6561@pws-pc.ntlworld.com> X-Mailer: OpenZMail Classic (0.9.2 24April2005) To: zsh-workers@zsh.org Subject: Re: How to misplace an entire pipeline MIME-version: 1.0 Content-type: text/plain; charset=us-ascii On Aug 9, 9:19pm, Peter Stephenson wrote: } Subject: Re: How to misplace an entire pipeline } } > } > + jobtab[thisjob].stat |= STAT_CURSH; } > } > + if (!jobtab[thisjob].procs) } > } > + jobtab[thisjob].stat |= STAT_NOPRINT; } > } > Should that be "if (hasprocs(thisjob))" instead, do you think? } } To a first approximation, the user isn't directly interested in the aux } procs, but there might still be some knock-on effect. So until we find } otherwise, I would guess not. Assuming that the aux procs are not subject to tty process group signals, it should be harmless to ignore them. I believe the patch below would make it possible to back out the above, because when the pipeline can't be stopped it's impossible to examine the job table anyway. However, I think it's not a problem to leave it, and there may be a few cases where it affects $jobtexts et al. in a useful way. } > What needs to be tested in place of (!list_pipe) to determine that } > the tail of the current pipeline is a simple shell builtin? } } I don't think we've remembered that fact, but it's available (as } is_builtin) in execcmd(), so could be stored before the builtin is } called. It appears sufficient to add this as another STAT_* bitflag in the job structure. It might actually make sense to make list_pipe into a bitflag as well, and mark every job in the table for which it was true at the time the job began, but I'm not prepared for that dive. } > In fact even that may not be enough, maybe this needs to know if the } > current job is a simple shell builtin -- it might be blocked on a } > "while read; do ..." or on a "wait" in the middle of a loop, etc. } } I suppose it depends on what cases the list_pipe logic would be } triggered. I fooled around with a few cases and I think I caught this one too. At this point list_pipe is true but the job receiving the stop signal is not in the process list of the current job (thisjob), so it's necessary to check whether the current job is a builtin. Unless you can think of a case where thisjob would not correctly identify the foreground job at the time the signal is handled? I'm still a little nervous about coughing up that zwarn() in the signal handler but I can't find a better place for it. One drawback here is that if the tail of the pipeline is a loop that's rapidly alternating between a builtin and an external command, ^Z might stop the loop sometimes and issue the warning other times, seemingly at random. diff -u ../zsh-forge/current/Src/exec.c ./Src/exec.c --- ../zsh-forge/current/Src/exec.c 2011-08-09 20:01:14.000000000 -0700 +++ ./Src/exec.c 2011-08-13 11:00:36.000000000 -0700 @@ -2848,6 +2848,8 @@ jobtab[thisjob].stat |= STAT_CURSH; if (!jobtab[thisjob].procs) jobtab[thisjob].stat |= STAT_NOPRINT; + if (is_builtin) + jobtab[thisjob].stat |= STAT_BUILTIN; } else { /* This is an exec (real or fake) for an external command. * * Note that any form of exec means that the subshell is fake * diff -u ../zsh-forge/current/Src/signals.c ./Src/signals.c --- ../zsh-forge/current/Src/signals.c 2011-08-06 11:13:23.000000000 -0700 +++ ./Src/signals.c 2011-08-11 09:55:26.000000000 -0700 @@ -490,14 +490,21 @@ * update it. */ if (findproc(pid, &jn, &pn, 0)) { + if (((jn->stat & STAT_BUILTIN) || + (list_pipe && (jobtab[thisjob].stat & STAT_BUILTIN))) && + WIFSTOPPED(status) && WSTOPSIG(status) == SIGTSTP) { + killjb(jn, SIGCONT); + zwarn("job can't be suspended"); + } else { #if defined(HAVE_WAIT3) && defined(HAVE_GETRUSAGE) - struct timezone dummy_tz; - gettimeofday(&pn->endtime, &dummy_tz); - pn->status = status; - pn->ti = ru; + struct timezone dummy_tz; + gettimeofday(&pn->endtime, &dummy_tz); + pn->status = status; + pn->ti = ru; #else - update_process(pn, status); + update_process(pn, status); #endif + } update_job(jn); } else if (findproc(pid, &jn, &pn, 1)) { pn->status = status; diff -u ../zsh-forge/current/Src/zsh.h ./Src/zsh.h --- ../zsh-forge/current/Src/zsh.h 2011-05-13 21:08:04.000000000 -0700 +++ ./Src/zsh.h 2011-08-10 09:10:35.000000000 -0700 @@ -907,6 +907,8 @@ #define STAT_ATTACH (0x1000) /* delay reattaching shell to tty */ #define STAT_SUBLEADER (0x2000) /* is super-job, but leader is sub-shell */ +#define STAT_BUILTIN (0x4000) /* job at tail of pipeline is a builtin */ + #define SP_RUNNING -1 /* fake status for jobs currently running */ struct timeinfo {