[PATCH v3] elf: Release dl_load_lock before running dlopen constructors (BZ 15686)

[Artem Proskurnev]

19.07.2026 16:52, Adhemerval Zanella Netto:
> This is a *very* complex solution, that refactor a lot of internal code, and
> change external lock assumptions.
>
> I really dislike the glibc.rtld.strict_init_order tunable as an escape hatch
> (even though we did add one for sorting dependencies).  And the XFAIL test
> documenting that constructors of independent DSOs can now interleave means
> this is a potential semantic change that can regress of a lot of usercases.
>
>  From the the reported deadlock, it doesn't really require releasing
> dl_load_lock around constructors at all. The actual blocking site is
> __cxa_thread_atexit_impl, which takes dl_load_lock only to look up the
> caller's link map. Replacing that lookup with the lock-free _dl_find_object
> removes the lock acquisition entirely.
>
> So here is alternative solution [1], that follows this suit. I could not
> check with codeblock because I could not reproduce the issue with the nighly
> build on x86_64, but I have added some tests from your patch as regression
> testcase (while fixing some UB on some).
>
> I checked on ARM and x86 without any regressions or surprises, but it would
> be good to have more testing.
>
> This is *not* a full fix for BZ#15686, the lock/release around ctors would
> require a more complex solution (as you have noticed). But there is the
> also the question whether we really need this to provide it to make full
> C-runtime support within constructors.
>
> PS: I might only reply this thread in two weeks.
>
> [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/bz15686
Thanks for the thorough review and the alternative patch.  I agree the
v3 design is heavier than ideal, and your lock-free __cxa_thread_atexit_impl
is clearly the right call for that specific path -- it removes a lock
acquisition that should never have been there in the first place, and
the cleanup of the dead per-thread dso_symbol_cache/lm_cache is a real
performance improvement on top.

I'd be happy to see your patch merged independently of v3.  They touch
orthogonal code paths and do not conflict.

Where I'd push back is on treating it as a full replacement.  I tested
your branch (azanella/bz15686, 9039a9cb0f) against the original
glycin/gdk-pixbuf reproducer for BZ 15686, and it still deadlocks.
v3 does not.

Worth noting: the lock-free __cxa_thread_atexit_impl approach is
actually the first thing I tried.  It's the obvious minimal fix, and
on paper it should have unblocked the glycin path because the
spawned thread's first thread_local access was assumed to be the
blocking site.  It did not.  The actual blocking site is elsewhere
(see backtrace below), and that's what pushed me towards the more
general "release dl_load_lock around ctor execution" design rather
than continuing to patch individual dl_load_lock callers.  v3 is the
result of running out of narrower approaches that worked on the real
user-visible failure.

The user-visible failure here isn't abstract: this regression breaks
Codeblocks startup on every system that has glycin installed alongside
a recent gdk-pixbuf, which in our ROSA Education deployment means
classroom systems where Codeblocks is the IDE students actually use.
Shipping a glibc that can't start Codeblocks wasn't an option, which
is why v3 takes the broader route even at the cost of more complexity.

== A/B test

Reproducer: a DSO whose constructor calls gdk_pixbuf_new_from_file()
on a PNG, which routes through glycin's sandboxed loader.  System
has libnss_systemd.so.2 installed and active.  Run as:

$NEW_LD --library-path $BUILD_LIBS ./test_dlopen

with timeout 15s.

azanella/bz15686 (9039a9cb0f)  DEADLOCK  exit 124
v3 (release-dl-load-lock...)   PASS      exit 0, PNG 256x256

== Where it deadlocks

Backtrace from the hung tokio blocking-pool worker that glycin spawns
inside the dlopen constructor (captured via sudo gdb attach):

   #0  futex_wait (futex_word=..., expected=2)
           at ../sysdeps/nptl/futex-internal.h:126
   #2  lll_mutex_lock_optimized (mutex=0x...<_rtld_local+1800>)
           at pthread_mutex_lock.c:48
   #4  _dl_open (file="libnss_systemd.so.2", mode=-2147483646,
                caller_dlopen=<module_load+153>, ...)
           at dl-open.c:819
   #5  do_dlopen (...) at dl-libc.c:95
   #9  __libc_dlopen_mode (...) at dl-libc.c:162
   #10 module_load (...) at nss_module.c:187
   #11 __nss_module_load (...) at nss_module.c:302
   #13 __nss_lookup_function (..., fct_name="initgroups_dyn")
           at nsswitch.c:125
   #14 internal_getgrouplist (user="tema", group=500, ...)
           at initgroups.c:95
   #15 getgrouplist (...) at initgroups.c:156
   #16 glycin::<...> () at /lib64/libglycin-2.so.0

