Re: FreeBSD compatability feature request

Re: FreeBSD compatability feature request

[Vincent Stemen]

On Mon, Apr 19, 2004 at 12:28:29PM +0100, Peter Stephenson wrote:
> Vincent Stemen wrote:
> > Here is the FreeBSD manual entry:
> > 
> >   -T trapsasync
> >         When waiting for a child, execute traps immediately.  If this
> >         option is not set, traps are executed after the child exits,
> >         as specified in IEEE Std 1003.2 (``POSIX.2'') This nonstandard
> >         option is useful for putting guarding shells around children
> >         that block signals.  The surrounding shell may kill the child
> >         or it may just return control to the tty and leave the child
> >         alone, like this:
> > 
> >            sh -T -c "trap 'exit 1' 2 ; some-blocking-program"
> 
> Good(ish) news...
> 
> Just tried this out, and it looks like the `trapsasync' behaviour
> is the standard (and only) one in zsh.  Hence it doesn't conform to
> POSIX (which isn't likely to be an earth-shattering revelation to zsh
> regulars).
> 
> This means that almost everything we need for both behaviours is already
> there.  It remains to add the option TRAPS_ASYNC, turned on by default
> in non-Bourne mode for compatibility, and make sure we can still handle
> SIGCHLD when the option is unset.
> 
> Someone already had the foresight to handle sh options separately, so -T
> does the right thing in sh or ksh emulation and the example you gave
> should now work as expected.
> 
> Further follow-ups should probably go to zsh-workers.
> 

Ok, I applied the patch to zsh-4.2.0 and tested under
FreeBSD-5.2.1-RELEASE.  It seems to work just fine.  I did not test the
actual signal handling but FreeBSD was able to boot without getting errors
from the init scripts that use "set -T". I was also pleased to find that
the other problem is fixed in 4.2.0 where eval was returning the result of
the last run command if eval was given a null argument.  Previously, that
had caused eval to return false in one of the scripts causing the rootfs
to stay mounted read only, which of course caused many other errors.

After replacing /bin/sh with zsh, it was able to fully boot up and
initialize the system except for one other problem, which appears to be a
different bug in sh emulation mode.  It hangs when running the nfsd init
script.  If I <cntl>c out of it, the rest of the system comes up, but
without NFS.

I finally isolated the problem.  Below is a small script, called t, that
reproduces it.

# cat t

_find_processes()
{
    ps | while read pid command
    do
        echo "pid=$pid  $command"
    done >&2

    echo "xxx Exited loop xxxxxxxxxxxxxxxxxxxxxxx" >&2
}

xxx=$(_find_processes)

#--- end of script -------

If I run it by calling zsh as sh, it hangs on the read command after the
last line of output from ps and I have to break out of it.

root@quark # ./sh t
pid=PID  TT  STAT      TIME COMMAND
pid=24781  p0  I      0:00.02 su
pid=24782  p0  I      0:00.23 su (zsh)
..
..
pid=578  v6  IWs+   0:00.00 /usr/libexec/getty Pc ttyv6
pid=579  v7  IWs+   0:00.00 /usr/libexec/getty Pc ttyv7
^C
root@quark # 

If I run it as zsh, it works properly.

root@quark # ./zsh t
pid=PID  TT  STAT      TIME COMMAND
pid=24781  p0  I      0:00.02 su
pid=24782  p0  I      0:00.23 su (zsh)
..
..
pid=578  v6  IWs+   0:00.00 /usr/libexec/getty Pc ttyv6
pid=579  v7  IWs+   0:00.00 /usr/libexec/getty Pc ttyv7
xxx Exited loop xxxxxxxxxxxxxxxxxxxxxxx
root@quark #

It only hangs when the function is called from inside of $() or back
ticks.  Calling it outside of that worked correctly.

Regards,
Vincent

-- 
Vincent Stemen
Avoid the VeriSign/Network Solutions domain registration trap!
http://www.InetAddresses.net

Re: FreeBSD compatability feature request

[Peter Stephenson]

Vincent Stemen wrote:
> After replacing /bin/sh with zsh, it was able to fully boot up and
> initialize the system except for one other problem, which appears to be a
> different bug in sh emulation mode.  It hangs when running the nfsd init
> script.  If I <cntl>c out of it, the rest of the system comes up, but
> without NFS.
> 
> I finally isolated the problem.  Below is a small script, called t, that
> reproduces it.
> 
> # cat t
> 
> _find_processes()
> {
>     ps | while read pid command
>     do
>         echo "pid=$pid  $command"
>     done >&2
> 
>     echo "xxx Exited loop xxxxxxxxxxxxxxxxxxxxxxx" >&2
> }
> 
> xxx=$(_find_processes)
> 
> #--- end of script -------

Making a link to sh from zsh and running `sh t' I can get this to happen
on Fedora, but only intermittently.  I think it's the new option I
added, since I can get this to happen with `zsh -f -o notrapsasync', but
not with `zsh -f'.  So it looks like I've introduced a race.

