Date: Mon, 07 Oct 2002 14:54:42 -0400 (EDT) From: John Baldwin <jhb@FreeBSD.org> To: Don Lewis <dl-freebsd@catspoiler.org> Cc: arch@FreeBSD.ORG, jmallett@FreeBSD.ORG Subject: Re: [jmallett@FreeBSD.org: [PATCH] Reliable signal queues, etc., Message-ID: <XFMail.20021007145442.jhb@FreeBSD.org> In-Reply-To: <200210051000.g95A0ZvU023752@gw.catspoiler.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 05-Oct-2002 Don Lewis wrote:
> On 5 Oct, Juli Mallett wrote:
>
>> diff -Nrdu -x *CVS* -x *dev* sys/kern/kern_exit.c kernel/kern/kern_exit.c
>> --- sys/kern/kern_exit.c Tue Oct 1 12:15:51 2002
>> +++ kernel/kern/kern_exit.c Sat Oct 5 01:20:57 2002
>
>> @@ -209,12 +210,12 @@
>> PROC_LOCK(p);
>> if (p == p->p_leader) {
>> q = p->p_peers;
>> + PROC_UNLOCK(p);
>> while (q != NULL) {
>> - PROC_LOCK(q);
>> psignal(q, SIGKILL);
>> - PROC_UNLOCK(q);
>> q = q->p_peers;
>> }
>> + PROC_LOCK(p);
>> while (p->p_peers)
>> msleep(p, &p->p_mtx, PWAIT, "exit1", 0);
>> }
>
> This scary looking fragment of code in exit1() is relying on the lock on
> p->p_leader being continuously held to prevent the p_peers list from
> changing while the list traversal is in progress. The code in
> kern_fork.c and elsewhere in kern_exit.c holds a lock on p_leader while
> the list modifications are done.
>
> The existing code looks like it could deadlock if q is locked because it
> is in fork() or exit(). Process p will block when it tries to lock q,
> and q will block when it tries to lock its p_leader, which happens to be
> p.
Ugh. Probably the code should be changed to do something like this:
--- kern_exit.c 2 Oct 2002 23:12:01 -0000 1.181
+++ kern_exit.c 7 Oct 2002 18:48:18 -0000
@@ -203,17 +203,18 @@
*/
p->p_flag |= P_WEXIT;
- PROC_UNLOCK(p);
/* Are we a task leader? */
- PROC_LOCK(p);
if (p == p->p_leader) {
q = p->p_peers;
while (q != NULL) {
+ nq = q->p_peers;
+ PROC_UNLOCK(p);
PROC_LOCK(q);
psignal(q, SIGKILL);
PROC_UNLOCK(q);
- q = q->p_peers;
+ PROC_LOCK(p);
+ q = nq;
}
while (p->p_peers)
msleep(p, &p->p_mtx, PWAIT, "exit1", 0);
Also, we might should check P_WEXIT and abort in fork1() if it is
set. (We don't appear to do that presently.)
--
John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!" - http://www.FreeBSD.org/
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.20021007145442.jhb>