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

21.07.2026 00:43, Carlos O'Donell:
> (1) Solving the narrower problem as a short-term solution.
>
> Adhemerval's solution seems like a good immediate fix for the problem.
>
> Does it fix the issue?
>
> As a project lead I would like to see Bug 15686 fixed, but if it takes
> us time to get to that solution, we also want to resolve the problem
> that you're seeing in a timely fashion and are willing to accept a 
> short-term
> solutions to help you out.
>
> (2) Solving the architectural problem may take longer or require more 
> testing.
>
> Large changes like those which are being proposed here will take time 
> to review
> and check against existing application expectations.
>
> The implementation is constrained by more than just what the standards
> say, or the glibc manual says, it is constrained by expectations that
> applications may have created.
>
> It should not be this way, but it is, and we attempt to consider such
> constraints when making changes because we don't want to knowingly break
> such applications unless there is a meaningful and valuable gain.
>
> We might be able to run with this change downstream in Fedora and use
> our build infrastructure to do A/B build testing of the distribution 
> to see if
> something comes out of the larger scale testing.
>
> I'm excited to see the results of ROSA Labs running with these changes in
> production and reporting back the results.
>
> (3) POSIX and glibc say nothing about this today --- making this a 
> hard problem.
>
> The problem *is* that we say nothing about what is and is not allowed 
> and have
> permitted the implementation to grow to the point that users expect a 
> completely
> flexible asynchronous processing of dependencies across multiple threads
> allowing calls to all library functionality.
>
> For example, some users expect a malloc interposer can call dlopen, 
> but this
> clearly has to have some constraints. If dlopen needs malloc then it 
> doesn't
> work as the solution has a dependency cycle (discussed previously on 
> list as a
> new form of safety e.g. "synchronous reetrancy").
>
> Since we have never said anything before about this, as Adhemerval 
> notes, this
> area is going to possibly cause problems with existing applications, 
> and for
> which this patch provides a tunable.
>
> It would be better if instead of using a tunable we spent more time 
> evaluating
> the problem space and a solution, hence I think Adhemerval's 
> suggestion to use
> an intermediate solution. This intermediate solution is not a final 
> solution,
> but an option to resolve the short-term problem while we take more 
> time to
> evaluate a full solution.
>
> In truth we say a few things about this in the glibc manual in the 
> Dynamic Linker
> chapter, but only to warn against having any expectations :-)
>
> (4) A full solution requires understanding what we're asking --- and 
> possibly tooling.
>
> Are we asking that all foreign function callbacks can call into the 
> library safely?
>
> Are we only asking that all foreign function callbacks for library 
> initializers and
> finalizers be able to call back into the library safely?
>
> Constructors and destructors are foreign function calls made during 
> library
> initialization and finalization, and as such we have never discussed 
> what is and
> isn't permitted to be called in those functions.
>
> These foreign functions are not occurring in the normal context of the 
> application
> runtime, nor do we provide ways for them to know what *other* non-libc 
> functionality
> can be safely accessed and if it will ever change (based on load order).
>
> Is there any way we can add tooling or _dl_debug_printf options to 
> detect problems?
>
> Example: lari: 
> https://docs.oracle.com/cd/E88353_01/html/E37839/lari-1.html
>
> Should we add the option to randomly order dependencies whose relative 
> order shouldn't
> matter? To shake out problems in development?
>
> I think a solution to this problem is going to take time, particularly 
> to evaluate
> the effect on existing applications and runtimes.
>
> Thoughts on (1), (2), (3) or (4)?
>
Thanks for the thoughtful response.  Replying to your four points in
the order that I think moves the discussion forward fastest: (1)
first because it is the easiest to settle factually, (3) next
because it is the most actionable, then (2) and (4).

== (1) Does Adhemerval's fix solve the reported issue?

No.  I A/B tested it before sending v3, and the result is in my reply
to Adhemerval earlier in this thread.  Repeating the headline for
directness:

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

The blocking site is getgrouplist -> __nss_lookup_function ->
__nss_module_load -> __libc_dlopen_mode -> _dl_open -> dl_load_lock,
not __cxa_thread_atexit_impl.  Adhemerval's patch removes one
acquisition site but leaves the rest of the dl_load_lock surface
intact, and the glycin reproducer fires through one of the sites
that is left intact.