This obviously needs fixing before we can release anything with the
option it it, but is going to take some tracing.  Here is what I've got.

I have two processes running:
pws      18664 14486  0 10:38 pts/2    00:00:00 sh t
pws      18665 18664  0 10:38 pts/2    00:00:00 sh t

(I thought I had another one but it seemed to be hung from a previous
go.)

18664 is waiting for output from the the $(...) to finish.  This is normal.

18665 is waiting for a job to finish in zwaitjob.  Somehow it's
got into a state where it wasn't prepared to handle the SIGCHLD, and
it's therefore missed it.

I'll keep looking.  Any suggestions would be gratefully received.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

PATCH: (2) Re: FreeBSD compatability feature request

[Peter Stephenson]

OK, plan B.

I am now frightened about what happens when any signals are blocked
while we are waiting for a child.  So instead of doing that, I propose
queuing traps separately from signals.  This can't cause any races,
since no extra signal or process handling is involved.  The code is a
bit more verbose, but should be much safer.  This patches on top of the
other one.

Again, this should be exactly equivalent to 4.2.0 unless TRAPS_ASYNC is
unset (POSIX compatibility mode).

This ought to fix the hang.

Index: Src/jobs.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/jobs.c,v
retrieving revision 1.26
diff -u -r1.26 jobs.c
--- Src/jobs.c	19 Apr 2004 16:02:22 -0000	1.26
+++ Src/jobs.c	21 Apr 2004 10:40:30 -0000
@@ -994,11 +994,8 @@
     int q = queue_signal_level();
     Job jn = jobtab + job;
 
-    queue_not_sigchld++;
-    if (isset(TRAPSASYNC))
-	dont_queue_signals();
-    else
-	queue_signals();
+    dont_queue_signals();
+    queue_traps();
     child_block();		 /* unblocked during child_suspend() */
     if (jn->procs || jn->auxprocs) { /* if any forks were done         */
 	jn->stat |= STAT_LOCKED;
@@ -1029,10 +1026,8 @@
 	numpipestats = 1;
     }
     child_unblock();
+    dont_queue_traps();
     restore_queue_signals(q);
-    if (!queueing_enabled)
-	run_queued_signals();
-    queue_not_sigchld--;
 }
 
 /* wait for running job to finish */
Index: Src/signals.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/signals.c,v
retrieving revision 1.26
diff -u -r1.26 signals.c
--- Src/signals.c	19 Apr 2004 16:02:22 -0000	1.26
+++ Src/signals.c	21 Apr 2004 10:40:30 -0000
@@ -49,12 +49,19 @@
 /* Variables used by signal queueing */
 
 /**/
-mod_export int queueing_enabled, queue_front, queue_rear, queue_not_sigchld;
+mod_export int queueing_enabled, queue_front, queue_rear;
 /**/
 mod_export int signal_queue[MAX_QUEUE_SIZE];
 /**/
 mod_export sigset_t signal_mask_queue[MAX_QUEUE_SIZE];
 
+/* Variables used by trap queueing */
+
+/**/
+mod_export int trap_queueing_enabled, trap_queue_front, trap_queue_rear;
+/**/
+mod_export int trap_queue[MAX_QUEUE_SIZE];
+
 /* This is only used on machines that don't understand signal sets.  *
  * On SYSV machines this will represent the signals that are blocked *
  * (held) using sighold.  On machines which can't block signals at   *
@@ -426,7 +433,7 @@
 #endif
 
     /* Are we queueing signals now?      */
