Date: Thu, 31 Jan 2008 10:00:18 -0800 From: Julian Elischer <julian@elischer.org> To: Landon Fuller <landonf@bikemonkey.org> Cc: freebsd-threads@freebsd.org Subject: Re: threads/119920: fork broken in libpthread Message-ID: <47A20CB2.1010508@elischer.org> In-Reply-To: <01819294-7795-493D-A054-D4ACEC8706D6@bikemonkey.org> References: <200801240850.m0O8o2JQ023500@freefall.freebsd.org> <4798564B.7070500@elischer.org> <Pine.GSO.4.64.0801240957550.16059@sea.ntplx.net> <488DBC6A-CF33-4E50-B1BB-C396C8957F92@bikemonkey.org> <Pine.GSO.4.64.0801291611130.12689@sea.ntplx.net> <892A73B3-0114-4718-ABC0-CADD45D9D0FA@bikemonkey.org> <Pine.GSO.4.64.0801292320260.15096@sea.ntplx.net> <D3DFD2DE-E6A3-4373-AC37-42C3CAB9B963@bikemonkey.org> <47A0D2DE.9060005@elischer.org> <Pine.GSO.4.64.0801301706010.18132@sea.ntplx.net> <35535E7A-0804-4DAD-B0A0-CCF9EE7060B0@bikemonkey.org> <47A18DCD.2070101@elischer.org> <01819294-7795-493D-A054-D4ACEC8706D6@bikemonkey.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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); > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?47A20CB2.1010508>