I would still like to see Adhemerval's patch merged independently.
It is a strict improvement on the path it touches, the cleanup of
the per-thread dso_symbol_cache/lm_cache is real, and it does not
conflict with v3.  My position is "both, not either-or".

== (3) POSIX is silent, tunable is not ideal

I agree the tunable is not ideal, and your concern is well-founded:
a runtime knob becomes permanent API surface the moment it ships,
and we should not enshrine a non-standard invariant just to hedge
against an unknown regression.  Two options, in order of my
preference:

(a) Keep the tunable, but ship it undocumented and explicitly marked
deprecated from day one.  The manual would say nothing about it;
the tunables list would carry a one-line entry flagged "deprecated,
escape hatch for unforeseen regressions, scheduled for removal".
If no regression surfaces within one release cycle, it gets removed.
If one does, we have a concrete report to discuss rather than a
hypothetical.  This keeps the escape hatch while making it clear
that the v3 default is the contract, not the fallback.

(b) Remove the tunable entirely for v4.  Default behaviour is the v3
behaviour, period.  If a regression surfaces, we handle it the way
we handle any other regression -- a revert or a targeted fix in the
next release, not a runtime knob.  This is the cleanest position
but also the riskiest, because it removes the safety net for
downstreams that may not have a way to test before shipping.

I prefer (a).  The cost of carrying an undocumented flag for one
release cycle is small, and the benefit is that if a regression
does surface we have a clean way to write "set this flag, verify
the regression goes away, then we know what we are fixing".  Without
it, we are stuck arguing from backtraces.

That said, if the consensus in this thread is (b), I will do that.
I can send a v4 with the tunable removed and the XFAIL test removed
or reworded accordingly.  Just say the word.

== (2) Architectural fix needs time, Fedora A/B testing

I would welcome Fedora A/B testing, and I will help however I can.
The minimal reproducer attached to my earlier reply runs in under a
second on any system with libnss_systemd.so.2 active and glycin
installed.  If Fedora wants a heavier workload, the original report
is ROSA bug 21031 -- Codeblocks startup -- and that is a real
user-visible scenario that a distro-scale build test can exercise.

For the ROSA side: Mikhail Novosyolov has confirmed earlier in this
thread that v3 fixes ROSA bug 21031 in our downstream testing.  Our
plan is to ship v3 (with Adhemerval's lock-free
__cxa_thread_atexit_impl rebased on top) once the upstream design
is settled.

On the application-expectations concern: the risk surface of v3 is
specifically an application that depends on the constructor of DSO A
always running before the constructor of DSO B when A and B are
loaded by independent dlopen calls in independent threads.  That is
the only ordering property v3 changes.  Constructors within a single
dlopen still run in dependency order; constructors of the executable
and its startup-time dependencies still run before main.  If Fedora
A/B testing turns up a regression, it is almost certainly of this
shape, and the discussion can focus on whether the application was
relying on undocumented behaviour, documented behaviour that we
missed, or whether v3 has a bug.

== (4) What are we asking for, and tooling

This is the question I have spent the most time thinking about.
Laying out what I believe v3 is and is not claiming, after
re-reading the patch.

v3 makes one specific claim:

   Constructors -- foreign function calls made during library
   initialisation -- may call any libc and dynamic-loader function
   that is safe to call from a regular non-signal-handler thread of
   the same process.

That claim implies:

   - A constructor may call dlsym, _dl_addr, _dl_find_dso_for_object,
     dlopen of a different DSO, dlclose of a different DSO, and the
     NSS-backed libc functions (getpwnam, getgrnam, getaddrinfo,
     getgrouplist, ...).  dl_iterate_phdr was already safe via
     dl_load_write_lock after BZ 28357 and is unaffected by v3.
   - A constructor may spawn threads, and those threads may call any
     of the above.
   - A constructor may call malloc, and a malloc interposer that is
     itself loaded via dlopen may in turn call any of the above from
     its own constructors.