-    if (queueing_enabled && (sig != SIGCHLD || !queue_not_sigchld)) {
+    if (queueing_enabled) {
         int temp_rear = ++queue_rear % MAX_QUEUE_SIZE;
 
 	DPUTS(temp_rear == queue_front, "BUG: signal queue full");
@@ -1058,5 +1065,17 @@
     if ((sigtrapped[sig] & ZSIG_IGNORED) || !sigfuncs[sig] || errflag)
 	return;
 
+    /* Adapted from signal queueing in zhandler */
+    if (trap_queueing_enabled && !isset(TRAPSASYNC)) {
+	int temp_rear = ++trap_queue_rear % MAX_QUEUE_SIZE;
+
+	DPUTS(temp_rear == trap_queue_front, "BUG: trap queue full");
+	if (temp_rear != trap_queue_front) {
+	    trap_queue_rear = temp_rear;
+	    trap_queue[trap_queue_rear] = sig;
+	}
+	return;
+    }
+
     dotrapargs(sig, sigtrapped+sig, sigfuncs[sig]);
 }
Index: Src/signals.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/signals.h,v
retrieving revision 1.4
diff -u -r1.4 signals.h
--- Src/signals.h	18 Jun 2001 07:24:23 -0000	1.4
+++ Src/signals.h	21 Apr 2004 10:40:30 -0000
@@ -101,6 +101,23 @@
 
 #define restore_queue_signals(q) (queueing_enabled = (q))
 
+/*
+ * Similar (but simpler) mechanism used for queueing traps.
+ * Only needed if NO_TRAPS_ASYNC is set.
+ */
+#define queue_traps()	(trap_queueing_enabled++)
+
+#define run_queued_traps() do { \
+    while (trap_queue_front != trap_queue_rear) { /* while traps in queue */ \
+	trap_queue_front = (trap_queue_front + 1) % MAX_QUEUE_SIZE; \
+	dotrap(trap_queue[trap_queue_front]);  /* handle queued trap   */ \
+    } \
+} while (0)
+
+#define dont_queue_traps() do { \
+    trap_queueing_enabled = 0; \
+    run_queued_traps(); \
+} while (0)
 
 /* Make some signal functions faster. */
 
-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: PATCH: (2) Re: FreeBSD compatability feature request

[Vincent Stemen]

On Wed, Apr 21, 2004 at 12:05:00PM +0100, Peter Stephenson wrote:
> OK, plan B.
> 
> I am now frightened about what happens when any signals are blocked
> while we are waiting for a child.  So instead of doing that, I propose
> queuing traps separately from signals.  This can't cause any races,
> since no extra signal or process handling is involved.  The code is a
> bit more verbose, but should be much safer.  This patches on top of the
> other one.
> 
> Again, this should be exactly equivalent to 4.2.0 unless TRAPS_ASYNC is
> unset (POSIX compatibility mode).
> 
> This ought to fix the hang.

Hi Peter.

I applied the patch.  It definitely fixed the hanging problem.  I can
now plug it in place of /bin/sh on FreeBSD and all the init scripts
seem to run properly without errors when booting the system.

However, I actually tested the signal handling this time and, unless I
am not understanding the trapsasync mode enough to test properly, I
don't think it is working.  If I understand it correctly, with "set -T"
shouldn't the parent process immediately get any signals it has trapped
even if a child process it is waiting on has disabled the signals?

With that in mind, I tested it with two scripts, sigtrap and sigblock.

quark # cat sigtrap

echo "sigtrap: trapping SIGINT and calling sigblock"
set -T
trap 'echo Got signal. Exiting.; exit' 2
./sigblock
echo "sigtrap: Exiting normally"


quark # cat sigblock

echo "sigblock: blocking SIGINT and sleeping"
trap '' 2
sleep 5


quark # ./sh sigtrap
sigtrap: trapping SIGINT and calling sigblock
sigblock: blocking SIGINT and sleeping
^C^Csigblock: exiting
Got signal. Exiting.
quark #


After hitting <cntl>c, it completed the 5 second delay before printing
"sigblock: exiting".  It behaved the same whether I had the "set -T" in
the script or not.  I was expecting sigtrap to immediately print "Got
signal. Exiting." when I hit <cntl>c with trapsasync set.

Be aware if you test this under Linux that Linux, at least as of the
2.4.x kernels about a year ago the last time I tested it, does not pass
signals on to the parent process like FreeBSD does.  So you may not see
the "Got signal" message from the parent sigtrap process.  In the past,
I always had to check for a 130 return code from every child process
(which was a pain) to know if the user hit <cntl>c so I can call my
signal handling routine.  I was not able to have dependable signal traps
in my scripts because you could not always depend on the child returning
130 when it got interrupted.

-- 
Vincent Stemen
Avoid the VeriSign/Network Solutions domain registration trap!
Read how Network Solutions (NSI) was involved in stealing our domain name.
http://www.InetAddresses.net

Re: PATCH: (2) Re: FreeBSD compatability feature request

[Peter Stephenson]

Vincent Stemen wrote:
> However, I actually tested the signal handling this time and, unless I
> am not understanding the trapsasync mode enough to test properly, I
> don't think it is working.  If I understand it correctly, with "set -T"
> shouldn't the parent process immediately get any signals it has trapped
> even if a child process it is waiting on has disabled the signals?

