Hi,
When killing a process, the signal is handled by any thread that
does not block the signal. If all threads block the signal, we
currently deliver it to the main thread. This does not conform to
POSIX. If any thread unblocks the signal, it should be delivered
immediately to this thread. We block it until the main thread
unblocks.
I see the problem as some tests of posixtestsuite hang. Various
people reported similar observations with multi threaded programs
to me.
Solution is to mark such signals at the process instead of one
thread. Then any thread can handle it later. There is a XXX comment
that suggests this behavior, NetBSD also does this.
This diff survived a full regress run on i386.
bluhm
Index: kern/exec_elf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/exec_elf.c,v
retrieving revision 1.149
diff -u -p -r1.149 exec_elf.c
--- kern/exec_elf.c 9 May 2019 22:25:42 -0000 1.149
+++ kern/exec_elf.c 10 May 2019 02:39:07 -0000
@@ -1224,7 +1224,7 @@ coredump_notes_elf(struct proc *p, void
cpi.cpi_signo = p->p_sisig;
cpi.cpi_sigcode = p->p_sicode;
- cpi.cpi_sigpend = p->p_siglist;
+ cpi.cpi_sigpend = p->p_siglist | pr->ps_siglist;
cpi.cpi_sigmask = p->p_sigmask;
cpi.cpi_sigignore = pr->ps_sigacts->ps_sigignore;
cpi.cpi_sigcatch = pr->ps_sigacts->ps_sigcatch;
Index: kern/kern_exit.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_exit.c,v
retrieving revision 1.173
diff -u -p -r1.173 kern_exit.c
--- kern/kern_exit.c 23 Jan 2019 22:39:47 -0000 1.173
+++ kern/kern_exit.c 10 May 2019 02:39:07 -0000
@@ -179,7 +179,7 @@ exit1(struct proc *p, int rv, int flags)
rup = pr->ps_ru;
}
}
- p->p_siglist = 0;
+ p->p_siglist = pr->ps_siglist = 0;
#if NKCOV > 0
kcov_exit(p);
Index: kern/kern_sig.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sig.c,v
retrieving revision 1.229
diff -u -p -r1.229 kern_sig.c
--- kern/kern_sig.c 1 May 2019 06:26:42 -0000 1.229
+++ kern/kern_sig.c 10 May 2019 02:41:20 -0000
@@ -365,6 +365,7 @@ setsigvec(struct proc *p, int signum, st
if (sa->sa_handler == SIG_IGN ||
(sigprop[signum] & SA_IGNORE && sa->sa_handler == SIG_DFL)) {
atomic_clearbits_int(&p->p_siglist, bit);
+ atomic_clearbits_int(&p->p_p->ps_siglist, bit);
if (signum != SIGCONT)
ps->ps_sigignore |= bit; /* easier in psignal */
ps->ps_sigcatch &= ~bit;
@@ -419,6 +420,7 @@ execsigs(struct proc *p)
if (nc != SIGCONT)
ps->ps_sigignore |= mask;
atomic_clearbits_int(&p->p_siglist, mask);
+ atomic_clearbits_int(&p->p_p->ps_siglist, mask);
}
ps->ps_sigact[nc] = SIG_DFL;
}
@@ -472,7 +474,7 @@ int
sys_sigpending(struct proc *p, void *v, register_t *retval)
{
- *retval = p->p_siglist;
+ *retval = p->p_siglist | p->p_p->ps_siglist;
return (0);
}
@@ -875,7 +877,6 @@ psignal(struct proc *p, int signum)
/*
* type = SPROCESS process signal, can be diverted (sigwait())
- * XXX if blocked in all threads, mark as pending in struct process
* type = STHREAD thread signal, but should be propagated if unhandled
* type = SPROPAGATED propagated to this thread, so don't propagate again
*/
@@ -885,6 +886,7 @@ ptsignal(struct proc *p, int signum, enu
int s, prop;
sig_t action;
int mask;
+ int *siglist;
struct process *pr = p->p_p;
struct proc *q;
int wakeparent = 0;
@@ -907,7 +909,7 @@ ptsignal(struct proc *p, int signum, enu
if (pr->ps_flags & PS_COREDUMP && signum == SIGKILL) {
if (pr->ps_single != NULL)
p = pr->ps_single;
- atomic_setbits_int(&p->p_siglist, mask);
+ atomic_setbits_int(&p->p_p->ps_siglist, mask);
return;
}
@@ -962,7 +964,6 @@ ptsignal(struct proc *p, int signum, enu
*/
if (pr->ps_flags & PS_TRACED) {
action = SIG_DFL;
- atomic_setbits_int(&p->p_siglist, mask);
} else {
/*
* If the signal is being ignored,
@@ -992,17 +993,23 @@ ptsignal(struct proc *p, int signum, enu
if (prop & SA_TTYSTOP && pr->ps_pgrp->pg_jobc == 0)
return;
}
-
- atomic_setbits_int(&p->p_siglist, mask);
}
-
- if (prop & SA_CONT)
- atomic_clearbits_int(&p->p_siglist, stopsigmask);
-
+ /*
+ * If delivered to process, mark as pending there. Continue and stop
+ * signals will be propagated to all threads. So they are always
+ * marked at thread level.
+ */
+ siglist = (type == SPROCESS) ? &pr->ps_siglist : &p->p_siglist;
+ if (prop & SA_CONT) {
+ siglist = &p->p_siglist;
+ atomic_clearbits_int(siglist, stopsigmask);
+ }
if (prop & SA_STOP) {
- atomic_clearbits_int(&p->p_siglist, contsigmask);
+ siglist = &p->p_siglist;
+ atomic_clearbits_int(siglist, contsigmask);
atomic_clearbits_int(&p->p_flag, P_CONTINUED);
}
+ atomic_setbits_int(siglist, mask);
/*
* XXX delay processing of SA_STOP signals unless action == SIG_DFL?
@@ -1045,7 +1052,7 @@ ptsignal(struct proc *p, int signum, enu
* be awakened.
*/
if ((prop & SA_CONT) && action == SIG_DFL) {
- atomic_clearbits_int(&p->p_siglist, mask);
+ atomic_clearbits_int(siglist, mask);
goto out;
}
/*
@@ -1059,7 +1066,7 @@ ptsignal(struct proc *p, int signum, enu
*/
if (pr->ps_flags & PS_PPWAIT)
goto out;
- atomic_clearbits_int(&p->p_siglist, mask);
+ atomic_clearbits_int(siglist, mask);
p->p_xstat = signum;
proc_stop(p, 0);
goto out;
@@ -1102,7 +1109,7 @@ ptsignal(struct proc *p, int signum, enu
atomic_clearbits_int(&p->p_flag, P_SUSPSIG);
wakeparent = 1;
if (action == SIG_DFL)
- atomic_clearbits_int(&p->p_siglist, mask);
+ atomic_clearbits_int(siglist, mask);
if (action == SIG_CATCH)
goto runfast;
if (p->p_wchan == 0)
@@ -1116,7 +1123,7 @@ ptsignal(struct proc *p, int signum, enu
* Already stopped, don't need to stop again.
* (If we did the shell could get confused.)
*/
- atomic_clearbits_int(&p->p_siglist, mask);
+ atomic_clearbits_int(siglist, mask);
goto out;
}
@@ -1181,7 +1188,7 @@ issignal(struct proc *p)
int s;
for (;;) {
- mask = p->p_siglist & ~p->p_sigmask;
+ mask = SIGPENDING(p);
if (pr->ps_flags & PS_PPWAIT)
mask &= ~stopsigmask;
if (mask == 0) /* no signal to send */
@@ -1189,6 +1196,7 @@ issignal(struct proc *p)
signum = ffs((long)mask);
mask = sigmask(signum);
atomic_clearbits_int(&p->p_siglist, mask);
+ atomic_clearbits_int(&p->p_p->ps_siglist, mask);
/*
* We should see pending but ignored signals
@@ -1243,6 +1251,7 @@ issignal(struct proc *p)
/* take the signal! */
atomic_clearbits_int(&p->p_siglist, mask);
+ atomic_clearbits_int(&p->p_p->ps_siglist, mask);
}
prop = sigprop[signum];
@@ -1638,7 +1647,8 @@ coredump_write(void *cookie, enum uio_se
csize = len;
do {
- if (io->io_proc->p_siglist & sigmask(SIGKILL))
+ if (sigmask(SIGKILL) &
+ (io->io_proc->p_siglist | io->io_proc->p_p->ps_siglist))
return (EINTR);
/* Rest of the loop sleeps with lock held, so... */
@@ -1740,8 +1750,8 @@ sys___thrsigdivert(struct proc *p, void
if (smask & mask) {
if (p->p_siglist & smask)
m = &p->p_siglist;
- else if (pr->ps_mainproc->p_siglist & smask)
- m = &pr->ps_mainproc->p_siglist;
+ else if (pr->ps_siglist & smask)
+ m = &pr->ps_siglist;
else {
/* signal got eaten by someone else? */
continue;
@@ -1868,7 +1878,7 @@ userret(struct proc *p)
KERNEL_UNLOCK();
}
- if (SIGPENDING(p)) {
+ if (SIGPENDING(p) != 0) {
KERNEL_LOCK();
while ((signum = CURSIG(p)) != 0)
postsig(p, signum);
Index: kern/tty.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/tty.c,v
retrieving revision 1.143
diff -u -p -r1.143 tty.c
--- kern/tty.c 6 Sep 2018 11:50:54 -0000 1.143
+++ kern/tty.c 10 May 2019 02:39:07 -0000
@@ -1676,11 +1676,11 @@ ttycheckoutq(struct tty *tp, int wait)
hiwat = tp->t_hiwat;
s = spltty();
- oldsig = wait ? curproc->p_siglist : 0;
+ oldsig = wait ? SIGPENDING(curproc) : 0;
if (tp->t_outq.c_cc > hiwat + TTHIWATMINSPACE)
while (tp->t_outq.c_cc > hiwat) {
ttstart(tp);
- if (wait == 0 || curproc->p_siglist != oldsig) {
+ if (wait == 0 || SIGPENDING(curproc) != oldsig) {
splx(s);
return (0);
}
Index: nfs/nfs_socket.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/nfs/nfs_socket.c,v
retrieving revision 1.132
diff -u -p -r1.132 nfs_socket.c
--- nfs/nfs_socket.c 9 Nov 2018 14:14:32 -0000 1.132
+++ nfs/nfs_socket.c 10 May 2019 02:39:07 -0000
@@ -1225,9 +1225,8 @@ nfs_sigintr(struct nfsmount *nmp, struct
return (EINTR);
if (!(nmp->nm_flag & NFSMNT_INT))
return (0);
- if (p && p->p_siglist &&
- (((p->p_siglist & ~p->p_sigmask) &
- ~p->p_p->ps_sigacts->ps_sigignore) & NFSINT_SIGMASK))
+ if (p && (SIGPENDING(p) & ~p->p_p->ps_sigacts->ps_sigignore &
+ NFSINT_SIGMASK))
return (EINTR);
return (0);
}
Index: sys/proc.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/proc.h,v
retrieving revision 1.263
diff -u -p -r1.263 proc.h
--- sys/proc.h 6 Jan 2019 12:59:45 -0000 1.263
+++ sys/proc.h 10 May 2019 02:39:07 -0000
@@ -186,6 +186,7 @@ struct process {
#define ps_startzero ps_klist
struct klist ps_klist; /* knotes attached to this process */
int ps_flags; /* PS_* flags. */
+ int ps_siglist; /* Signals pending for the process. */
struct proc *ps_single; /* Single threading to this thread. */
int ps_singlecount; /* Not yet suspended threads. */
Index: sys/signalvar.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/signalvar.h,v
retrieving revision 1.35
diff -u -p -r1.35 signalvar.h
--- sys/signalvar.h 17 Dec 2018 14:51:57 -0000 1.35
+++ sys/signalvar.h 10 May 2019 02:39:07 -0000
@@ -67,24 +67,30 @@ struct sigacts {
/*
* Check if process p has an unmasked signal pending.
+ * Return mask of pending signals.
*/
-#define SIGPENDING(p) (((p)->p_siglist & ~(p)->p_sigmask) != 0)
+#define SIGPENDING(p) \
+ (((p)->p_siglist | (p)->p_p->ps_siglist) & ~(p)->p_sigmask)
/*
* Determine signal that should be delivered to process p, the current
* process, 0 if none. If there is a pending stop signal with default
* action, the process stops in issignal().
*/
-#define CURSIG(p)
\
- (((p)->p_siglist == 0 || \
+#define CURSIG(p) \
+ ((((p)->p_siglist | (p)->p_p->ps_siglist) == 0 || \
(((p)->p_p->ps_flags & PS_TRACED) == 0 && \
- ((p)->p_siglist & ~(p)->p_sigmask) == 0)) ? \
+ SIGPENDING(p) == 0)) ? \
0 : issignal(p))
/*
* Clear a pending signal from a process.
*/
-#define CLRSIG(p, sig) atomic_clearbits_int(&(p)->p_siglist,
sigmask(sig))
+#define CLRSIG(p, sig) do { \
+ int _mask = sigmask(sig); \
+ atomic_clearbits_int(&(p)->p_siglist, _mask); \
+ atomic_clearbits_int(&(p)->p_p->ps_siglist, _mask); \
+} while (0)
/*
* Signal properties and actions.
Index: sys/sysctl.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/sysctl.h,v
retrieving revision 1.185
diff -u -p -r1.185 sysctl.h
--- sys/sysctl.h 9 May 2019 14:59:30 -0000 1.185
+++ sys/sysctl.h 10 May 2019 02:39:07 -0000
@@ -595,7 +595,7 @@ do {
\
(kp)->p_tracep = PTRTOINT64((pr)->ps_tracevp); \
(kp)->p_traceflag = (pr)->ps_traceflag; \
\
- (kp)->p_siglist = (p)->p_siglist; \
+ (kp)->p_siglist = (p)->p_siglist | (pr)->ps_siglist; \
(kp)->p_sigmask = (p)->p_sigmask; \
(kp)->p_sigignore = (sa) ? (sa)->ps_sigignore : 0; \
(kp)->p_sigcatch = (sa) ? (sa)->ps_sigcatch : 0; \
