signal handling bug

signal handling bug

[Zoltan Hidvegi]

Try

% zsh -c 'trap exit INT ; while true ; do sleep 1 ; done'

It is not interruptible with ^C.  The INT signal terminates the sleep 1
process but zsh ignores this and starts an other sleep.  Without sleep
^C works.  It also works without `trap exit INT' or in interractive shells.

Bye,

Zoltan
 

Re: signal handling bug

[Peter Stephenson]

hzoli@cs.elte.hu wrote:
> Try
> 
> % zsh -c 'trap exit INT ; while true ; do sleep 1 ; done'
> 
> It is not interruptible with ^C.  The INT signal terminates the sleep 1
> process but zsh ignores this and starts an other sleep.  Without sleep
> ^C works.  It also works without `trap exit INT' or in interractive shells.

I sent various patches for problems related to traps a long time ago
which haven't appeared yet.  They certainly involved problems like
this.

Here's a collection of the trap-related mail I sent last year.  I
don't even know if the patches still apply cleanly; several at least
probably don't, but all are nonetheless probably still applicable.
(Note the third one which doesn't contain any patches was a reply to a
message from Bart about the previous patch.)

--------
Delivery-Date: Mon, 15 May 95 20:07:36 +0100
Message-Id: <26426.9505151532@pyro.swan.ac.uk>
Received: from localhost by pyro.swan.ac.uk; Mon, 15 May 1995 16:32:50 +0100
To: zsh-list@sterling.com
Cc: Greg Boehnlein <root@bab5.nacs.net>
Subject: Re: Ctrl-C kills Shell (+ bugfix + bug)
In-Reply-To: "root@bab5.nacs.net"'s message of "Fri, 12 May 95 04:04:55 EDT." <Pine.LNX.3.91.950512040427.17583A-100000@bab5.nacs.net>
Date: Mon, 15 May 95 16:32:49 +0100
From: P.Stephenson@swansea.ac.uk
X-Mts: smtp
Errors-To: owner-zsh-list@sterling.com
Resent-Sender: owner-zsh-list@sterling.com
Resent-From: owner-zsh-list@sterling.com
Reply-To: zsh-list@sterling.com
X-Mailing-List: Z shell interest*6190 <zsh-list@sterling.com>

root@bab5.nacs.net wrote:
> Now.. Most everything works FINE in it except that it seems to handle the 
> Ctl-C (Interrupt) a bit stragenly.
> 
> For example, if this "shell" program runs a talk session, and the user 
> hits Ctrl-C to cancel the Talk session, the Shell exits and hangs up.

I believe that's correct.  This is what ksh does, anyway.

> 1. How can I stop this?

The easiest way is to tell the shell to handle the interrupt but
return from it immediately: then the programme gets the signal but zsh
ignores it.  (You can't do this in ksh; even a handled interrupt won't
return to where it was, and if you ignore an interrupt that happens in
the subshell too.)  You can reset the handler afterwards.  For
example,

trap 'return 0' INT	# Status zero means continue after handling
sleep 2			# Interrupting this stops the `sleep' only
echo after 1
trap - INT		# Turn off trap
sleep 2			# Interrupting this stops the script too
echo after 2

Unfortunately, this doesn't currently work in a script (though it does
in a function), since zsh won't restore the SIGINT handler
non-interactively.  This patch to 2.6 beta 8 fixes that.

Here's a more portable solution.  With the recent changes to zsh's
`exec' behaviour in a subshell, this ought to be fairly efficient.

trap '' INT		# Ignore traps.
(trap - INT; sleep 2)	# Execute command, restoring ^C handler
echo after 1
trap - INT
sleep 2
echo after 2

This also requires the patch to work non-interactively; however it
still doesn't work as a shell function in zsh, though it does in ksh.
I tried changing that `interact' test to a `jobbing' test, but that
didn't help: it claimed it was unsetting the trap, but it was still
uninterruptible. Anyone want to fix this?  (By the way, should
entersubh() unset the interactive option?)

*** Src/signals.c~	Fri May  5 03:22:17 1995
--- Src/signals.c	Mon May 15 15:29:02 1995
***************
*** 657,663 ****
          return;
      }
      sigtrapped[t0] = 0;
!     if (t0 == SIGINT)
          intr();
      else if (t0 == SIGHUP)
          install_handler(t0);