I think you're right.  I didn't do anything with basic signal handling
(which I don't know much about), just rewrote the way traps are queued.
It looks like it will need someone to rewire the signal handling not to
block other signals in this case.  The function is signal_suspend() in
signals.c which calls sigfillset() then removes SIGCHLD from the blocked
set.

However, that now gives us three different modes: the original zsh one,
POSIX where all traps are deferred, and this mode where signals are
delivered straight through.  So one option isn't enough.  Presumably
TRAPS_ASYNC should be off for the standard zsh behaviour, on for the new
behaviour, and we need another option for POSIX which will be overridden
if TRAPS_ASYNC is set.  Yuk.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: PATCH: (2) Re: FreeBSD compatability feature request

[Peter Stephenson]

Peter Stephenson wrote:
> However, that now gives us three different modes: the original zsh one,
> POSIX where all traps are deferred, and this mode where signals are
> delivered straight through.  So one option isn't enough.  Presumably
> TRAPS_ASYNC should be off for the standard zsh behaviour, on for the new
> behaviour, and we need another option for POSIX which will be overridden
> if TRAPS_ASYNC is set.  Yuk.

Actually (sorry to generate all the traffic) it's possible I was getting
confused...

Zsh will usually block other signals while waiting for SIGCHLD.  This
may be all that POSIX requires.  What it doesn't do (and why I was able
to get asynchronous traps working) is ensure that traps don't run if it
stops waiting, for example because a different process exited.  I
originally tested this by running a background functions which exited,
generating a SIGCHLD, which caused all queued signals to be delivered.

Hence it's possible that the queueing traps thing is a red herring, that
zsh is sufficiently POSIX compliant by default, and all we need is the
ability to receive signals other than SIGCHLD while waiting for a child.
(It's a bit late to back out now.)

If anybody actually knows anything about all this, their views would be
especially welcome.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: PATCH: (2) Re: FreeBSD compatability feature request

[Jos Backus]

On Thu, Apr 22, 2004 at 11:17:19AM +0100, Peter Stephenson wrote:
> If anybody actually knows anything about all this, their views would be
> especially welcome.

Does this help?

    http://www.cons.org/cracauer/sigint.html

(Martin Cracauer has been maintaining the FreeBSD /bin/sh on and off.)

-- 
Jos Backus                       _/  _/_/_/      Sunnyvale, CA
                                _/  _/   _/
                               _/  _/_/_/
                          _/  _/  _/    _/
jos at catnook.com        _/_/   _/_/_/          require 'std/disclaimer'

Re: PATCH: (2) Re: FreeBSD compatability feature request

[Vincent Stemen]

On Thu, Apr 22, 2004 at 09:09:35AM -0700, Jos Backus wrote:
> On Thu, Apr 22, 2004 at 11:17:19AM +0100, Peter Stephenson wrote:
> > If anybody actually knows anything about all this, their views would be
> > especially welcome.
> 
> Does this help?
> 
>     http://www.cons.org/cracauer/sigint.html
> 
> (Martin Cracauer has been maintaining the FreeBSD /bin/sh on and off.)

That is an informative article.  It includes an interesting section on
"How to be a proper shell".  However, I didn't see any reference to
implementing asynchronous signal traps.  I did find another article
that discusses process waiting and might have helpful information.

http://www.cs.pdx.edu/~jrb/cs303/handouts/on.wait

I also don't have a full understanding of all aspects of signal
handling, but it looks to me like in order to implement trapsasync
mode, it would involve using the WNOHANG option to waitpid() or
wait3() or wait4() to periodically check the child status without
blocking the parent until the child exits.  That way I am guessing you
can immediately receive signals in the parent and kill the child,
exit, or whatever.  My question is, does that affect the child's
ability to be interactive.

I saw a place in the zsh code where WNOHANG was being used in a call
to WAIT() in signals.c:zhandler() but I havn't studied the code well
enough to understand what it is doing or whether that is the relevant
area of code relating to trapsasync.  

-- 
Vincent Stemen
Avoid the VeriSign/Network Solutions domain registration trap!
Read how Network Solutions (NSI) was involved in stealing our domain name.
http://www.InetAddresses.net

Re: PATCH: (2) Re: FreeBSD compatability feature request

[Peter Stephenson]

Vincent Stemen wrote:
> I also don't have a full understanding of all aspects of signal
> handling, but it looks to me like in order to implement trapsasync
> mode, it would involve using the WNOHANG option to waitpid() or
> wait3() or wait4() to periodically check the child status without
> blocking the parent until the child exits.  That way I am guessing you
> can immediately receive signals in the parent and kill the child,
> exit, or whatever.  My question is, does that affect the child's
> ability to be interactive.

Some parts of the shell do use this, but in the main part where the
shell waits for a job (zwaitjob() in jobs.c) it simple calls
sigsuspend(), installing a particular signal mask.  What's more, there's
already a loop, so it's reasonably safe about bogus signals.  It looks
likes it's probably not too hard to alter, except that signals are tricky
things.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: PATCH: (2) Re: FreeBSD compatability feature request

[Bart Schaefer]

On Apr 22, 11:17am, Peter Stephenson wrote:
}
} If anybody actually knows anything about all this, their views would be
} especially welcome.

