threads/119920: fork broken in libpthread
Landon Fuller
landonf at bikemonkey.org
Wed Jan 30 11:22:56 PST 2008
On Jan 29, 2008, at 20:23, Daniel Eischen wrote:
> On Tue, 29 Jan 2008, Landon Fuller wrote:
>
>>
>> On Jan 29, 2008, at 13:13, Daniel Eischen wrote:
>>> There is a bug somewhere else or something is stomping
>>> on the thread's lockuser. It is allocated once when the
>>> thread is created and should never be null thereafter.
>>> Hence, it should never be malloc'd and the reinit should
>>> be sufficient.
>>
>> I'm totally unfamiliar with KSE, so perhaps this a stupid question
>> -- it seems to solve the issue locally, so I'll ask it --
>> Why not place the fork() code inside of _kse_critical_enter /
>> _kse_critical_leave, to ensure upcalls are blocked while re-
>> initializing in the child process post-fork?
>
> That just prevents an upcall from happening (which my patch solves),
> but doesn't prevent the corruption of the lockuser or lock.
My assumption was that lockuser was being corrupted in a post-fork
upcall; after wrapping fork in kse_critical_enter(), and can not
reproduce the lockuser-reallocation I was seeing before.
Is my assumption that kse_critical_enter() prevents any other code
from being run during the critical section incorrect?
--- thread/thr_fork.c.orig 2008-01-30 11:08:45.000000000 -0800
+++ thread/thr_fork.c 2008-01-30 11:09:36.000000000 -0800
@@ -93,12 +93,16 @@
if (_kse_isthreaded() != 0) {
_spinlock(__malloc_lock);
}
+
+ kse_critical_t crit = _kse_critical_enter();
+
if ((ret = __sys_fork()) == 0) {
/* Child process */
errsave = errno;
/* Kernel signal mask is restored in
_kse_single_thread */
_kse_single_thread(curthread);
+ _kse_critical_leave(crit);
/* Run down atfork child handlers. */
TAILQ_FOREACH(af, &_thr_atfork_list, qe) {
@@ -107,6 +111,7 @@
}
_thr_mutex_reinit(&_thr_atfork_mutex);
} else {
+ _kse_critical_leave(crit);
if ((_kse_isthreaded() != 0) && (__malloc_lock !=
NULL)) {
_spinunlock(__malloc_lock);
}
> My patch does solve this in -current, but -stable probably lacks
> a few other patches. It (-stable) really needs all of -current's
> code, not just this patch.
Is this change viable for an errata fix for 6.3?
-landonf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PGP.sig
Type: application/pgp-signature
Size: 186 bytes
Desc: This is a digitally signed message part
Url : http://lists.freebsd.org/pipermail/freebsd-threads/attachments/20080130/694b8b61/PGP.pgp
More information about the freebsd-threads
mailing list