--- 657,663 ----
          return;
      }
      sigtrapped[t0] = 0;
!     if (t0 == SIGINT && interact)
          intr();
      else if (t0 == SIGHUP)
          install_handler(t0);

--------
Delivery-Date: Wed, 17 May 95 18:04:32 +0100
Message-Id: <669.9505171516@pyro.swan.ac.uk>
Received: from localhost by pyro.swan.ac.uk; Wed, 17 May 1995 16:16:56 +0100
To: zsh-list@sterling.com (Zsh mailing list)
Subject: trap - INT in subshell
Date: Wed, 17 May 95 16:16:55 +0100
From: P.Stephenson@swansea.ac.uk
X-Mts: smtp
Errors-To: owner-zsh-list@sterling.com
Resent-Sender: owner-zsh-list@sterling.com
Resent-From: owner-zsh-list@sterling.com
Reply-To: zsh-list@sterling.com
X-Mailing-List: Z shell interest*6200 <zsh-list@sterling.com>

[Resent message 3/4]

The problem I reported earlier which my patch didn't fix boils down
to:

% trap '' INT
% (trap - INT; sleep 2)
^C			# nothing happens.
% trap - INT
% (sleep 2)
^C			# works.

It seems that ^C is being blocked from delivery in the subshell:
presumably due to the holdintr() in execcmd() after the fork, though I
haven't checked.  This blocks ^C in the subshell if it's being ignored
in the parent:  anyone know why?

Anyway, if I revise the previous patch to unblock ^C when the trap is
unset, and change the printjob() code so that it only sets errflag on
a SIGINT if that's not being ignored, the code works.  I'd like some
signals expert to explain all this, though.

The patch allows both the hunks of code I posted yesterday [Monday,
actually] to be used both as scripts and functions.  I've added
comments in case it causes problems.

(Arguably we don't need doputnl = 1 in the first hunk either, since
the signal presumably can't have arisen from the current shell, but
that's just a nuance.)

*** Src/jobs.c.intr	Tue May 16 09:52:34 1995
--- Src/jobs.c	Tue May 16 10:18:51 1995
***************
*** 225,231 ****
  		    len = llen;
  		if (sig != SIGINT && sig != SIGPIPE)
  		    sflag = 1;
! 		else if (sig == SIGINT)
  		    errflag = 1;
  		if (job == thisjob && sig == SIGINT)
  		    doputnl = 1;
--- 225,232 ----
  		    len = llen;
  		if (sig != SIGINT && sig != SIGPIPE)
  		    sflag = 1;
! 		else if (sig == SIGINT && sigtrapped[SIGINT] != 2)
! 		    /* PWS 1995/05/16 added test for ignoring SIGINT */
  		    errflag = 1;
  		if (job == thisjob && sig == SIGINT)
  		    doputnl = 1;
*** Src/signals.c.intr	Mon May 15 13:48:30 1995
--- Src/signals.c	Tue May 16 10:20:32 1995
***************
*** 657,665 ****
          return;
      }
      sigtrapped[t0] = 0;
!     if (t0 == SIGINT)
          intr();
!     else if (t0 == SIGHUP)
          install_handler(t0);
      else if (t0 && t0 <= SIGCOUNT &&
  #ifdef SIGWINCH
--- 657,669 ----
          return;
      }
      sigtrapped[t0] = 0;
!     if (t0 == SIGINT && interact) {
! 	/* PWS 1995/05/16:  added test for interactive, also noholdintr()
! 	 * as subshells ignoring SIGINT have it blocked from delivery
! 	 */
          intr();
! 	noholdintr();
!     } else if (t0 == SIGHUP)
          install_handler(t0);
      else if (t0 && t0 <= SIGCOUNT &&
  #ifdef SIGWINCH

--------
Delivery-Date: Fri, 19 May 95 13:49:29 +0100
Resent-Date: Fri, 19 May 95 11:35:47 +0100
Old-Return-Path: <P.Stephenson@swansea.ac.uk>
Message-Id: <14488.9505191035@pyro.swan.ac.uk>
To: zsh-workers@math.gatech.edu
Subject: Re: trap - INT in subshell
In-Reply-To: "schaefer@z-code.com"'s message of "Wed, 17 May 95 10:00:47 PDT." <9505171000.ZM2944@zipx1.z-code.com>
Date: Fri, 19 May 95 11:35:47 +0100
From: P.Stephenson@swansea.ac.uk
X-Mts: smtp
Resent-Message-Id: <"KKeIR2.0.Mb4.JJ7ll"@math>
Resent-From: zsh-workers@math.gatech.edu
X-Mailing-List: <zsh-workers@math.gatech.edu> archive/latest/11
X-Loop: zsh-workers@math.gatech.edu
Precedence: list
Resent-Sender: zsh-workers-request@math.gatech.edu

schaefer@z-code.com wrote:
> I wrote about trappint SIGINT in subshells:
> } % trap '' INT
> } % (trap - INT; sleep 2)
> } ^C			# nothing happens.
>
> Zsh is apparently also blocking the signal, which it shouldn't need to
> if the handler has really been set to SGN_IGN; I'd suggest you try to
> find out why it is being blocked before you unblock it.  The blocking
> may have nothing to do with the trap, except by this accident of bad
> interaction between them.