This sounds like something to ask about on the <shell@research.att.com>
list.

Re: PATCH: (2) Re: FreeBSD compatability feature request

[Peter Stephenson]

Bart Schaefer wrote:
> On Apr 22, 11:17am, Peter Stephenson wrote:
> }
> } If anybody actually knows anything about all this, their views would be
> } especially welcome.
> 
> This sounds like something to ask about on the <shell@research.att.com>
> list.

It's not so much the standard as implementing an alternative in a
consistent way that I'm worried about.

However, there is a difference in the current implementation:

$ fn() { sleep 5; kill -USR1 $$; }
$ trap 'echo USR1 fired' USR1
$ fn & sleep 10

ksh (88), bash and Solaris sh wait till the end of the sleep before
delivering the `USR1 fired' (and, where appropriate, sending the
notification message).  In zsh they appear in the middle.  That's
because the exiting background process gives a SIGCHLD and we don't care
which child exits, we just go into end-of-job-processing mode, then
return to wait for the foreground process when we find that's not the
one that exited.

I don't know if this is important enough to worry about.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: PATCH: (2) Re: FreeBSD compatability feature request

[Vincent Stemen]

On Tue, Apr 27, 2004 at 10:58:59AM +0100, Peter Stephenson wrote:
> Bart Schaefer wrote:
> > On Apr 22, 11:17am, Peter Stephenson wrote:
> > }
> > } If anybody actually knows anything about all this, their views would be
> > } especially welcome.
> > 
> > This sounds like something to ask about on the <shell@research.att.com>
> > list.
> 
> It's not so much the standard as implementing an alternative in a
> consistent way that I'm worried about.
> 
> However, there is a difference in the current implementation:
> 
> $ fn() { sleep 5; kill -USR1 $$; }
> $ trap 'echo USR1 fired' USR1
> $ fn & sleep 10
> 
> ksh (88), bash and Solaris sh wait till the end of the sleep before
> delivering the `USR1 fired' (and, where appropriate, sending the
> notification message).  In zsh they appear in the middle.  That's
> because the exiting background process gives a SIGCHLD and we don't care
> which child exits, we just go into end-of-job-processing mode, then
> return to wait for the foreground process when we find that's not the
> one that exited.
> 
> I don't know if this is important enough to worry about.

I tested this under /bin/sh on FreeBSD and it also behaves the same as
bash and the others.  Even though, unless I am missing something here,
zsh seems to have the most appropriate behavior.  I don't quite
understand why the others don't immediately process the signal.  As
long as you are not blocking the signal I would expect it to print the
'echo USR1 fired' message as soon as it receives the signal whether it
is in the middle of a sleep or any other command.  

Vincent

-- 
Vincent Stemen
Avoid the VeriSign/Network Solutions domain registration trap!
Read how Network Solutions (NSI) was involved in stealing our domain name.
http://www.InetAddresses.net

PATCH: (3) Re: FreeBSD compatability feature request

[Peter Stephenson]

Patch to apply on top of previous goes to attempt to handle TRAPS_ASYNC
the way I think is intended.  It's basically an `if' with a
sigemptyset() inside it, hope no one was expecting anything sophisticated.

It doesn't fix the behaviour I noted, that without the option any child
exiting, not just the one the shell is currently expecting, causes traps
to be run.  Probably that's not a big issue; I should really check the
wording of POSIX.

So I think that's it... Vincent?

Index: Doc/Zsh/options.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/options.yo,v
retrieving revision 1.31
diff -u -r1.31 options.yo
--- Doc/Zsh/options.yo	19 Apr 2004 16:02:22 -0000	1.31
+++ Doc/Zsh/options.yo	30 Apr 2004 21:15:10 -0000
@@ -1191,11 +1191,11 @@
 )
 pindex(TRAPS_ASYNC)
 cindex(traps, asynchronous)