The claim does NOT cover:

   - Destructors.  v3 deliberately scopes to the constructor path,
     which is where the actual reproducer fires.  Destructors run
     with dl_load_lock held inside _dl_close_worker just as
     constructors used to inside dl_open_worker, so the same
     theoretical deadlock shape exists for a destructor that calls
     dlopen, spawns a thread that hits NSS, etc.  In practice this
     does not seem to happen: destructors usually free resources
     rather than load new code or spawn threads, and the one
     recursive case that does come up (a destructor calling dlclose)
     is already handled separately via dl_close_state in dl-close.c.
     Extending v3 to also release dl_load_lock around _dl_call_fini
     would require the same state machine on the close path without
     a real-world reproducer justifying it.  If a destructor
     deadlock of this shape surfaces, it should be a follow-up
     patch with its own reproducer; v3 does not pretend to fix it.

   - Synchronous loader-internal reentrancy -- the case you describe
     in (4) where the loader's own malloc calls during dlopen
     processing (outside the ctor window) route through an
     interposer that itself calls dlopen.  v3 does not address this;
     it remains the "synchronous reentrancy" problem.  Note that
     malloc calls from inside a constructor are a different matter
     and ARE covered, because the constructor runs with the lock
     released.

Two special cases that v3 does handle, mentioned because they are
easy to get wrong when reading the patch:

   - Recursive dlopen of the same DSO from within its own
     constructor: detected via l_init_owner (same-TID check in
     call_init), returns immediately without waiting for itself.
     This is the l_init_owner field, not l_init_pending.

   - Concurrent dlopen of an already-being-initialised DSO from a
     different thread: the late caller takes the already-loaded
     early-return path, observes l_init_pending or l_init_called
     set, and waits on l_init_once until the constructor finishes.
     This preserves the property that dlopen does not return before
     the constructor has run.

== Fundamental limitation: ctor-spawned threads are not originally
    independent

Beyond the NOT-covered list above, there is one deadlock shape
that no same-design patch can lift, and v3 does not claim to.

A thread spawned from inside a DSO's constructor is not originally
independent -- its existence begins inside the ctor, and any work
it does that transitively depends on the ctor's DSO must wait for
the ctor to complete.  If a spawned worker calls dlopen on a DSO
that (directly or transitively) depends on the ctor's DSO, the
worker correctly blocks on l_init_once: unblocking it would expose
a half-constructed DSO to the new load.  That is DSO dependency
ordering, not lock contention, and the serialization is required
for correctness.

What v3 does lift is the orthogonal case: the spawned worker (or
any other thread) doing dlopen of a DSO that has no dependency on
the ctor's DSO.  The glycin reproducer is exactly this shape --
the worker's getgrouplist call routes through NSS module loading,
which dlopens libnss_systemd.so.2, which does not depend on the
ctor's DSO.  v3 lets the worker proceed; pre-v3 it deadlocked on
dl_load_lock.

So v3's actual claim, stated precisely, is: originally independent
threads can run independent dlopens concurrently, and a ctor (or a
thread it spawns) can do dlopen of an independent DSO without
serialising against the in-progress ctor.  It cannot untie work
that transitively depends on the ctor's DSO -- but no patch in
this design space can, because that serialization is what makes
concurrent dlopen correct.

That answers your question "all callbacks, or just init/fini
callbacks?".  v3's claim is precisely about init callbacks
(constructors).  Fini callbacks (destructors) are not yet covered
by v3; see above.  Other foreign function callbacks (signal
handlers, malloc hooks, pthread destructors, atfork handlers) are
not in scope.

I think stating this scope explicitly is more useful than leaving
it implicit.  If the project's position is that the scope should be
narrower (e.g. exclude some of the bullets above), or that v3
should not ship without also covering one of the NOT-covered
cases, that is a conversation worth having now rather than after
v3 ships.

On tooling:

- Random-order dependency init is orthogonal to v3 and would be a
useful shakedown tool.  I would support it as a separate patch
series, behind a tunable with the same "undocumented, deprecated,
removed after one cycle" caveat if it turns up no real-world
breakage.

- An _dl_debug_printf option that logs every dl_load_lock
acquisition site with a backtrace would help diagnose residual
deadlocks of this shape.  I do not have a patch for this but I am
happy to write one if there is interest.

- lari-style tooling for "which DSO registered which constructor"
is useful but I think out of scope for this thread.

Alexander Pevzner's reply on the same question is worth reading in
this light -- he makes the architectural argument independently
and reaches the same place: per-case patches do not fix the
underlying invariant, they just move manifestations around.

== Next steps

Concretely, what would help from the list:

- A steer on (3): undocumented-and-deprecated vs. removed entirely.
Once I have that I can send v4.  If the answer takes time, that is
fine -- I would rather get it right than get it fast.

- A steer on (4): does the scope claim above match what the project
wants to guarantee, or is it too strong?  If too strong, which
bullet should I narrow?

Thanks,
Artem