brucee@runx.ips.oz (Bruce Evans) (09/27/88)
While testing some shell modifications recently, I ran into and fixed
some bugs in signal handling. Try running
while :; echo ok; done
and interrupting it with a SIGINT. It will probably hang within 10 tries.
The F1 dump will probably show the shell stuck with flag 10 (signal
PENDING). Another SIGINT may free the shell.
Due to another bug in my own kernel changes, the signal bug crashed the
system completely, so I had to fix everything. (Mine was readying a
process with signals pending).
(1) In lib/signal.c, there is a race which can lead to the signal catcher
calling location 0 or 1 corresponding to SIG_DFL or SIG_IGN. This is
because the "address" of the new signal handler is installed in vectab[]
too early, so switching from a catcher to a non-function may cause
trouble. The solution is to keep the old address longer. There is also
a problem switching in the opposite direction, solved by installing the
catcher early. It is possible to avoid the system call completely when
switching from a catcher to another catcher, but I didn't try this since
it looked like the code would be larger.
I think this bug may be hard to duplicate but it was reliable in combination
with my local bug.
(2) In kernel/system.c, the (new in 1.3) signal PENDING flag is not managed
properly. In the example, MM spends a lot of time forking and a signal in
the middle can lead to the PENDING flag being copied. This messes up the
count of PENDING flags in sig_procs, at least. It turns out to be correct
to avoid copying the flag, since bumping sig_procs gives an extra signal
after the first one is propagated through the process group.
A quick look at other assignments of the flags showed the one in do_xit()
which may invalidate sig_procs. MM could be just about to call sys_exit()
when a signal arrives, so the test is necessary.
(3) This is not quite a bug. In kernel/proc.c/mini_rec(), after inform()
is called, the current process remains active, even though MM has just
been readied and a user process is running. This violates the priorities
and delays the signal handling unnecessarily. I just call pick_proc().
Perhaps pick_proc() should be called at the end of sendrec() instead of
after unready(), so it picks up high priority processes activated by
ready() without being called twice.
#! /bin/sh
# Contents: proc.c.cdif signal.c.cdif system.c.cdif
# Wrapped by sys@besplex on Sun Sep 25 21:23:52 1988
PATH=/bin:/usr/bin:/usr/ucb ; export PATH
if test -f 'proc.c.cdif' -a "${1}" != "-c" ; then
echo shar: Will not clobber existing file \"'proc.c.cdif'\"
else
echo shar: Extracting \"'proc.c.cdif'\" \(688 characters\)
sed "s/^X//" >'proc.c.cdif' <<'END_OF_FILE'
X*** /user/sys/kernel/proc.c Wed Aug 3 21:18:45 1988
X--- proc.c Sun Sep 25 21:00:15 1988
X***************
X*** 241,247 ****
X /* If MM has just blocked and there are kernel signals pending, now is the
X * time to tell MM about them, since it will be able to accept the message.
X */
X! if (sig_procs > 0 && caller == MM_PROC_NR && src == ANY) inform();
X return(OK);
X }
X
X--- 241,250 ----
X /* If MM has just blocked and there are kernel signals pending, now is the
X * time to tell MM about them, since it will be able to accept the message.
X */
X! if (sig_procs > 0 && caller == MM_PROC_NR && src == ANY) {
X! inform();
X! pick_proc();
X! }
X return(OK);
X }
X
END_OF_FILE
if test 688 -ne `wc -c <'proc.c.cdif'`; then
echo shar: \"'proc.c.cdif'\" unpacked with wrong size!
fi
# end of 'proc.c.cdif'
fi
if test -f 'signal.c.cdif' -a "${1}" != "-c" ; then
echo shar: Will not clobber existing file \"'signal.c.cdif'\"
else
echo shar: Extracting \"'signal.c.cdif'\" \(1018 characters\)
sed "s/^X//" >'signal.c.cdif' <<'END_OF_FILE'
X*** /user/sys/lib/signal.c Wed Aug 3 21:37:26 1988
X--- signal.c Sun Sep 25 20:42:36 1988
X***************
X*** 19,29 ****
X int r,(*old)();
X
X old = vectab[signr - 1];
X! vectab[signr - 1] = func;
X! M.m6_i1 = signr;
X! M.m6_f1 = ( (func == SIG_IGN || func == SIG_DFL) ? func : begsig);
X! r = callx(MM, SIGNAL);
X! if (r == 1)
X! old = SIG_IGN;
X! return( (r < 0 ? (int (*)()) r : old) );
X! }
X--- 19,40 ----
X int r,(*old)();
X
X old = vectab[signr - 1];
X! M.m6_i1 = signr;
X! if (func == SIG_IGN || func == SIG_DFL)
X! /* keep old signal catcher until it is completely de-installed */
X! M.m6_f1 = func;
X! else {
X! /* use new signal catcher immediately (old one may not exist) */
X! vectab[signr - 1] = func;
X! M.m6_f1 = begsig;
X! }
X! r = callx(MM, SIGNAL);
X! if (r < 0) {
X! vectab[signr - 1] = old; /* undo any pre-installation */
X! return( (int (*)()) r );
X! }
X! vectab[signr - 1] = func; /* redo any pre-installation */
X! if (r == 1)
X! return(SIG_IGN);
X! return(old);
X! }
END_OF_FILE
if test 1018 -ne `wc -c <'signal.c.cdif'`; then
echo shar: \"'signal.c.cdif'\" unpacked with wrong size!
fi
# end of 'signal.c.cdif'
fi
if test -f 'system.c.cdif' -a "${1}" != "-c" ; then
echo shar: Will not clobber existing file \"'system.c.cdif'\"
else
echo shar: Extracting \"'system.c.cdif'\" \(670 characters\)
sed "s/^X//" >'system.c.cdif' <<'END_OF_FILE'
X*** /user/sys/kernel/system.c Wed Aug 3 21:18:46 1988
X--- system.c Sun Sep 25 21:11:27 1988
X***************
X*** 136,141 ****
X--- 136,143 ----
X while (bytes--) *dptr++ = *sptr++; /* copy parent struct to child */
X
X rpc->p_flags |= NO_MAP; /* inhibit the process from running */
X+ rp->p_flags &= ~PENDING; /* only one in group should have PENDING */
X+ rp->p_pending = 0;
X rpc->p_pid = pid; /* install child's pid */
X rpc->p_reg[RET_REG] = 0; /* child sees pid = 0 to know it is child */
X
X***************
X*** 271,276 ****
X--- 273,279 ----
X }
X }
X }
X+ if (rc->p_flags & PENDING) --sig_procs;
X rc->p_flags = P_SLOT_FREE;
X return(OK);
X }
END_OF_FILE
if test 670 -ne `wc -c <'system.c.cdif'`; then
echo shar: \"'system.c.cdif'\" unpacked with wrong size!
fi
# end of 'system.c.cdif'
fi
echo shar: End of shell archive.
exit 0
Bruce Evans
Internet: brucee@runx.ips.oz.au UUCP: uunet!runx.ips.oz.au!brucee