Re: signal handling bug

[Peter Stephenson <pws@xxxxxxxxxxxx>]

hzoli@xxxxxxxxxx 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@xxxxxxxxxxxxxxx>
Received: from localhost by pyro.swan.ac.uk; Mon, 15 May 1995 16:32:50 +0100
To: zsh-list@xxxxxxxxxxxx
Cc: Greg Boehnlein <root@xxxxxxxxxxxxx>
Subject: Re: Ctrl-C kills Shell (+ bugfix + bug)
In-Reply-To: "root@xxxxxxxxxxxxx"'s message of "Fri, 12 May 95 04:04:55 EDT." <Pine.LNX.3.91.950512040427.17583A-100000@xxxxxxxxxxxxx>
Date: Mon, 15 May 95 16:32:49 +0100
From: P.Stephenson@xxxxxxxxxxxxx
X-Mts: smtp
Errors-To: owner-zsh-list@xxxxxxxxxxxx
Resent-Sender: owner-zsh-list@xxxxxxxxxxxx
Resent-From: owner-zsh-list@xxxxxxxxxxxx
Reply-To: zsh-list@xxxxxxxxxxxx
X-Mailing-List: Z shell interest*6190 <zsh-list@xxxxxxxxxxxx>

root@xxxxxxxxxxxxx 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@xxxxxxxxxxxxxxx>
Received: from localhost by pyro.swan.ac.uk; Wed, 17 May 1995 16:16:56 +0100
To: zsh-list@xxxxxxxxxxxx (Zsh mailing list)
Subject: trap - INT in subshell
Date: Wed, 17 May 95 16:16:55 +0100
From: P.Stephenson@xxxxxxxxxxxxx
X-Mts: smtp
Errors-To: owner-zsh-list@xxxxxxxxxxxx
Resent-Sender: owner-zsh-list@xxxxxxxxxxxx
Resent-From: owner-zsh-list@xxxxxxxxxxxx
Reply-To: zsh-list@xxxxxxxxxxxx
X-Mailing-List: Z shell interest*6200 <zsh-list@xxxxxxxxxxxx>

[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@xxxxxxxxxxxxx>
Message-Id: <14488.9505191035@xxxxxxxxxxxxxxx>
To: zsh-workers@xxxxxxxxxxxxxxx
Subject: Re: trap - INT in subshell
In-Reply-To: "schaefer@xxxxxxxxxx"'s message of "Wed, 17 May 95 10:00:47 PDT." <9505171000.ZM2944@xxxxxxxxxxxxxxxx>
Date: Fri, 19 May 95 11:35:47 +0100
From: P.Stephenson@xxxxxxxxxxxxx
X-Mts: smtp
Resent-Message-Id: <"KKeIR2.0.Mb4.JJ7ll"@math>
Resent-From: zsh-workers@xxxxxxxxxxxxxxx
X-Mailing-List: <zsh-workers@xxxxxxxxxxxxxxx> archive/latest/11
X-Loop: zsh-workers@xxxxxxxxxxxxxxx
Precedence: list
Resent-Sender: zsh-workers-request@xxxxxxxxxxxxxxx

schaefer@xxxxxxxxxx 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@xxxxxxxxxxxxx>
Message-Id: <18246.9506081853@xxxxxxxxxxxxxxx>
To: zsh-list@xxxxxxxxxxxx
Subject: Re: interrupting loops
In-Reply-To: "mark%eggman.uucp@xxxxxxxx"'s message of "Tue, 18 Apr 95 22:20:34 PDT." <9504190520.AA12684@xxxxxxxxxxx>
Date: Thu, 08 Jun 95 19:53:16 +0100
From: P.Stephenson@xxxxxxxxxxxxx
X-Mts: smtp
Resent-Message-Id: <"PH2Y-.0.iB4.QUqrl"@math>
Resent-From: zsh-workers@xxxxxxxxxxxxxxx
X-Mailing-List: <zsh-workers@xxxxxxxxxxxxxxx> archive/latest/89
X-Loop: zsh-workers@xxxxxxxxxxxxxxx
Precedence: list
Resent-Sender: zsh-workers-request@xxxxxxxxxxxxxxx

Just found this old message:

mark%eggman.uucp@xxxxxxxx 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@xxxxxxxxxxxxx>
Message-Id: <21471.9506091337@xxxxxxxxxxxxxxx>
To: zsh-workers@xxxxxxxxxxxxxxx (Zsh hackers list)
Subject: Traps called for child signals
Date: Fri, 09 Jun 95 14:37:11 +0100
From: P.Stephenson@xxxxxxxxxxxxx
X-Mts: smtp
Resent-Message-Id: <"uvn5O2.0.PD6.Pw4sl"@math>
Resent-From: zsh-workers@xxxxxxxxxxxxxxx
X-Mailing-List: <zsh-workers@xxxxxxxxxxxxxxx> archive/latest/91
X-Loop: zsh-workers@xxxxxxxxxxxxxxx
Precedence: list
Resent-Sender: zsh-workers-request@xxxxxxxxxxxxxxx

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@xxxxxx>       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.