-item(tt(TRAPS_ASYNC) <C> <Z>)(
-While waiting for a program to exit, run traps immediately.  Otherwise
-the trap is run after the program has exited.  Note this does not affect
-the point at which traps are run for any case other than when the shell is
-waiting for a child process.
+item(tt(TRAPS_ASYNC))(
+While waiting for a program to exit, handle signals and run traps
+immediately.  Otherwise the trap is run after a child process has exited.
+Note this does not affect the point at which traps are run for any case
+other than when the shell is waiting for a child process.
 )
 pindex(TYPESET_SILENT)
 item(tt(TYPESET_SILENT))(
Index: Src/jobs.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/jobs.c,v
retrieving revision 1.27
diff -u -r1.27 jobs.c
--- Src/jobs.c	21 Apr 2004 11:21:24 -0000	1.27
+++ Src/jobs.c	30 Apr 2004 21:15:19 -0000
@@ -971,14 +971,14 @@
 
     /* child_block() around this loop in case #ifndef WNOHANG */
     dont_queue_signals();
-    child_block();		/* unblocked in child_suspend() */
+    child_block();		/* unblocked in signal_suspend() */
     while (!errflag && (kill(pid, 0) >= 0 || errno != ESRCH)) {
 	if (first)
 	    first = 0;
 	else
 	    kill(pid, SIGCONT);
 
-	child_suspend(SIGINT);
+	signal_suspend(SIGCHLD, SIGINT);
 	child_block();
     }
     child_unblock();
@@ -995,8 +995,7 @@
     Job jn = jobtab + job;
 
     dont_queue_signals();
-    queue_traps();
-    child_block();		 /* unblocked during child_suspend() */
+    child_block();		 /* unblocked during signal_suspend() */
     if (jn->procs || jn->auxprocs) { /* if any forks were done         */
 	jn->stat |= STAT_LOCKED;
 	if (jn->stat & STAT_CHANGED)
@@ -1004,7 +1003,7 @@
 	while (!errflag && jn->stat &&
 	       !(jn->stat & STAT_DONE) &&
 	       !(interact && (jn->stat & STAT_STOPPED))) {
-	    child_suspend(sig);
+	    signal_suspend(SIGCHLD, sig);
 	    /* Commenting this out makes ^C-ing a job started by a function
 	       stop the whole function again.  But I guess it will stop
 	       something else from working properly, we have to find out
@@ -1026,7 +1025,6 @@
 	numpipestats = 1;
     }
     child_unblock();
-    dont_queue_traps();
     restore_queue_signals(q);
 }
 
Index: Src/options.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/options.c,v
retrieving revision 1.18
diff -u -r1.18 options.c
--- Src/options.c	19 Apr 2004 16:02:22 -0000	1.18
+++ Src/options.c	30 Apr 2004 21:15:22 -0000
@@ -203,7 +203,7 @@
 {NULL, "singlelinezle",	      OPT_KSH,			 SINGLELINEZLE},
 {NULL, "sunkeyboardhack",     0,			 SUNKEYBOARDHACK},
 {NULL, "transientrprompt",    0,			 TRANSIENTRPROMPT},
-{NULL, "trapsasync",	      OPT_EMULATE|OPT_NONBOURNE, TRAPSASYNC},
+{NULL, "trapsasync",	      0,			 TRAPSASYNC},
 {NULL, "typesetsilent",	      OPT_EMULATE|OPT_BOURNE,	 TYPESETSILENT},
 {NULL, "unset",		      OPT_EMULATE|OPT_BSHELL,	 UNSET},
 {NULL, "verbose",	      0,			 VERBOSE},
Index: Src/signals.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/signals.c,v
retrieving revision 1.27
diff -u -r1.27 signals.c
--- Src/signals.c	21 Apr 2004 11:21:34 -0000	1.27
+++ Src/signals.c	30 Apr 2004 21:15:24 -0000
@@ -350,11 +350,15 @@
     sigset_t oset;
 #endif /* BROKEN_POSIX_SIGSUSPEND */
 
-    sigfillset(&set);
-    sigdelset(&set, sig);
-    sigdelset(&set, SIGHUP);  /* still don't know why we add this? */
-    if (sig2)
-        sigdelset(&set, sig2);
+    if (isset(TRAPSASYNC)) {
+	sigemptyset(&set);
+    } else {
+	sigfillset(&set);
+	sigdelset(&set, sig);
+	sigdelset(&set, SIGHUP);  /* still don't know why we add this? */
+	if (sig2)
+	    sigdelset(&set, sig2);
+    }
 #ifdef BROKEN_POSIX_SIGSUSPEND
     sigprocmask(SIG_SETMASK, &set, &oset);
     pause();
@@ -366,11 +370,15 @@
 # ifdef BSD_SIGNALS
     sigset_t set;
 
-    sigfillset(&set);
-    sigdelset(&set, sig);
-    if (sig2)
-      sigdelset(&set, sig2);
-    ret = sigpause(set);
+    if (isset(TRAPSASYNC)) {
+	sigemptyset(&set);
+    } else {
+	sigfillset(&set);
+	sigdelset(&set, sig);
+	if (sig2)
+	    sigdelset(&set, sig2);
+	ret = sigpause(set);
+    }
 # else
 #  ifdef SYSV_SIGNALS
     ret = sigpause(sig);
@@ -426,7 +434,7 @@
     do_jump = suspend_longjmp;              /* do we need to longjmp to signal_suspend */
     suspend_longjmp = 0;                    /* In case a SIGCHLD somehow arrives       */
 
-    if (sig == SIGCHLD) {                   /* Traps can cause nested child_suspend()  */
+    if (sig == SIGCHLD) {                   /* Traps can cause nested signal_suspend()  */
         if (do_jump)
             jump_to = suspend_jmp_buf;      /* Copy suspend_jmp_buf                    */
     }
@@ -1065,17 +1073,5 @@
     if ((sigtrapped[sig] & ZSIG_IGNORED) || !sigfuncs[sig] || errflag)
 	return;
 
-    /* Adapted from signal queueing in zhandler */
-    if (trap_queueing_enabled && !isset(TRAPSASYNC)) {
-	int temp_rear = ++trap_queue_rear % MAX_QUEUE_SIZE;
-
-	DPUTS(temp_rear == trap_queue_front, "BUG: trap queue full");
-	if (temp_rear != trap_queue_front) {
-	    trap_queue_rear = temp_rear;
-	    trap_queue[trap_queue_rear] = sig;
-	}
-	return;
-    }
-
     dotrapargs(sig, sigtrapped+sig, sigfuncs[sig]);
 }
Index: Src/signals.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/signals.h,v
retrieving revision 1.5
diff -u -r1.5 signals.h
--- Src/signals.h	21 Apr 2004 11:21:34 -0000	1.5
+++ Src/signals.h	30 Apr 2004 21:15:24 -0000
@@ -58,7 +58,6 @@
  
 #define child_block()      signal_block(sigchld_mask)
 #define child_unblock()    signal_unblock(sigchld_mask)
-#define child_suspend(S)   signal_suspend(SIGCHLD, S)
 
 /* ignore a signal */
 #define signal_ignore(S)   signal(S, SIG_IGN)
@@ -101,24 +100,6 @@
 
 #define restore_queue_signals(q) (queueing_enabled = (q))
 
-/*
- * Similar (but simpler) mechanism used for queueing traps.
- * Only needed if NO_TRAPS_ASYNC is set.
- */
-#define queue_traps()	(trap_queueing_enabled++)
-
-#define run_queued_traps() do { \
-    while (trap_queue_front != trap_queue_rear) { /* while traps in queue */ \
-	trap_queue_front = (trap_queue_front + 1) % MAX_QUEUE_SIZE; \
-	dotrap(trap_queue[trap_queue_front]);  /* handle queued trap   */ \
-    } \
-} while (0)
-
-#define dont_queue_traps() do { \
-    trap_queueing_enabled = 0; \
-    run_queued_traps(); \
-} while (0)
-
 /* Make some signal functions faster. */
 
 #ifdef POSIX_SIGNALS

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>
Work: pws@csr.com
Web: http://www.pwstephenson.fsnet.co.uk

Re: PATCH: (3) Re: FreeBSD compatability feature request

[Vincent Stemen]

On Fri, Apr 30, 2004 at 10:26:24PM +0100, Peter Stephenson wrote:
> Patch to apply on top of previous goes to attempt to handle TRAPS_ASYNC
> the way I think is intended.  It's basically an `if' with a
> sigemptyset() inside it, hope no one was expecting anything sophisticated.
> 
> It doesn't fix the behaviour I noted, that without the option any child
> exiting, not just the one the shell is currently expecting, causes traps
> to be run.  Probably that's not a big issue; I should really check the
> wording of POSIX.
> 
> So I think that's it... Vincent?

Hi Peter.

I applied the patch and, unfortunately, it did not seem to change the
behavior at all under FreeBSD.  It still waits for the child to exit
before processing the signal in the parent even though "set -T" is
used.  Does your patch work under Linux using the two small test
scripts, sigtrap, and sigblock, that I posted the other day?

Vincent

-- 
Vincent Stemen
Avoid the VeriSign/Network Solutions domain registration trap!
Read how Network Solutions (NSI) was involved in stealing our domain name.
http://www.InetAddresses.net

Re: PATCH: (3) Re: FreeBSD compatability feature request

[Vincent Stemen]

On Fri, Apr 30, 2004 at 08:45:30PM -0500, Vincent Stemen wrote:
> On Fri, Apr 30, 2004 at 10:26:24PM +0100, Peter Stephenson wrote:
> > Patch to apply on top of previous goes to attempt to handle TRAPS_ASYNC
> > the way I think is intended.  It's basically an `if' with a
> > sigemptyset() inside it, hope no one was expecting anything sophisticated.
> > 
> > It doesn't fix the behaviour I noted, that without the option any child
> > exiting, not just the one the shell is currently expecting, causes traps
> > to be run.  Probably that's not a big issue; I should really check the
> > wording of POSIX.
> > 
> > So I think that's it... Vincent?
> 
> Hi Peter.
> 
> I applied the patch and, unfortunately, it did not seem to change the
> behavior at all under FreeBSD.  It still waits for the child to exit
> before processing the signal in the parent even though "set -T" is
> used.  Does your patch work under Linux using the two small test
> scripts, sigtrap, and sigblock, that I posted the other day?

I take that back.  My mistake.  I accidently tested it running as zsh
so it was not in sh compatibility mode.  It does appear to work
correctly.  Thank you very much.  I will do some more testing with the
FreeBSD init scripts and let you know if I encounter any other
problems.

Regards,
Vincent

-- 
Vincent Stemen
Avoid the VeriSign/Network Solutions domain registration trap!
Read how Network Solutions (NSI) was involved in stealing our domain name.
http://www.InetAddresses.net

Re: PATCH: (3) - FreeBSD compatability issue resolved

[Vincent Stemen]

On Fri, Apr 30, 2004 at 10:26:24PM +0100, Peter Stephenson wrote:
> Patch to apply on top of previous goes to attempt to handle TRAPS_ASYNC
> the way I think is intended.  It's basically an `if' with a
> sigemptyset() inside it, hope no one was expecting anything sophisticated.
> 
> It doesn't fix the behaviour I noted, that without the option any child
> exiting, not just the one the shell is currently expecting, causes traps
> to be run.  Probably that's not a big issue; I should really check the
> wording of POSIX.
> 
> So I think that's it... Vincent?

Just following up with a status report.  I did further testing, running the
FreeBSD boot and shutdown scripts and, so far as I can tell, everything is
working great as a plug in replacement for /bin/sh now.  Nice work!

The only minor thing I noticed is some warnings I get when I compile:

parameter.c: In function `scanpmparameters':
parameter.c:191: warning: dereferencing type-punned pointer will break strict-aliasing rules
parameter.c: In function `scanpmcommands':
parameter.c:330: warning: dereferencing type-punned pointer will break strict-aliasing rules

...etc, etc.


I saw in the list archive, that you discussed this back in February.  I did
not know the status of it so I thought I would mention it in case you were
unaware these warnings were still being produced under FreeBSD.  I am
compiling with gcc-3.2.2.

Thanks again, Peter, for quickly addressing the FreeBSD sh compatibility
signal issue.

Best regards,
Vincent

-- 
Vincent Stemen
Avoid the VeriSign/Network Solutions domain registration trap!
Read how Network Solutions (NSI) was involved in stealing our domain name.
http://www.InetAddresses.net

Re: PATCH: (3) - FreeBSD compatability issue resolved

[Peter Stephenson]

Vincent Stemen wrote:
> Just following up with a status report.  I did further testing, running the
> FreeBSD boot and shutdown scripts and, so far as I can tell, everything is
> working great as a plug in replacement for /bin/sh now.  Nice work!

Splendid.

> The only minor thing I noticed is some warnings I get when I compile:
> 
> parameter.c: In function `scanpmparameters':
> parameter.c:191: warning: dereferencing type-punned pointer will break strict
> -aliasing rules
> parameter.c: In function `scanpmcommands':
> parameter.c:330: warning: dereferencing type-punned pointer will break strict
> -aliasing rules
> 
> ...etc, etc.

Yes, we really need to do something... however, I'm not yet convinced
anyone quite knows what.  Wayne's theory is that some casts to (void *)
are good enough.  Alternatively, --fno-strict-aliasing would probably be
OK.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************