The mutex at _rtld_local+1800 is dl_load_lock.

The triggering sequence is:

   1. main thread: dlopen() grabs dl_load_lock
   2. dl_open_worker -> call_dl_init -> libtest_init.so constructor
   3. constructor -> gdk_pixbuf_new_from_file() -> glycin loader
   4. glycin spawns a tokio blocking worker, which calls
      getgrouplist("tema", ...) to compute supplementary groups for
      the sandboxed subprocess credentials
   5. getgrouplist -> __nss_lookup_function(initgroups_dyn) ->
      __nss_module_load -> __libc_dlopen_mode("libnss_systemd.so.2")
   6. _dl_open -> dl_load_lock acquisition -- already held by the
      main thread inside the constructor -- deadlock

This is the exact class of bug described in BZ 15686; it just happens
to fire through the NSS _dl_open path rather than through
__cxa_thread_atexit_impl.  The same deadlock shape arises for any
code path that a constructor-spawned thread can reach and that needs
dl_load_lock: dlsym, _dl_addr (via backtrace), dl_iterate_phdr in
older builds, _dl_find_dso_for_object, recursive dlopen from a
worker thread, and so on.

== Why v3 takes the route it does

The decision to release dl_load_lock around constructor execution
rather than patch each call site is deliberate.  The class of bugs
isn't really "__cxa_thread_atexit_impl takes the lock", it's
"dl_load_lock is held while running arbitrary user code, and arbitrary
user code can do anything that needs dl_load_lock".  Patching each
manifestation produces a perpetual stream of narrow fixes without
removing the underlying invariant that causes the whole class.

Releasing the lock around ctor execution does remove that invariant.
Anything user code (or threads spawned by it) does during the ctor
becomes a legitimate operation, because the lock that those operations
compete for is no longer held by the thread running the ctor.

== On the semantic change concern

The XFAIL test isn't documenting a removed guarantee -- it's documenting
the removal of an implementation accident that was never a guarantee.

POSIX is explicit that dlopen does not provide atomic visibility across
threads: an object opened by dlopen in one thread is only guaranteed to
be visible to another thread after the opener synchronises with it. If
total initialisation order across concurrent dlopens were a real
contract, the same logic would have to extend to constructor side
effects, and POSIX simply doesn't make that promise.  An application
that depends on plugin A's constructor always running before plugin B's
constructor when both are loaded from independent threads needs to
express that with its own mutex or pthread_once, the same way it would
for any other cross-thread ordering constraint.

The tunable exists as an explicit, opt-in escape hatch for the (rare)
case where some downstream binary does turn out to depend on the old
accidental ordering.  Defaulting it to off matches what POSIX and ELF
actually guarantee; defaulting it to on would enshrine a non-standard
invariant that glibc would then have to preserve forever.

== Proposal

- Take your lock-free __cxa_thread_atexit_impl patch.  It's a strict
   improvement: cleaner code, better performance, and it does remove one
   of the deadlock manifestations on its own.
- Keep v3 as the general fix for BZ 15686.  The two patches touch
   different files (__cxa_thread_atexit_impl vs dl-open.c) and don't
   conflict.

If you'd prefer to land them as a single series I can rebase v3 on top
of your patch so the commit graph reads cleanly.  Either way, I think
the path of least friction is treating them as complementary rather
than competing.

== On patch size

One thing worth correcting, since "very complex solution" came up:
stripped of tests and documentation, the actual production code change
in v3 is four files and 91 lines of code (excluding comments and
blank lines).  The headline 15-files/+903 number is dominated by the
test suite (tst-create2..5, ~726 lines across 10 files) and the
tunables documentation (~29 lines of texi).  Those are necessary but
shouldn't drive the complexity perception.

The comment-heavy ratio (117 comments for 91 lines of code) is mostly
the l_init_once / l_init_owner / l_init_pending state machine, where
each transition is spelled out explicitly so that the locking
invariants can be read off the comments without having to reconstruct
them from the code.  I'm happy to tighten the prose if the verbosity
is what's making the patch feel heavier than it is.

Mikhail Novosyolov has also confirmed that v3 fixes ROSA bug 21031 in
our downstream testing -- the same glycin path that breaks Codeblocks
startup on its own.

== Minimal reproducer

