Message ID | mvmblmtjmed.fsf@suse.de |
---|---|
State | New |
Headers | show |
Series | nptl: wait for pending setxid request also in detached thread (bug 25942) | expand |
* Andreas Schwab: > There is a race between __nptl_setxid and exiting detached thread, which > causes a deadlock on stack_cache_lock. The deadlock happens in this > state: > > T1: setgroups -> __nptl_setxid (holding stack_cache_lock, waiting on cmdp->cntr == 0) > T2 (detached, exiting): start_thread -> __deallocate_stack (waiting on stack_cache_lock) > more threads waiting on stack_cache_lock in pthread_create > > For non-detached threads, start_thread waits for its own setxid handler to > finish before exiting. Do this for detached threads as well. > --- > nptl/pthread_create.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c > index afd379e89a..a43089065c 100644 > --- a/nptl/pthread_create.c > +++ b/nptl/pthread_create.c > @@ -552,11 +552,7 @@ START_THREAD_DEFN > advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd, > pd->guardsize); > > - /* If the thread is detached free the TCB. */ > - if (IS_DETACHED (pd)) > - /* Free the TCB. */ > - __free_tcb (pd); > - else if (__glibc_unlikely (pd->cancelhandling & SETXID_BITMASK)) > + if (__glibc_unlikely (pd->cancelhandling & SETXID_BITMASK)) > { > /* Some other thread might call any of the setXid functions and expect > us to reply. In this case wait until we did that. */ > @@ -572,6 +568,11 @@ START_THREAD_DEFN > pd->setxid_futex = 0; > } > > + /* If the thread is detached free the TCB. */ > + if (IS_DETACHED (pd)) > + /* Free the TCB. */ > + __free_tcb (pd); > + > /* We cannot call '_exit' here. '_exit' will terminate the process. > > The 'exit' implementation in the kernel will signal when the This patch looks reasonable to me. The history here is this, as far as I can tell: Commit dff9a7a163e9f5e28f36f9a701acf74f8f9d7968 did not have detached threads participating in the signaling (see the !IS_DETACHED in setxid_signal_thread). This was changed ater in commit 25db0f6ca996f799de308aa2dc7c62caa99ee9dc or thereabouts, but the race you fix now was apparently missed. Thanks, Florian
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index afd379e89a..a43089065c 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -552,11 +552,7 @@ START_THREAD_DEFN advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd, pd->guardsize); - /* If the thread is detached free the TCB. */ - if (IS_DETACHED (pd)) - /* Free the TCB. */ - __free_tcb (pd); - else if (__glibc_unlikely (pd->cancelhandling & SETXID_BITMASK)) + if (__glibc_unlikely (pd->cancelhandling & SETXID_BITMASK)) { /* Some other thread might call any of the setXid functions and expect us to reply. In this case wait until we did that. */ @@ -572,6 +568,11 @@ START_THREAD_DEFN pd->setxid_futex = 0; } + /* If the thread is detached free the TCB. */ + if (IS_DETACHED (pd)) + /* Free the TCB. */ + __free_tcb (pd); + /* We cannot call '_exit' here. '_exit' will terminate the process. The 'exit' implementation in the kernel will signal when the