Well, I removed both the holdintr() I talked about, just after the
shell forked in execcmd(), and the noholdintr, and sure enough that
stopped the behaviour above.  However, it also made children
interruptible:

% trap '' INT
% sleep 2
^C
%	# whoops

This is despite the fact that zsh appears happily to be setting
SIG_IGN for SIGINT in settrap() in signals.c, so the holdintr() is
clearly necessary at the moment.  This doesn't square with your
remarks above, so maybe that needs some investigation by the
signalling department.

However, it does make me pretty confident that the patch I posted does
the right thing given the current code.

> } Anyway, if I revise the previous patch to unblock ^C when the trap is
> } unset
> 
> This sounds right.  My only concern is that it might unblock the signal
> too soon and introduce a race condition; I haven't looked at zsh signal
> handling code in many months.

I wondered about this, but the code should only be executed during a
`trap', `disable' or `unfunction'/`unhash' builtin, so I don't think
this is an issue.  None of the holdintr() in the code appear to
conflict with this.

> } and change the printjob() code so that it only sets errflag on
> } a SIGINT if that's not being ignored, the code works.
> 
> This part I'm less sure about, but I think it's right.  That's the code
> that's trying to propagate the interrupt up to zsh when a child process
> is what actually caught the signal?
> 

This was part I was actually more sure about:  as you way, it marks an
error within zsh when the child died with SIGINT (and only then).
The new test is only going to apply when zsh was ignoring SIGINT but a
child wasn't, i.e. under just the cirumstances I'm fixing up.  (Well,
that's my claim.)

So I think the patch is O.K. (braces notwithstanding).

--------
Delivery-Date: Thu, 08 Jun 95 20:45:43 +0100
Resent-Date: Thu, 08 Jun 95 19:53:16 +0100
Old-Return-Path: <P.Stephenson@swansea.ac.uk>
Message-Id: <18246.9506081853@pyro.swan.ac.uk>
To: zsh-list@sterling.com
Subject: Re: interrupting loops
In-Reply-To: "mark%eggman.uucp@nosc.mil"'s message of "Tue, 18 Apr 95 22:20:34 PDT." <9504190520.AA12684@eggman.uucp>
Date: Thu, 08 Jun 95 19:53:16 +0100
From: P.Stephenson@swansea.ac.uk
X-Mts: smtp
Resent-Message-Id: <"PH2Y-.0.iB4.QUqrl"@math>
Resent-From: zsh-workers@math.gatech.edu
X-Mailing-List: <zsh-workers@math.gatech.edu> archive/latest/89
X-Loop: zsh-workers@math.gatech.edu
Precedence: list
Resent-Sender: zsh-workers-request@math.gatech.edu

Just found this old message:

mark%eggman.uucp@nosc.mil wrote:
> Richard writes:
> >> prompt% for x in 1 2 3 4 5; do
> >> > echo $x
> >> > sleep 1
> >> > done
> >> If you try to interrupt that loop, the sleep command gets interrupted,
> >> but then the loop continues with the next iteration.  I tried the same
> >> loop on ksh and sh, and both of the popped out of the loop just fine.
> >
> >I'm not able to duplicate this.  What machine type and zsh options
> >are you using?
> It looks like it's triggered by my having a TRAPZERR function defined.

dotrap() was being called although the function itself didn't do
anything because of the error flag.  I've put a test into dotrap(),
but note the comment.  The last part of the comment refers to another
bug I found at the same time which I'll post separately.

*** Src/signals.cerrt	Wed May 31 05:10:36 1995
--- Src/signals.c	Thu Jun  8 19:48:18 1995
***************
*** 681,688 ****
   
      sav = sigtrapped[sig];
      savval = lastval;
!     if (sav == 2)          /* if signal is being ignored, return */
          return;
      sigtrapped[sig] = 2;
      if (sigfuncs[sig]) {
          Lklist args;
--- 681,696 ----
   
      sav = sigtrapped[sig];
      savval = lastval;
!     if (errflag || sav == 2)
          return;
+     /* If signal is being ignored, return.
+      *  
+      *  Also return if errflag is set.  In fact, the code in the
+      *  function will test for this, but this way we keep status flags
+      *  intact without working too hard.  Special cases (e.g. calling
+      *  a trap for SIGINT after the error flag was set) are handled
+      *  by the calling code.  (PWS 1995/06/08).
+      */
      sigtrapped[sig] = 2;
      if (sigfuncs[sig]) {
          Lklist args;

--------
Delivery-Date: Fri, 09 Jun 95 15:30:22 +0100
Resent-Date: Fri, 09 Jun 95 14:37:11 +0100
Old-Return-Path: <P.Stephenson@swansea.ac.uk>
Message-Id: <21471.9506091337@pyro.swan.ac.uk>
To: zsh-workers@math.gatech.edu (Zsh hackers list)
Subject: Traps called for child signals
Date: Fri, 09 Jun 95 14:37:11 +0100
From: P.Stephenson@swansea.ac.uk
X-Mts: smtp
Resent-Message-Id: <"uvn5O2.0.PD6.Pw4sl"@math>
Resent-From: zsh-workers@math.gatech.edu
X-Mailing-List: <zsh-workers@math.gatech.edu> archive/latest/91
X-Loop: zsh-workers@math.gatech.edu
Precedence: list
Resent-Sender: zsh-workers-request@math.gatech.edu

This is what I mentioned in the fix for traps with interrupted shell
structures yesterday evening.

There's code in zsh to call a trap for e.g. SIGINT if it was actually
a child that got the signal (ksh does this too).  Unfortunately it
doesn't work at the moment because errflag is being set in printjob()
which means the trap code barfs.  I don't know when this got broken
and if anybody can shed some light on the rationale for it I'd like to
hear.

The answer is simply to set errflag = 0 before calling the trap.
That's because (as documented in the entry for `return' in the
zshbuiltins manual page) returning from a trap either continues the
job (zero return status) or aborts it (non zero return status), so the
previous errflag is not useful.  (Ksh doesn't have this feature, but
it does seem extremely useful; what ksh has instead is traps local to
functions which are called in the environment of that function,
i.e. traps are eval'd instead of themselves called as functions --- so
you can actually return from a function within the trap.)