For anyone who wants to verify the A/B result above locally, here is
the minimal reproducer.  Three files plus any small PNG named test.png
(any format gdk-pixbuf recognises; if glycin is installed, gdk-pixbuf
will route the PNG through the glycin sandboxed loader and trigger the
deadlock).  Requires gcc and gdk-pixbuf-2.0-devel.

-----8<---------- test_dlopen.c ----------8<-----
/*
  * Minimal reproducer for the glycin dlopen deadlock (BZ 15686).
  *
  * Build:
  *   gcc -o test_dlopen test_dlopen.c -ldl
  */
#include <stdio.h>
#include <stdlib.h>
#include <dlfcn.h>

int main(void)
{
     fprintf(stderr, "[test_dlopen] about to dlopen libtest_init.so ...\n");
     fflush(stderr);

     void *handle = dlopen("./libtest_init.so", RTLD_NOW);
     if (handle) {
         fprintf(stderr, "[test_dlopen] dlopen SUCCESS\n");
         dlclose(handle);
     } else {
         fprintf(stderr, "[test_dlopen] dlopen FAILED: %s\n", dlerror());
         return 1;
     }

     return 0;
}
-----8<---------- end test_dlopen.c ----------8<-----

-----8<---------- test_init.c ----------8<-----
/*
  * Shared library whose constructor triggers the glycin deadlock.
  *
  * Build:
  *   gcc -fPIC -shared -o libtest_init.so test_init.c \
  *       $(pkg-config --cflags --libs gdk-pixbuf-2.0)
  *
  * When dlopen()'d, the constructor calls gdk_pixbuf_new_from_file()
  * on a PNG.  This reaches glycin's sandboxed loader path, which
  * spawns a tokio worker that calls getgrouplist(); the NSS module
  * loader then tries to acquire dl_load_lock, which is held by the
  * dlopen thread running this constructor.  Deadlock.
  */
#include <stdio.h>
#include <gdk-pixbuf/gdk-pixbuf.h>

__attribute__((constructor))
static void load_png_in_constructor(void)
{
     fprintf(stderr, "[test_init] constructor: loading PNG via 
gdk-pixbuf...\n");
     fflush(stderr);

     GError *error = NULL;
     GdkPixbuf *pixbuf = gdk_pixbuf_new_from_file("test.png", &error);

     if (pixbuf) {
         fprintf(stderr, "[test_init] constructor: SUCCESS %dx%d\n",
                 gdk_pixbuf_get_width(pixbuf), 
gdk_pixbuf_get_height(pixbuf));
         g_object_unref(pixbuf);
     } else {
         fprintf(stderr, "[test_init] constructor: FAILED: %s\n",
                 error ? error->message : "unknown error");
         if (error) g_error_free(error);
     }
     fflush(stderr);
}
-----8<---------- end test_init.c ----------8<-----

-----8<---------- Makefile ----------8<-----
CFLAGS  := $(shell pkg-config --cflags gdk-pixbuf-2.0)
LDFLAGS := $(shell pkg-config --libs   gdk-pixbuf-2.0)

all: libtest_init.so test_dlopen

libtest_init.so: test_init.c
     gcc -fPIC -shared -o $@ $< $(CFLAGS) $(LDFLAGS)

test_dlopen: test_dlopen.c
     gcc -o $@ $< -ldl

clean:
     rm -f libtest_init.so test_dlopen
-----8<---------- end Makefile ----------8<-----

Build and run against a given glibc build (replace $BUILD with the
build directory, e.g. /path/to/build):

   make

   timeout 15 \
     $BUILD/elf/ld-linux-x86-64.so.2 \
       --library-path \
$BUILD:$BUILD/nptl:$BUILD/math:$BUILD/dlfcn:$BUILD/rt:$BUILD/resolv:$BUILD/login 
\
       ./test_dlopen

Expected outcomes:

   - Under unpatched master, or under azanella/bz15686: prints
     "[test_init] constructor: loading PNG via gdk-pixbuf..." and then
     hangs; timeout kills it after 15s with exit code 124.
   - Under v3 (release-dl-load-lock-before-running-dlopen-constructors):
     prints "...constructor: SUCCESS 256x256" and exits 0 within a
     second.

To reproduce the NSS-path deadlock specifically, libnss_systemd.so.2
must be installed and active in /etc/nsswitch.conf (it is on every
recent systemd-based distro, which is why this hits general users).
If you can't reproduce the hang on your system and want me to capture
a more detailed trace, let me know and I'll do it on mine.