Skip site navigation (1)Skip section navigation (2)
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>