Re: [PATCH] stdlib: Make __cxa_thread_atexit_impl lock-free (BZ 15686)

Hi!

I attached a minimal reproducible deadlock to BZ 15686.

https://sourceware.org/bugzilla/show_bug.cgi?id=15686

Unfortunately, this fix is not sufficient to resolve the issue.

You suggested trying it here:
https://inbox.sourceware.org/libc-alpha/[email protected]/

I've done the testing:

https://inbox.sourceware.org/libc-alpha/[email protected]/

30.07.2026 21:26, Adhemerval Zanella:
> __cxa_thread_atexit_impl takes dl_load_lock to protect the
> _dl_find_dso_for_object lookup and the l_tls_dtor_count increment
> against a racing dlclose.  This deadlocks when the function is
> reached from a thread spawned by an ELF constructor, because dlopen
> runs constructors with dl_load_lock held.
>
> Instead of releasing dl_load_lock around constructor execution
> (which requires per-map serialization of the constructor calls and
> re-auditing all the state accessed by dl_open_worker after the
> initializers run), remove the lock acquisition by using
> _dl_find_object instead.
>
> The lock is not required for correctness, DSO_SYMBOL is the address
> of the caller's __dso_handle, so the calling thread is executing
> code of the object being looked up.  A dlclose that unloaded the
> object concurrently would unmap the running code itself, which is
> undefined regardless of the lock.
>
> The per-thread dso_symbol_cache/lm_cache is also removed, the cache
> key was never updated.
>
> Using _dl_find_object also shows a slight better performance, it
> replaces a O(n) by a lock-free O(log n) lookup.
>
> This does not fully fix BZ 15686.  dlopen still runs ELF
> constructors with dl_load_lock held, so a thread spawned by a
> constructor that the constructor then joins still deadlocks if it
> calls dlopen and related functions; or if it lazily binds to a
> symbol defined in another dlopen'ed object (which reaches
> add_dependency in elf/dl-lookup.c).  Only the TLS destructor
> registration path is addressed here.
>
> Checked on aarch64-linux-gnu, x86_64-linux-gnu, i686-linux-gnu, and
> arm-linux-gnueabihf.
> ---
>   stdlib/cxa_thread_atexit_impl.c  | 81 +++++++++++++++++---------------
>   sysdeps/pthread/Makefile         |  6 +++
>   sysdeps/pthread/tst-create2.c    | 52 ++++++++++++++++++++
>   sysdeps/pthread/tst-create2mod.c | 51 ++++++++++++++++++++
>   4 files changed, 152 insertions(+), 38 deletions(-)
>   create mode 100644 sysdeps/pthread/tst-create2.c
>   create mode 100644 sysdeps/pthread/tst-create2mod.c
>
> diff --git a/stdlib/cxa_thread_atexit_impl.c b/stdlib/cxa_thread_atexit_impl.c
> index c4382bc9efe..c162461afb5 100644
> --- a/stdlib/cxa_thread_atexit_impl.c
> +++ b/stdlib/cxa_thread_atexit_impl.c
> @@ -25,15 +25,24 @@
>      combinations of all three functions are the link map list, a link map for a
>      DSO and the link map member l_tls_dtor_count.
>   
> -   __cxa_thread_atexit_impl acquires the dl_load_lock before accessing any
> -   shared state and hence multiple of its instances can safely execute
> -   concurrently.
> +   __cxa_thread_atexit_impl does not take dl_load_lock (taking it deadlocks
> +   if this function is reached from a thread spawned by an ELF constructor,
> +   because dlopen runs constructors with dl_load_lock held).  It locates the
> +   caller's link map with _dl_find_object, which is async-signal-safe and
> +   lock-free, and then increments l_tls_dtor_count atomically.
>   
> -   _dl_close_worker acquires the dl_load_lock before accessing any shared state
> -   as well and hence can concurrently execute multiple of its own instances as
> -   well as those of __cxa_thread_atexit_impl safely.  Not all accesses to
> -   l_tls_dtor_count are protected by the dl_load_lock, so we need to
> -   synchronize using atomics.
> +   Not taking the lock is safe because DSO_SYMBOL is the address of the
> +   caller's __dso_handle, so the calling thread is executing code of the very
> +   object.  A concurrent dlclose that unloads the object while this function
> +   runs would unmap the caller's code as well, which is undefined.  A
> +   conforming program must ensure, via its own synchronization, that the
> +   object stays loaded across this call, and that same synchronization
> +   publishes the l_tls_dtor_count increment to any subsequent
> +   _dl_close_worker.
> +
> +   _dl_close_worker acquires the dl_load_lock before accessing any shared
> +   state.  Not all accesses to l_tls_dtor_count are protected by the
> +   dl_load_lock, so we need to synchronize using atomics.
>   
>      __call_tls_dtors accesses the l_tls_dtor_count without taking the lock; it
>      decrements the value by one.  It does not need the big lock because it does
> @@ -63,7 +72,9 @@
>   
>      Concurrent executions of __call_tls_dtors should only ensure that the value
>      is accessed atomically; no reordering constraints need to be considered.
> -   Likewise for the increment of l_tls_dtor_count in __cxa_thread_atexit_impl.
> +   The same holds for the increment in __cxa_thread_atexit_impl, whose
> +   ordering against _dl_close_worker is provided by the caller as described
> +   above.
>   
>      There is still a possibility on concurrent execution of _dl_close_worker and
>      __call_tls_dtors where _dl_close_worker reads the value of l_tls_dtor_count
> @@ -72,6 +83,7 @@
>      is not very different from a case where __call_tls_dtors is called after
>      _dl_close_worker on the DSO and hence is an accepted execution.  */
>   
> +#include <dlfcn.h>
>   #include <stdio.h>
>   #include <stdlib.h>
>   #include <ldsodefs.h>
> @@ -88,8 +100,6 @@ struct dtor_list
>   };
>   
>   static __thread struct dtor_list *tls_dtor_list;
> -static __thread void *dso_symbol_cache;
> -static __thread struct link_map *lm_cache;
>   
>   /* Register a destructor for TLS variables declared with the 'thread_local'
>      keyword.  This function is only called from code generated by the C++
> @@ -102,42 +112,37 @@ __cxa_thread_atexit_impl (dtor_func func, void *obj, void *dso_symbol)
>   {
>     PTR_MANGLE (func);
>   
> -  /* Prepend.  */
>     struct dtor_list *new = calloc (1, sizeof (struct dtor_list));
>     if (__glibc_unlikely (new == NULL))
>       __libc_fatal ("Fatal glibc error: failed to register TLS destructor: "
>   		  "out of memory\n");
> +
> +  /* A concurrent dlclose may already have reset the link map of a matching
> +     entry, so check for it as well.  Either way the object is being unloaded
> +     from underneath the caller, which is undefined; assume the main program
> +     as for an unrecognized address.  */
> +  struct link_map *map;
> +  struct dl_find_object dfo;
> +  if (GLRO (dl_find_object) (dso_symbol, &dfo) == 0
> +      && dfo.dlfo_link_map != NULL)
> +    map = dfo.dlfo_link_map;
> +  else
> +    map = GL(dl_ns)[LM_ID_BASE]._ns_loaded;
> +
>     new->func = func;
>     new->obj = obj;
> +  new->map = map;
> +
> +  /* This increment is only concurrently observed by the decrement in
> +     __call_tls_dtors and by the load in _dl_close_worker.  For the latter,
> +     the caller's own synchronization with dlclose provides the required
> +     ordering (see CONCURRENCY NOTES), so Relaxed MO is sufficient.  */
> +  atomic_fetch_add_relaxed (&map->l_tls_dtor_count, 1);
> +
> +  /* Prepend.  */
>     new->next = tls_dtor_list;
>     tls_dtor_list = new;
>   
> -  /* We have to acquire the big lock to prevent a racing dlclose from pulling
> -     our DSO from underneath us while we're setting up our destructor.  */
> -  __rtld_lock_lock_recursive (GL(dl_load_lock));
> -
> -  /* See if we already encountered the DSO.  */
> -  if (__glibc_unlikely (dso_symbol_cache != dso_symbol))
> -    {
> -      ElfW(Addr) caller = (ElfW(Addr)) dso_symbol;
> -
> -      struct link_map *l = _dl_find_dso_for_object (caller);
> -
> -      /* If the address is not recognized the call comes from the main
> -	 program (we hope).  */
> -      lm_cache = l ? l : GL(dl_ns)[LM_ID_BASE]._ns_loaded;
> -    }
> -
> -  /* This increment may only be concurrently observed either by the decrement
> -     in __call_tls_dtors since the other l_tls_dtor_count access in
> -     _dl_close_worker is protected by the dl_load_lock.  The execution in
> -     __call_tls_dtors does not really depend on this value beyond the fact that
> -     it should be atomic, so Relaxed MO should be sufficient.  */
> -  atomic_fetch_add_relaxed (&lm_cache->l_tls_dtor_count, 1);
> -  __rtld_lock_unlock_recursive (GL(dl_load_lock));
> -
> -  new->map = lm_cache;
> -
>     return 0;
>   }
>   
> diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile
> index d0f3cd59ac6..5b062094ec4 100644
> --- a/sysdeps/pthread/Makefile
> +++ b/sysdeps/pthread/Makefile
> @@ -349,6 +349,7 @@ tests += \
>     tst-atfork3 \
>     tst-atfork4 \
>     tst-create1 \
> +  tst-create2 \
>     tst-fini1 \
>     tst-pt-tls4 \
>     # tests
> @@ -365,6 +366,7 @@ modules-names += \
>     tst-atfork3mod \
>     tst-atfork4mod \
>     tst-create1mod \
> +  tst-create2mod \
>     tst-fini1mod \
>     tst-stack2-mod \
>     tst-tls4moda \
> @@ -540,6 +542,10 @@ LDFLAGS-tst-create1 = -Wl,-export-dynamic
>   $(objpfx)tst-create1: $(shared-thread-library)
>   $(objpfx)tst-create1.out: $(objpfx)tst-create1mod.so
>   
> +$(objpfx)tst-create2: $(shared-thread-library)
> +$(objpfx)tst-create2mod.so: $(libsupport) $(shared-thread-library)
> +$(objpfx)tst-create2.out: $(objpfx)tst-create2mod.so
> +
>   $(objpfx)tst-stack2.out: $(objpfx)tst-stack2-mod.so
>   $(objpfx)tst-stack2-mod.so: $(shared-thread-library)
>   LDFLAGS-tst-stack2-mod.so = -Wl,-z,execstack
> diff --git a/sysdeps/pthread/tst-create2.c b/sysdeps/pthread/tst-create2.c
> new file mode 100644
> index 00000000000..b5c81dc64ff
> --- /dev/null
> +++ b/sysdeps/pthread/tst-create2.c
> @@ -0,0 +1,52 @@
> +/* Verify that a thread spawned by a dlopen constructor can register a
> +   TLS destructor without deadlocking (BZ 15686).
> +   Copyright (C) 2026 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +/* thread 1: dlopen -> ctor -> pthread_create (worker) -> pthread_join
> +   thread 2 (worker): __cxa_thread_atexit_impl -> lock (dl_load_lock)
> +
> +   dl_load_lock is held by thread 1 across the constructor execution, so if
> +   __cxa_thread_atexit_impl acquires it the worker thread blocks forever and
> +   pthread_join in the constructor never returns.  */
> +
> +#include <support/check.h>
> +#include <support/xdlfcn.h>
> +
> +static int
> +do_test (void)
> +{
> +  void *h = xdlopen ("tst-create2mod.so", RTLD_NOW);
> +
> +  /* The worker thread exited before the constructor's pthread_join
> +     returned, so its TLS destructor has already run.  */
> +  int *dtor_done = xdlsym (h, "tst_create2mod_dtor_done");
> +  TEST_COMPARE (*dtor_done, 1);
> +
> +  xdlclose (h);
> +
> +  /* The destructor already ran, so no reference is left on the module's
> +     l_tls_dtor_count and dlclose must have unloaded it.  */
> +  void *h2 = dlopen ("tst-create2mod.so", RTLD_NOW | RTLD_NOLOAD);
> +  TEST_VERIFY (h2 == NULL);
> +  if (h2 != NULL)
> +    xdlclose (h2);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/pthread/tst-create2mod.c b/sysdeps/pthread/tst-create2mod.c
> new file mode 100644
> index 00000000000..3ec31ce15ea
> --- /dev/null
> +++ b/sysdeps/pthread/tst-create2mod.c
> @@ -0,0 +1,51 @@
> +/* Verify that a thread spawned by a dlopen constructor can register a
> +   TLS destructor without deadlocking (BZ 15686).
> +   Copyright (C) 2026 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <stdlib.h>
> +#include <dso_handle.h>
> +#include <support/check.h>
> +#include <support/xthread.h>
> +
> +int tst_create2mod_dtor_done;
> +
> +static void
> +dtor (void *obj)
> +{
> +  *(int *) obj = 1;
> +}
> +
> +/* The module TLS access mirrors the real-world trigger (a C++ thread_local
> +   or Rust thread_local! first access), exercising __tls_get_addr from the
> +   spawned thread as well.  */
> +static __thread int tls_obj;
> +
> +static void *
> +worker (void *closure)
> +{
> +  tls_obj = 1;
> +  TEST_COMPARE (__cxa_thread_atexit_impl (dtor, &tst_create2mod_dtor_done,
> +					  __dso_handle), 0);
> +  return NULL;
> +}
> +
> +static void __attribute__ ((constructor))
> +do_init (void)
> +{
> +  xpthread_join (xpthread_create (NULL, worker, NULL));
> +}