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

[Carlos O'Donell]

On 7/21/26 6:47 AM, Artem Proskurnev wrote:
> 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".

Thanks for confirming that.

So the short-term fix doesn't solve your problem.

That makes the issue more pressing to review and discuss.

> == (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.

Since you ask for guidance further down:

* We should always document tunables.

* We should not ship undocumented tunables.

* Tunables are not ABI. We should remind downstreams.

* Tunables should not change standards conforming behaviour (they don't in this case).

I think it's OK to keep the tunable.

However, the fact that we need a tunable means we should be reviewing
the solution space more carefully and thinking about the impact.

For example how will developers know the crashing application can be
fixed by using the tunable? This is why I asked the open question about
_dl_debug_printf, tooling, and other means of observability. If we can
add observability via LD_DEBUG=all or another means that supports the
analysis then that would be beneficial.

Lastly, we should consider that glibc uptake in downstream takes
almost 1-2 years, and as such the tunable would have to remain in place
probably for 4 releases before we see all the reports, resolve them
and then remove the tunable.

All of this is normal for glibc, and I just wanted to set the expectation.

> == (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.

Great. I'd like to hear how the deployment goes and if you see any
issues.

> 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.

Agreed.

> == (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.

OK.

> 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.

Good. This is a *better* more narrowly scoped definition of the problem.

> 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.

This is correct (only for same-TID case).

>    - 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.

This is correct.

> == 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.

Correct. This would be a compositional defect in the application.
Tooling to detect this would be beneficial but not required.

> 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.

Agreed.

> 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.

Agreed.

> 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.

Good.

> 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.

No, the scope as defined is sufficiently narrow.

However, I would like to discuss destructors too, and if anything
needs to be done there.

> 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.

Agreed.

> - 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.

I think we should add this as a 2/2 patch, which improves the
observability.

My question to you is: How does a system admin, or developer know
to flip the tunable?

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

Agreed. I just showed it as an example of observability improvements.

I want us to think of "solutions" not just "patches."

That is to say that solving this problem might require more than
just patching.

> 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.

Right.

> == 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.

I think we should keep the tunable, it should be documented, and the
documentation should clearly state we intend to remove this when the
ecosystem has been shown to work with the new implemetnation.

> - 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?

The scope makes sense, but I'm also concerned about destructors since
both see this kind of problem. What prevents us from providing the same
invariant in destructors?


> Thanks,
> Artem
> 


-- 
Cheers,
Carlos.