HM Government Health Warning:  setting errflag to zero in general can
seriously damage your shell.

The point about the `breaks = loops' test is the way errors are
handled by the if/for/while/... code:  if errflag is found, lastval is
set to 1, so you lose the usual 128+SIGNO value.  If the loop is to be
exited, however, the previous value is retained.  This seems
preferable in this case.

*** Src/jobs.c.trp	Wed May 31 05:10:22 1995
--- Src/jobs.c	Fri Jun  9 14:13:28 1995
***************
*** 157,163 ****
--- 157,178 ----
      /* 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.
+ 	     * Setting breaks = loops ensures a consistent return
+ 	     * status if inside a loop.  Maybe the code in loops
+ 	     * should be changed.
+ 	     */
+ 	    if (errflag)
+ 		breaks = loops;
  	} else if (WTERMSIG(status) == SIGINT ||
  		   WTERMSIG(status) == SIGQUIT) {
  	    breaks = loops;

-- 
Peter Stephenson <pws@ifh.de>       Tel: +49 33762 77366
WWW:  http://www.ifh.de/~pws/       Fax: +49 33762 77330
Deutches Electronen-Synchrotron --- Institut fuer Hochenergiephysik Zeuthen
DESY-IfH, 15735 Zeuthen, Germany.

Re: signal handling bug

[Zoltan Hidvegi]

[-- Attachment #1: Type: application/pgp, Size: 5707 bytes --]

Re: signal handling bug

[Peter Stephenson]

hzoli@cs.elte.hu wrote:
> % zsh -c 'trap "echo Interrupt ; exit" INT ; while true ; do sleep 1 ; done'
> 
> After applying the patches, this is interruptible, but the trap is not
> executed (and it is interruptible even if I use trap - INT).

I was about to report failure when I had an idea (pity this doesn't
happen very often) and I've managed to fix the first half of the
problem, so what Robert the Bruce said is probably right.

The problem was the test for whether the job that just ran was in the
foreground:  it was relying on the job handling code, so didn't work
in a non-interactive shell.  I've separated the job handling part out
and set `inforeground' in updatestatus() simply according to whether
we are using the status of the current job to update the global status
variable, which sounds pretty plausible to me.  inforeground is later
tested to see if the trap should be run.

