On Mon, Jul 09, 2018 at 10:59:54PM +0200, Alexander Bluhm wrote:
> On Mon, Jul 09, 2018 at 11:27:55AM -0900, Philip Guenther wrote:
> > Those signals are handled by the first thread that
> > > doesn't have them masked. In that case, it should be put on the pending
> > > list of the process and any unmasking operation should check the pending
> > > list on whether a signal should be delivered delayed.
> > >
> >
> > Yep.
>
> This is my original diff with some twaeks from visa@.
While I think this is a step in the right direction I don't think is the
proper solution to the problem.
I don't think this handles pthread_kill() signals and I agree with mpi@
that the path should be split between thread signals and process signals.
I am tracking this issue myself for almost two weeks now and while
indeed this diff seems to workaround the posixsuite issue, it does still
manifest in complex scenarios like lang/mono.
I will try to come up with something better in the following days.
In the meantime if you guys want to commit this bit, I will not
object.
> Index: kern/kern_sig.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_sig.c,v
> retrieving revision 1.220
> diff -u -p -r1.220 kern_sig.c
> --- kern/kern_sig.c 28 Apr 2018 03:13:04 -0000 1.220
> +++ kern/kern_sig.c 9 Jul 2018 20:36:07 -0000
> @@ -1155,14 +1155,17 @@ 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 */
> return (0);
> signum = ffs((long)mask);
> mask = sigmask(signum);
> - atomic_clearbits_int(&p->p_siglist, mask);
> + if (p->p_siglist & mask)
> + atomic_clearbits_int(&p->p_siglist, mask);
> + else
> + atomic_clearbits_int(&pr->ps_mainproc->p_siglist, mask);
>
> /*
> * We should see pending but ignored signals
> @@ -1840,7 +1843,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: sys/signalvar.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/sys/signalvar.h,v
> retrieving revision 1.30
> diff -u -p -r1.30 signalvar.h
> --- sys/signalvar.h 24 Mar 2018 04:13:59 -0000 1.30
> +++ sys/signalvar.h 9 Jul 2018 20:52:50 -0000
> @@ -68,7 +68,9 @@ struct sigacts {
> /*
> * Check if process p has an unmasked signal pending.
> */
> -#define SIGPENDING(p) (((p)->p_siglist & ~(p)->p_sigmask) != 0)
> +#define SIGPENDING(p)
> \
> + (((p)->p_siglist | (p)->p_p->ps_mainproc->p_siglist) & \
> + ~(p)->p_sigmask)
>
> /*
> * Determine signal that should be delivered to process p, the current
> @@ -76,10 +78,9 @@ struct sigacts {
> * action, the process stops in issignal().
> */
> #define CURSIG(p)
> \
> - (((p)->p_siglist == 0 || \
> - (((p)->p_p->ps_flags & PS_TRACED) == 0 && \
> - ((p)->p_siglist & ~(p)->p_sigmask) == 0)) ? \
> - 0 : issignal(p))
> + (((((p)->p_siglist | (p)->p_p->ps_mainproc->p_siglist) == 0) || \
> + (((p)->p_p->ps_flags & PS_TRACED) == 0 && SIGPENDING(p) == 0)) \
> + ? 0 : issignal(p))
>
> /*
> * Clear a pending signal from a process.
