From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4068 invoked from network); 11 Jan 1997 17:29:38 -0000 Received: from euclid.skiles.gatech.edu (list@130.207.146.50) by coral.primenet.com.au with SMTP; 11 Jan 1997 17:29:38 -0000 Received: (from list@localhost) by euclid.skiles.gatech.edu (8.7.3/8.7.3) id MAA00710; Sat, 11 Jan 1997 12:36:03 -0500 (EST) Resent-Date: Sat, 11 Jan 1997 12:36:03 -0500 (EST) From: Zoltan Hidvegi Message-Id: <199701111727.SAA01061@hzoli.ppp.cs.elte.hu> Subject: Re: signal weirdness fix To: schaefer@brasslantern.com Date: Sat, 11 Jan 1997 18:27:26 +0100 (MET) Cc: zefram@dcs.warwick.ac.uk, zsh-workers@math.gatech.edu In-Reply-To: <961126024255.ZM3729774@srf-79.nbn.com> from Bart Schaefer at "Nov 26, 96 02:41:51 am" X-Mailer: ELM [version 2.4ME+ PL17 (25)] MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Resent-Message-ID: <"LR57c.0.1B.3-yro"@euclid> Resent-From: zsh-workers@math.gatech.edu X-Mailing-List: archive/latest/2776 X-Loop: zsh-workers@math.gatech.edu Precedence: list Resent-Sender: zsh-workers-request@math.gatech.edu Bart Schaefer wrote: > On Nov 26, 8:46am, Zefram wrote: > > Remember that odd behaviour I reported, that zsh thought it received a > > signal actually sent to its foreground job? > > > > This patch limits it to SIGHUP, SIGINT and SIGQUIT, and disables this > > behaviour completely in non-interactive shells. I think this is a > good > > semantic. > > Actually, I think this is almost exactly the wrong semantic. The whole > point was so that zsh scripts (which are pretty much by definition not > "interactive shells" even though the script may be doing terminal I/O) > could have their HUP/INT/QUIT traps tripped even when not executing a > builtin command at the time the signal came in. > > If you must limit this to some subset of shells (and not just to some > subset of signals), I beg you to choose some other criteria. I have no > problem with limiting it to HUP/INT/QUIT, but requiring interact is too > much. I investigated this problem a little more and it turns out that Zefram's patch in article 2480 was almost the right thing to do. A terminal signal is sent to all processes whose process group is the same as the terminal's process group. When the MONITOR option is set, foreground processes have a different process group from the shell so in that case the shell itself could not see terminal signals. Examining the Linux kernel, and the behaviour of ksh and pdksh it seems that only SIGINT, SIGQUIT and SIGTSTP needs this special treatment. SIGTSTP is not handled by this patch as it requires a different treatment. When MONITOR is not set the shell and the foreground process runs in the same process group so both receive the signal. As a result zsh executed the handler twice before this patch. There was an other bug here: when SIGINT was received errflag was set to 1 in printjob() called from update_job() called from handler(). I do not completely understand this but errflag should only be set if SIGINT is not trapped. Originally it was always set. Peter sent a patch to the old zsh-list in article 6200 to set it only when INT is not ignored and now I only set it when INT is neither ignored nor trapped. I also noticed that on Linux signals received while they are blocked are not dropped they are just delayed until they are unblocked. If that happens on all systems we may even leave the current zsh behaviour which blocks all signals while it is waiting for foreground process. Now the answer to the `WHY DO WE USE THE RETURN STATUS OF PROCESS GROUP LEADER HERE?' question in update_job is clear: a foreground job runs in a separate process group when monitor is set so it is quite natural to use the status of the leader. Although this patch fixes some bad signal problems in zsh I still think that it is a big mess and should be ceaned up. The most important is to move trap execution out of the signal handler. We should only use libc calls which are guaranteed to be reentrant on all systems. This probably means that we cannot use malloc() and stdio functions. Preferably we should use system calls only. This patch should be applied to both zsh-3.0.x and zsh-3.1.x. If you applied patch 2480 from Zefram back it out before applying this patch. Zoltan *** Src/jobs.c 1997/01/11 00:45:31 3.1.1.3 --- Src/jobs.c 1997/01/11 16:54:48 *************** *** 177,193 **** if (sigtrapped[SIGCHLD] && job != thisjob) dotrap(SIGCHLD); ! /* WHY DO WE USE THE RETURN STATUS OF PROCESS GROUP LEADER HERE? */ ! /* If the foreground job got a signal, pretend we got it, too. */ ! if (inforeground && WIFSIGNALED(status)) { ! if (sigtrapped[WTERMSIG(status)]) { /* Run the trap with the error flag unset. * Errflag is set in printjobs if the jobs terminated * with SIGINT. I don't know why it's done there and * not here. (PWS 1995/06/08) */ errflag = 0; ! dotrap(WTERMSIG(status)); /* We keep the errflag as set or not by dotrap. * This is to fulfil the promise to carry on * with the jobs if trap returns zero. --- 177,199 ---- if (sigtrapped[SIGCHLD] && job != thisjob) dotrap(SIGCHLD); ! /* When MONITOR is set, the foreground process runs in a different * ! * process group from the shell, so the shell will not receive * ! * terminal signals, therefore we we pretend that the shell got * ! * the signal too. */ ! if (inforeground && isset(MONITOR) && WIFSIGNALED(status)) { ! int sig = WTERMSIG(status); ! ! if(sig != SIGINT && sig != SIGQUIT) ! ; ! else if (sigtrapped[sig]) { /* Run the trap with the error flag unset. * Errflag is set in printjobs if the jobs terminated * with SIGINT. I don't know why it's done there and * not here. (PWS 1995/06/08) */ errflag = 0; ! dotrap(sig); /* We keep the errflag as set or not by dotrap. * This is to fulfil the promise to carry on * with the jobs if trap returns zero. *************** *** 197,204 **** */ if (errflag) breaks = loops; ! } else if (WTERMSIG(status) == SIGINT || ! WTERMSIG(status) == SIGQUIT) { breaks = loops; errflag = 1; } --- 203,209 ---- */ if (errflag) breaks = loops; ! } else if (sig == SIGINT || sig == SIGQUIT) { breaks = loops; errflag = 1; } *************** *** 424,431 **** len = llen; if (sig != SIGINT && sig != SIGPIPE) sflag = 1; ! else if (sig == SIGINT && !(sigtrapped[SIGINT] & ZSIG_IGNORED)) ! /* PWS 1995/05/16 added test for ignoring SIGINT */ errflag = 1; if (job == thisjob && sig == SIGINT) doputnl = 1; --- 429,435 ---- len = llen; if (sig != SIGINT && sig != SIGPIPE) sflag = 1; ! else if (sig == SIGINT && !sigtrapped[SIGINT]) errflag = 1; if (job == thisjob && sig == SIGINT) doputnl = 1;