From owner-freebsd-threads@FreeBSD.ORG Thu Jan 31 18:00:20 2008 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 50C2816A46C for ; Thu, 31 Jan 2008 18:00:20 +0000 (UTC) (envelope-from julian@elischer.org) Received: from outI.internet-mail-service.net (outI.internet-mail-service.net [216.240.47.232]) by mx1.freebsd.org (Postfix) with ESMTP id 2A56613C45A for ; Thu, 31 Jan 2008 18:00:20 +0000 (UTC) (envelope-from julian@elischer.org) Received: from mx0.idiom.com (HELO idiom.com) (216.240.32.160) by out.internet-mail-service.net (qpsmtpd/0.40) with ESMTP; Thu, 31 Jan 2008 10:00:19 -0800 Received: from julian-mac.elischer.org (localhost [127.0.0.1]) by idiom.com (Postfix) with ESMTP id AC5E0127081; Thu, 31 Jan 2008 10:00:18 -0800 (PST) Message-ID: <47A20CB2.1010508@elischer.org> Date: Thu, 31 Jan 2008 10:00:18 -0800 From: Julian Elischer User-Agent: Thunderbird 2.0.0.9 (Macintosh/20071031) MIME-Version: 1.0 To: Landon Fuller References: <200801240850.m0O8o2JQ023500@freefall.freebsd.org> <4798564B.7070500@elischer.org> <488DBC6A-CF33-4E50-B1BB-C396C8957F92@bikemonkey.org> <892A73B3-0114-4718-ABC0-CADD45D9D0FA@bikemonkey.org> <47A0D2DE.9060005@elischer.org> <35535E7A-0804-4DAD-B0A0-CCF9EE7060B0@bikemonkey.org> <47A18DCD.2070101@elischer.org> <01819294-7795-493D-A054-D4ACEC8706D6@bikemonkey.org> In-Reply-To: <01819294-7795-493D-A054-D4ACEC8706D6@bikemonkey.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-threads@freebsd.org Subject: Re: threads/119920: fork broken in libpthread X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 31 Jan 2008 18:00:20 -0000 Landon Fuller wrote: > > On Jan 31, 2008, at 12:58 AM, Julian Elischer wrote: > >> Landon Fuller wrote: >> >>> Thanks -- pulling in relevant change from sys/lock.c -- plus your >>> previous patch -- solves my reproduction case: >>> >>> http://landonf.bikemonkey.org/static/code/freebsd/patch-libpthread63-fork >>> >> >> >> is this the patch you used? It's huge.. > > Are you sure you fetched the patch-libpthread63-fork file? I cherry > picked just the changes to _lockuser_reinit() and _kse_single_thread(). hmm maybe I clicked on the wrong file somewhere. > > Posted patch here, inlined below: > http://landonf.bikemonkey.org/static/code/freebsd/patch-libpthread63-fork ok that's more what I was expecting.. lets's see if we can get this in RELENG_6 (and errata for 6.3) and ensure it is in 7.0kjfnb8_k > > > Cheers, > Landon > > diff -ru lib/libpthread.orig/sys/lock.c lib/libpthread/sys/lock.c > --- lib/libpthread.orig/sys/lock.c 2008-01-30 16:30:07.000000000 -0800 > +++ lib/libpthread/sys/lock.c 2008-01-30 16:58:59.000000000 -0800 > @@ -117,14 +117,23 @@ > { > if (lu == NULL) > return (-1); > - /* > - * All lockusers keep their watch request and drop their > - * own (lu_myreq) request. Their own request is either > - * some other lockuser's watch request or is the head of > - * the lock. > - */ > - lu->lu_myreq = lu->lu_watchreq; > - if (lu->lu_myreq == NULL) > + > + if (lu->lu_watchreq != NULL) { > + /* > + * In this case the lock is active. All lockusers > + * keep their watch request and drop their own > + * (lu_myreq) request. Their own request is either > + * some other lockuser's watch request or is the > + * head of the lock. > + */ > + lu->lu_myreq = lu->lu_watchreq; > + lu->lu_watchreq = NULL; > + } > + if (lu->lu_myreq == NULL) > + /* > + * Oops, something isn't quite right. Try to > + * allocate one. > + */ > return (_lockuser_init(lu, priv)); > else { > lu->lu_myreq->lr_locked = 1; > diff -ru lib/libpthread.orig/thread/thr_kern.c > lib/libpthread/thread/thr_kern.c > --- lib/libpthread.orig/thread/thr_kern.c 2008-01-30 > 16:30:07.000000000 -0800 > +++ lib/libpthread/thread/thr_kern.c 2008-01-30 16:55:19.000000000 -0800 > @@ -345,6 +345,17 @@ > _LCK_SET_PRIVATE2(&curthread->kse->k_lockusers[i], NULL); > } > curthread->kse->k_locklevel = 0; > + > + /* > + * Reinitialize the thread and signal locks so that > + * sigaction() will work after a fork(). > + */ > + _lock_reinit(&curthread->lock, LCK_ADAPTIVE, _thr_lock_wait, > + _thr_lock_wakeup); > + _lock_reinit(&_thread_signal_lock, LCK_ADAPTIVE, _kse_lock_wait, > + _kse_lock_wakeup); > + > + > _thr_spinlock_init(); > if (__isthreaded) { > _thr_rtld_fini(); > @@ -354,6 +365,20 @@ > curthread->kse->k_kcb->kcb_kmbx.km_curthread = NULL; > curthread->attr.flags |= PTHREAD_SCOPE_SYSTEM; > > + /* > + * After a fork, it is possible that an upcall occurs in > + * the parent KSE that fork()'d before the child process > + * is fully created and before its vm space is copied. > + * During the upcall, the tcb is set to null or to another > + * thread, and this is what gets copied in the child process > + * when the vm space is cloned sometime after the upcall > + * occurs. Note that we shouldn't have to set the kcb, but > + * we do it for completeness. > + */ > + _kcb_set(curthread->kse->k_kcb); > + _tcb_set(curthread->kse->k_kcb, curthread->tcb); > + > + > /* After a fork(), there child should have no pending signals. */ > sigemptyset(&curthread->sigpend); > >