[posix] E: Implement pthread_once and accept pthread_key_create destructors#29
Closed
esaurez wants to merge 2 commits into
Closed
[posix] E: Implement pthread_once and accept pthread_key_create destructors#29esaurez wants to merge 2 commits into
esaurez wants to merge 2 commits into
Conversation
This was referenced Jun 6, 2026
The dynamic loader walks `DT_NEEDED` recursively, but two bugs broke
diamond dependency graphs of the shape:
libdiamond.so -> libleft.so -> libbase.so
-> libright.so -> libbase.so
both of which only surface once `DT_NEEDED` chains are nontrivial
(the existing dlfcn-needed-c test stays a single-edge linear graph,
so it never exercised either path). Symptoms before this commit:
1. `dlopen(libdiamond.so)` HANGS inside the recursive load.
2. After the hang fix is in place, `dlclose(libdiamond.so)` panics
with `assertion left == right failed: expected to remove exactly
one dynamic library file`.
Fix 1 -- dlopen deadlock on ancestor lock
-----------------------------------------
`load_all_dependencies_recursive` holds the parent library's mutex
while iterating the registry's already-loaded entries. Each entry
gets `.lock()`-ed to compare names. The previous code skipped only
the immediate `new_dlhandle` self; it did not skip ancestors held
by outer recursive frames. Loading libright (depth 1) and then
recursing into libright's deps (depth 2) tried to lock libdiamond
in the inner scan, which deadlocked: libdiamond was held by the
outer frame.
Fix: thread a `BTreeSet<DlHandle>` of ancestor handles down the
recursion and skip every entry whose handle is in that set. The
ancestor is by definition not a candidate for "already loaded by
a sibling" so the skip is functionally safe.
Fix 2 -- dlopen now also handles transitive load races
------------------------------------------------------
Once the deadlock is gone, a second issue surfaces: the pre-loop
`retain` only sees libraries loaded *before* the current frame
started. In a diamond, libbase is loaded by libleft's recursion
*during* libdiamond's frame, after libdiamond's retain has already
run. Without a re-check, the outer loop would then open libbase a
second time (creating two distinct in-memory copies with their own
globals) or trip the `unreachable!()` when the VFS reuses the file
descriptor.
Fix: before each `DynamicLibrary::open` call, re-scan the registry
for a matching name and bind the existing copy if found.
Fix 3 -- dlclose strong_count assertion for diamond
---------------------------------------------------
The BFS unload loop extracts each library from the registry when
`Arc::strong_count == 2` (registry + current pop). For a diamond,
libbase has three Arc references at the time it is first popped
(registry + libleft.dependencies + libright.dependencies), so
`extract_if` correctly returns 0 entries -- but the assertion
demanded exactly 1, panicking the kernel. A later iteration of the
loop pops libright/libleft, drops their dependencies (releasing
the extra libbase ref), pushes libbase again, and the second pop
succeeds at the now-correct refcount.
Fix: replace the strict `assert_eq!(== 1)` with an early `continue`
when `extract_if` returns zero entries, keeping the (now relaxed)
"at most one" assertion to catch any future runtime invariant
violation.
Validation
----------
Companion test suite at esaurez/posix-tests `dlfcn-diamond-c`
proves both fixes end-to-end on a standalone Nanvix VM:
=== dlfcn diamond DT_NEEDED tests ===
PASS: dlopen(libright.so) [depth-2 chain]
PASS: dlopen(libdiamond.so)
PASS: re-dlopen(libdiamond.so) returns same handle
3 passed, 0 failed
All previously passing dlfcn tests (dlfcn-c, dlfcn-init-runpath-c,
dlfcn-pie-c) still pass; full posix-tests suite passes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…uctors Two related libposix gaps that together block any OpenSSL 3 consumer: 1. pthread_once was a no-op stub (nanvix#513) that returned 0 without ever invoking init_routine. Every consumer that uses pthread_once for lazy initialization (OpenSSL, libstdc++ static init, glib, ...) silently never initialized. OpenSSL 3 routes every internal one-time init through this mechanism; ossl_init_base never runs, EVP_MD_fetch returns NULL, _hashlib.openssl_sha256() raises UnsupportedDigestmodError, SSL_CTX_new returns NULL with empty error queue. 2. pthread_key_create rejected non-null destructors with OperationNotSupported. OpenSSL's ossl_init_thread always passes a destructor (defensive practice) so the call fails. With pthread_once fixed, this is the very next step in OpenSSL's init chain. Mechanics: - pthread_once: 4-state machine encoded in init_executed (NEVER_RUN=0, DONE=1, IN_PROGRESS=2, WAIT=3) -- musl's encoding, future-compatible with the multi-threaded futex upgrade documented in the source comments. Uses volatile read/write + compiler_fence for memory ordering. Rust Drop guard provides POSIX cancellation semantics (reset to NEVER_RUN on panic/unwind). Sanity-checks is_initialized==1 to catch PTHREAD_ONCE_INIT misuse. Recursive same-control calls log a warning and return 0 (POSIX undefined behavior; our choice over deadlock). - pthread_key_create: drop the destructor-is-some hard error. Log a one-line warning and continue. Worst case is a per-thread resource leak at thread exit, which is benign in the single-threaded user-space model. Once thread-exit destructor support lands, the warning will get removed. - pthread_once_t accessor methods (init_executed_ptr, is_initialized, etc.) added to sysapi so libposix can read/write the state-machine word without making the struct fields pub. Validation: - OpenSSL 3.5.0 standalone C probe goes PROBE_FAIL -> PROBE_PASS (EVP_MD_fetch and OSSL_PROVIDER_load both succeed). - Python smoke tests on cpython.elf: OPENSSL_CHAIN_PASS (SSLContext, _hashlib.openssl_sha256 direct call -- the Phase-3 documented init-order bug is fixed) LXML_CHAIN_PASS (regression) CTYPES_CHAIN_PASS (regression) Closes nanvix#513. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
8f1d424 to
78dd76d
Compare
bfdd573 to
6f4e0ae
Compare
Owner
Author
|
Superseded by upstream PR nanvix#2481, which carries the same diff (cherry-picked onto upstream/dev, deep-reviewed, with esaurez/* and cpython-migration references stripped). Two SHOULD-FIX findings from the deep review were applied: (a) eliminated &mut pthread_once_t materialisation to avoid Stacked-Borrows UB on the recursive-call path the implementation explicitly supports (raw-pointer accessors added to sysapi); (b) rewrote OnceGuard comment to honestly describe its forward-compat scope (no panic-unwind across extern C, no pthread_cancel today, panic=abort in release). Closing this fork PR; tracking continues upstream. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implement two related libposix gaps that together block any OpenSSL 3 consumer (Python's
_ssl/_hashlib, openssl CLI tooling, anything that goes throughEVP_*/SSL_*):pthread_once— was a no-op stub (this PR resolves [posix] Supportpthread_once()nanvix/nanvix#513) that returned 0 without ever invokinginit_routine. Every consumer that usespthread_oncefor lazy initialization (OpenSSL, libstdc++ static init, glib, ICU, …) was silently never initialised.pthread_key_create— rejected non-NULL destructors withOperationNotSupported. OpenSSL'sossl_init_threadalways passes a destructor (defensive practice) so the call fails. Withpthread_oncefixed, this was the very next step in OpenSSL's init chain to break.Why this matters now
OpenSSL 3 routes every internal one-time init through
pthread_once(crypto/threads_pthread.c:684→RUN_ONCEmacro atinclude/internal/thread_once.h:129):OPENSSL_init_cryptoreturns0EVP_MD_fetch("SHA2-256")returnsNULLSSL_CTX_new()returnsNULLwith empty error queue_hashlib.UnsupportedDigestmodError: unsupported hash type sha256andssl.SSLError: unknown error (_ssl.c:3076)This was the long-documented "OpenSSL init-order quirk" from cpython Phase 3 (esaurez/cpython#10) where the prediction was "the Group B unbundling will fix it for free". The prediction was correct in spirit but the unbundling on its own surfaces the real underlying cause: the Nanvix
pthread_oncestub.Implementation
pthread_once—src/libs/posix/src/pthread/mod.rs4-state machine encoded in
init_executed, mirroring musl libc's encoding so the future MT upgrade is a mechanical rewrite (swapread_volatile/write_volatileforAtomicI32::load(Acquire)/store(Release)+ futex wait/wake):0PTHREAD_ONCE_INIT)123Key properties:
init_executed; ifDONE, acquire compiler fence + return. No-op on single-CPU runtime, required when SMP lands.NEVER_RUN→IN_PROGRESS, runinit_routine, transitionIN_PROGRESS→DONEwith a release fence.Dropguard resets toNEVER_RUNifinit_routinepanics/unwinds — POSIX-compliant ("the effect ononce_controlshall be as ifpthread_once()was never called").is_initialized != 1→EINVAL(catchesPTHREAD_ONCE_INITmisuse).The 4-state encoding is documented inline; the multi-threaded upgrade path is sketched in a comment block. The struct already has
init_executed, which doubles as the futex word for the upgrade.pthread_key_create—src/libs/syscall/src/pthread/bindings/pthread_key_create.rsDrop the destructor-is-some hard error. Log a one-line warning and continue. Worst case is a per-thread resource leak at thread exit, which is benign in the single-threaded user-space model. Once thread-exit destructor support lands, the warning will get removed.
pthread_once_taccessors —src/libs/sysapi/src/sys_types.rsAdded
pub fn is_initialized(),init_executed(),set_init_executed(),init_executed_ptr()(the last for the volatile / future-atomic state-machine word). The struct fields stay private; the accessors letlibposix::pthread_onceoperate on the state-machine word without exposing internal layout.Validation
Standalone C probe (statically linked OpenSSL 3.5.0 + libposix)
Before:
After:
Python end-to-end on cpython.elf
STEP_6fixes the long-documented init-order bug where_hashlib.openssl_sha256(b"x")called as the first OpenSSL API in the process returnedUnsupportedDigestmodError.Regression tests
LXML_CHAIN_PASS([nanvix] E: Phase 4 — build lxml C extensions as .so (unbundled) cpython#11 path)CTYPES_CHAIN_PASS([nanvix] E: Unbundle libffi via DT_NEEDED libffi.so cpython#14 path)What's not in scope (deliberately)
pthread_key_create. Out of scope for this PR; the warning will get removed when implemented.pthread_once. The single-threaded variant is what current Nanvix needs; the comment block in the code documents the mechanical upgrade. Doing the upgrade requiresfutex_wait/futex_wake(or equivalent) in libposix, which doesn't exist yet.#[repr(C, packed)]onpthread_once_t— flagged in the research as a future MT correctness concern (atomic accesses require alignment). Doesn't matter for the ST single-CPU implementation, but should be revisited at the MT upgrade. Left as-is to not change the libc ABI.Closes
pthread_once()nanvix/nanvix#513.Downstream consumers
feat/build-shared-library— produceslibcrypto.so+libssl.so. Functionally requires this PR at runtime.feat/phase3b-unbundle-openssl— Phase 3b CPython unbundling that switches_ssl/_hashlibto dlopen the .so chain. Functionally requires this PR.Both downstream PRs are independently complete and will be filed once this lands.