Date: Wed, 2 Jul 2003 21:51:48 -0400 From: Mike Makonnen <mtm@identd.net> To: freebsd-threads@FreeBSD.Org Subject: kernel signal bug Message-ID: <20030703015148.GA1593@kokeb.ambesa.net>
next in thread | raw e-mail | index | archive | help
--PNTmBPCT7hxwcZjr Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi guys, I've tracked down the cause of an apparent deadlock in libthr, and the problem is that signals targeted at a specific thread were sometimes not delivered to it. For example, if Thread1 sends a signal to Thread2, but Thread2 has that signal masked, the signal is instead posted to the process. If another threads unmasks the signal before Thread2 it will receive the signal. This is clearly wrong according to POSIX. A signal sent to a specific thread must be delivered to that thread. This is problematic for us because thr and kse disagree on what 'thread' means :-) To solve the problem correctly I'm going to need the help of the KSE guys in formulating a solution that takes into account the difference in thread mapping between thr and kse. In the meantime; however, I would like to commit the following patch which solves the problem well enough. I would like to know if this breaks anything for KSE. There is one problem that I know of with the patch: when a signal is reposted for some reason, it is posted back to the thread that handled it because the information about the target of the signal has been lost by this time. I think this is an acceptable compromise for the moment. As I said, solving the problem correctly will require that we take into account both thr and kse. Cheers. -- Mike Makonnen | GPG-KEY: http://www.identd.net/~mtm/mtm.asc mtm@identd.net | D228 1A6F C64E 120A A1C9 A3AA DAE1 E2AF DBCC 68B9 mtm@FreeBSD.Org| FreeBSD - The Power To Serve --PNTmBPCT7hxwcZjr Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="tdsignal.diff" Index: sys/kern/kern_sig.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_sig.c,v retrieving revision 1.247 diff -u -r1.247 kern_sig.c --- sys/kern/kern_sig.c 28 Jun 2003 08:29:04 -0000 1.247 +++ sys/kern/kern_sig.c 2 Jul 2003 22:45:34 -0000 @@ -95,7 +95,7 @@ static struct thread *sigtd(struct proc *p, int sig, int prop); static int kern_sigtimedwait(struct thread *td, sigset_t set, siginfo_t *info, struct timespec *timeout); -static void do_tdsignal(struct thread *td, int sig); +static void do_tdsignal(struct thread *td, int sig, sigtarget_t target); struct filterops sig_filtops = { 0, filt_sigattach, filt_sigdetach, filt_signal }; @@ -761,7 +761,7 @@ /* Repost if we got an error. */ if (error && info.si_signo) { PROC_LOCK(td->td_proc); - tdsignal(td, info.si_signo); + tdsignal(td, info.si_signo, SIGTARGET_TD); PROC_UNLOCK(td->td_proc); } return (error); @@ -800,7 +800,7 @@ /* Repost if we got an error. */ if (error && info.si_signo) { PROC_LOCK(td->td_proc); - tdsignal(td, info.si_signo); + tdsignal(td, info.si_signo, SIGTARGET_TD); PROC_UNLOCK(td->td_proc); } else { td->td_retval[0] = info.si_signo; @@ -831,7 +831,7 @@ /* Repost if we got an error. */ if (error && info.si_signo) { PROC_LOCK(td->td_proc); - tdsignal(td, info.si_signo); + tdsignal(td, info.si_signo, SIGTARGET_TD); PROC_UNLOCK(td->td_proc); } else { td->td_retval[0] = info.si_signo; @@ -1538,7 +1538,7 @@ mtx_unlock(&ps->ps_mtx); p->p_code = code; /* XXX for core dump/debugger */ p->p_sig = sig; /* XXX to verify code */ - tdsignal(td, sig); + tdsignal(td, sig, SIGTARGET_TD); } PROC_UNLOCK(p); } @@ -1607,21 +1607,21 @@ */ td = sigtd(p, sig, prop); - tdsignal(td, sig); + tdsignal(td, sig, SIGTARGET_P); } /* * MPSAFE */ void -tdsignal(struct thread *td, int sig) +tdsignal(struct thread *td, int sig, sigtarget_t target) { sigset_t saved; struct proc *p = td->td_proc; if (p->p_flag & P_SA) saved = p->p_siglist; - do_tdsignal(td, sig); + do_tdsignal(td, sig, target); if ((p->p_flag & P_SA) && !(p->p_flag & P_SIGEVENT)) { if (SIGSETEQ(saved, p->p_siglist)) return; @@ -1634,7 +1634,7 @@ } static void -do_tdsignal(struct thread *td, int sig) +do_tdsignal(struct thread *td, int sig, sigtarget_t target) { struct proc *p; register sig_t action; @@ -1656,12 +1656,17 @@ /* * If this thread is blocking this signal then we'll leave it in the - * proc so that we can find it in the first thread that unblocks it. + * proc so that we can find it in the first thread that unblocks + * it-- unless the signal is meant for the thread and not the process. */ - if (SIGISMEMBER(td->td_sigmask, sig)) - siglist = &p->p_siglist; - else + if (target == SIGTARGET_TD) { siglist = &td->td_siglist; + } else { + if (SIGISMEMBER(td->td_sigmask, sig)) + siglist = &p->p_siglist; + else + siglist = &td->td_siglist; + } /* * If proc is traced, always give parent a chance; Index: sys/kern/kern_thr.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_thr.c,v retrieving revision 1.10 diff -u -r1.10 kern_thr.c --- sys/kern/kern_thr.c 11 Jun 2003 00:56:57 -0000 1.10 +++ sys/kern/kern_thr.c 3 Jul 2003 01:12:42 -0000 @@ -258,11 +258,7 @@ goto out; } - /* - * We need a way to force this to go into this thread's siglist. - * Until then blocked signals will go to the proc. - */ - tdsignal(ttd, uap->sig); + tdsignal(ttd, uap->sig, SIGTARGET_TD); out: PROC_UNLOCK(p); Index: sys/kern/kern_thread.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_thread.c,v retrieving revision 1.148 diff -u -r1.148 kern_thread.c --- sys/kern/kern_thread.c 30 Jun 2003 10:04:04 -0000 1.148 +++ sys/kern/kern_thread.c 2 Jul 2003 22:37:28 -0000 @@ -413,7 +413,7 @@ if (sig > 0) { td2->td_flags &= ~TDF_INTERRUPT; mtx_unlock_spin(&sched_lock); - tdsignal(td2, sig); + tdsignal(td2, sig, SIGTARGET_P); } else if (sig == 0) { mtx_unlock_spin(&sched_lock); } else { Index: sys/sys/signalvar.h =================================================================== RCS file: /home/ncvs/src/sys/sys/signalvar.h,v retrieving revision 1.62 diff -u -r1.62 signalvar.h --- sys/sys/signalvar.h 14 May 2003 15:00:24 -0000 1.62 +++ sys/sys/signalvar.h 2 Jul 2003 22:47:00 -0000 @@ -205,6 +205,13 @@ #ifdef _KERNEL +/* + * Specifies the target of a signal. + * P - Doesn't matter which thread it gets delivered to. + * TD - Must be delivered to a specific thread. + */ +typedef enum sigtarget_enum { SIGTARGET_TD, SIGTARGET_P } sigtarget_t; + /* Return nonzero if process p has an unmasked pending signal. */ #define SIGPENDING(td) \ (!SIGISEMPTY((td)->td_siglist) && \ @@ -267,7 +274,7 @@ int sig_ffs(sigset_t *set); void siginit(struct proc *p); void signotify(struct thread *td); -void tdsignal(struct thread *td, int sig); +void tdsignal(struct thread *td, int sig, sigtarget_t target); void trapsignal(struct thread *td, int sig, u_long code); /* --PNTmBPCT7hxwcZjr--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20030703015148.GA1593>