This doesn't fix the other half of the problem, that the job is
interruptible when supposedly ignoring SIGINT.  That's certainly a
separate problem, possibly beyond my signal knowledge (meaning of
course ``knowledge of signals'', rather than ``notable knowledge''),
but I'll probably take a look at it anyway.

Point to note: ksh M-11/16/88f (the most widespread version of ksh
'88) has the same problem --- the problem reported by Zoltan after the
previous set of patches, that is --- so we should feel either smug or
uneasy.

This patch is intended to go on top of all the other trap patches, of
course.  I don't know whether or not it actually needs them, but it's
pointless to use it without the others.

*** Src/jobs.c.spec	Mon Apr 15 11:45:03 1996
--- Src/jobs.c	Tue Apr 16 17:36:50 1996
***************
*** 123,140 ****
  	    return;
      } else {                   /* job is done, so remember return value */
  	lastval2 = val;
! 	/* If last process was run in the currrent shell, keep old status */
! 	if (job == thisjob && !(jn->stat & STAT_CURSH))
! 	    lastval = val;
      }
  
      pgrp = gettygrp();           /* get process group of tty      */
  
!     /* is this job in the foreground */
!     if (jn->gleader == pgrp || (pgrp > 1 && kill(-pgrp, 0) == -1))
! 	inforeground = 1;
! 
!     if (inforeground && !ttyfrozen && !val && !jn->stty_in_env)
  	gettyinfo(&shttyinfo);
      adjustwinsize();             /* check window size and adjust if necessary */
      if (somestopped && jn->stat & STAT_SUPERJOB)
--- 123,142 ----
  	    return;
      } else {                   /* job is done, so remember return value */
  	lastval2 = val;
! 	/* If last process was run in the current shell, keep old status
! 	 * and let it handle its own traps
! 	 */
! 	if (job == thisjob && !(jn->stat & STAT_CURSH)) {
! 	  lastval = val;
! 	  inforeground = 1;
! 	}
      }
  
      pgrp = gettygrp();           /* get process group of tty      */
  
!     /* is this job in the foreground of an interactive shell? */
!     if ((jn->gleader == pgrp || (pgrp > 1 && kill(-pgrp, 0) == -1)) &&
! 	!ttyfrozen && !val && !jn->stty_in_env)
  	gettyinfo(&shttyinfo);
      adjustwinsize();             /* check window size and adjust if necessary */
      if (somestopped && jn->stat & STAT_SUPERJOB)
-- 
Peter Stephenson <pws@ifh.de>       Tel: +49 33762 77366
WWW:  http://www.ifh.de/~pws/       Fax: +49 33762 77330
Deutches Electronen-Synchrotron --- Institut fuer Hochenergiephysik Zeuthen
DESY-IfH, 15735 Zeuthen, Germany.

signal handling bug

[Zoltan Hidvegi]

Here is a strange bug:

% zsh -f
hzoli% trap 'echo interrupt' INT
hzoli% while true ; do ; done
<press ^C many times quickly (eg. with an auto-repeating keyboard)>
interrupt
interrupt
BUG: empty(args) in exec.c
zsh: 1459 segmentation fault (core dumped)  zsh -f

The empry args message is very strange.  It can only happen if the
interrupt handler changes the args list of the interrupted execcmd()
function somehow.

The core dump comes from dotrap() in signals.c line 712 where the arguments
of the trap is freed after execution because someone already free'd that
(zsh-mem-warning  notices this).  Sometimes there is no coredump but after
a few interrupts the shell blocks the INT signal and continues execution.

Zoltan