[posix] E: Implement pthread_once and accept pthread_key_create destructors

src/libs/posix/src/pthread/mod.rs

@@ -368,6 +368,33 @@ pub extern "C" fn pthread_equal(thread1: pthread_t, thread2: pthread_t) -> c_int
 // pthread_once()
 //==================================================================================================
 
+/// State constants used internally by `pthread_once()` for the
+/// `init_executed` field of `pthread_once_t`.
+///
+/// # Description
+///
+/// `init_executed` doubles as a state-machine word.  The four
+/// values directly mirror musl libc's encoding in
+/// `src/thread/pthread_once.c`, which lets us upgrade to a
+/// futex-based multi-threaded implementation without changing
+/// the state semantics.
+///
+/// In a future multi-threaded model:
+///
+/// - `ONCE_NEVER_RUN` → CAS to `ONCE_IN_PROGRESS` and run `init`.
+/// - `ONCE_IN_PROGRESS` → CAS to `ONCE_WAIT` and block on futex.
+/// - `ONCE_WAIT` → continue waiting on the futex word.
+/// - `ONCE_DONE` → fast-path return.
+///
+/// In the current single-threaded model only `NEVER_RUN`,
+/// `IN_PROGRESS`, and `DONE` are reachable; `ONCE_WAIT` is
+/// reserved for the future upgrade.
+const ONCE_NEVER_RUN: c_int = 0;
+const ONCE_DONE: c_int = 1;
+const ONCE_IN_PROGRESS: c_int = 2;
+#[allow(dead_code)]
+const ONCE_WAIT: c_int = 3;
+
 ///
 /// # Description
 ///
@@ -396,8 +423,144 @@ pub unsafe extern "C" fn pthread_once(
     once_control: *mut pthread_once_t,
     init_routine: Option<unsafe extern "C" fn()>,
 ) -> c_int {
-    // TODO: https://github.com/nanvix/nanvix/issues/513
-    ::syslog::debug!("pthread_once(): not implemented");
+    // Argument validation.
+    if once_control.is_null() {
+        ::syslog::warn!("pthread_once(): null once_control");
+        return ErrorCode::InvalidArgument.get();
+    }
+    let Some(init_fn) = init_routine else {
+        ::syslog::warn!("pthread_once(): null init_routine");
+        return ErrorCode::InvalidArgument.get();
+    };
+
+    // Sanity check: `is_initialized` must equal 1 after
+    // `PTHREAD_ONCE_INIT`.  Any other value indicates a caller
+    // bug (e.g. a forgotten or corrupted static initializer).
+    //
+    // SAFETY: `once_control` was checked non-null above and is
+    // assumed to point to a valid `pthread_once_t` per POSIX
+    // contract.
+    let once: &mut pthread_once_t = unsafe { &mut *once_control };
+    if once.is_initialized() != pthread_once_t::IS_INITIALIZED_VALUE {
+        ::syslog::warn!(
+            "pthread_once(): once_control not initialized with PTHREAD_ONCE_INIT"
+        );
+        return ErrorCode::InvalidArgument.get();
+    }
+
+    // Fast path: already done.
+    //
+    // # Memory ordering
+    //
+    // POSIX requires that on return from `pthread_once`, the
+    // effects of `init_routine` are visible.  On the fast path
+    // this is provided by the volatile read + acquire compiler
+    // fence: the read cannot be hoisted, and the fence prevents
+    // subsequent loads from being reordered before the
+    // observation of `ONCE_DONE`.
+    //
+    // On the current single-threaded target this fence is a
+    // no-op at runtime (no SMP), but it is required for
+    // correctness when the multi-threaded upgrade is performed.
+    let state_ptr: *mut c_int = once.init_executed_ptr();
+    {
+        let current = unsafe { ::core::ptr::read_volatile(state_ptr) };
+        if current == ONCE_DONE {
+            ::core::sync::atomic::compiler_fence(::core::sync::atomic::Ordering::Acquire);
+            return 0;
+        }
+        if current == ONCE_IN_PROGRESS {
+            // POSIX (APPLICATION USAGE) says recursive calls on
+            // the same `once_control` from inside `init_routine`
+            // are undefined and shall not return.  In a
+            // single-threaded model the only way to reach this
+            // state is a recursive call: log and return success
+            // without re-running (the in-flight init will finish
+            // when control unwinds).
+            ::syslog::warn!(
+                "pthread_once(): recursive call on the same once_control \
+                 (in-progress) -- not re-running init"
+            );
+            return 0;
+        }
+    }
+
+    // Slow path: transition NEVER_RUN -> IN_PROGRESS, run init,
+    // transition IN_PROGRESS -> DONE.
+    //
+    // # Multi-threaded upgrade
+    //
+    // Replace the volatile write with a CAS:
+    //
+    //     loop {
+    //         match cas(state_ptr, NEVER_RUN, IN_PROGRESS) {
+    //             Ok(_)              => break,            // we are the initializer
+    //             Err(DONE)          => return 0,         // someone else finished
+    //             Err(IN_PROGRESS)
+    //               | Err(WAIT)      => {
+    //                 cas(state_ptr, IN_PROGRESS, WAIT);
+    //                 futex_wait(state_ptr, WAIT);
+    //                 continue;
+    //             }
+    //             _                  => unreachable!(),
+    //         }
+    //     }
+    //
+    // and replace the final `write_volatile(DONE)` with an
+    // atomic release-store followed by `futex_wake_all`.
+    unsafe { ::core::ptr::write_volatile(state_ptr, ONCE_IN_PROGRESS) };
+
+    // POSIX cancellation semantics:
+    //
+    //   "If init_routine is a cancellation point and is canceled,
+    //    the effect on once_control shall be as if pthread_once()
+    //    was never called."
+    //
+    // Rust does not have `pthread_cleanup_push`, but a Drop guard
+    // gives equivalent behavior across both panic-unwind and
+    // future cancellation paths: if `init_fn` panics or unwinds,
+    // `OnceGuard::drop` resets the state to `NEVER_RUN` so the
+    // next call retries from scratch.
+    //
+    // In ST mode without unwinding there is no waiter to wake;
+    // in the MT upgrade `OnceGuard::drop` would also call
+    // `futex_wake_all` on the state pointer.
+    struct OnceGuard {
+        state_ptr: *mut c_int,
+        completed: bool,
+    }
+    impl ::core::ops::Drop for OnceGuard {
+        fn drop(&mut self) {
+            if !self.completed {
+                // SAFETY: `state_ptr` was validated by the
+                // caller of `pthread_once` and remains live
+                // for the duration of this call.
+                unsafe {
+                    ::core::ptr::write_volatile(self.state_ptr, ONCE_NEVER_RUN);
+                }
+            }
+        }
+    }
+    let mut guard = OnceGuard {
+        state_ptr,
+        completed: false,
+    };
+
+    // SAFETY: `init_fn` is a non-null `extern "C" fn()` per the
+    // POSIX contract; the caller is responsible for ensuring it
+    // does not violate Rust's aliasing rules on shared state.
+    unsafe { init_fn() };
+
+    guard.completed = true;
+    drop(guard);
+
+    // Release fence: ensures all stores performed by `init_fn`
+    // are globally visible before `ONCE_DONE` becomes observable
+    // on other CPUs.  No-op at runtime in ST/single-CPU mode;
+    // required for correctness once SMP/MT lands.
+    ::core::sync::atomic::compiler_fence(::core::sync::atomic::Ordering::Release);
+    unsafe { ::core::ptr::write_volatile(state_ptr, ONCE_DONE) };
+
     0
 }
 

src/libs/sysapi/src/sys_types.rs

@@ -301,6 +301,61 @@ impl pthread_once_t {
 
     /// Size of `pthread_once_t` structure.
     pub const SIZE: usize = Self::SIZE_OF_IS_INITIALIZED + Self::SIZE_OF_INIT_EXECUTED;
+
+    /// Sentinel value of `is_initialized` set by `PTHREAD_ONCE_INIT`.
+    pub const IS_INITIALIZED_VALUE: c_int = 1;
+
+    /// Returns the value of the `is_initialized` field.
+    ///
+    /// # Description
+    ///
+    /// `is_initialized` is set to `1` by `PTHREAD_ONCE_INIT`.  A
+    /// non-`1` value indicates that the caller forgot to use
+    /// `PTHREAD_ONCE_INIT` and the `pthread_once_t` is uninitialized.
+    pub fn is_initialized(&self) -> c_int {
+        self.is_initialized
+    }
+
+    /// Returns the value of the `init_executed` field.
+    ///
+    /// # Description
+    ///
+    /// `init_executed` is used by `pthread_once()` as a state
+    /// machine to coordinate calling `init_routine` exactly once
+    /// per `pthread_once_t` object.  See the implementation in
+    /// `libposix` for the state encoding.
+    pub fn init_executed(&self) -> c_int {
+        self.init_executed
+    }
+
+    /// Sets the value of the `init_executed` field.
+    ///
+    /// # Description
+    ///
+    /// See `init_executed()` for the state encoding.  This setter
+    /// is used by `pthread_once()` to transition between states.
+    pub fn set_init_executed(&mut self, value: c_int) {
+        self.init_executed = value;
+    }
+
+    /// Returns a mutable raw pointer to the `init_executed` field.
+    ///
+    /// # Description
+    ///
+    /// `pthread_once()` uses this pointer with `ptr::read_volatile`
+    /// / `ptr::write_volatile` to implement the state-machine
+    /// transitions without the compiler optimising the loads or
+    /// stores away.  It is the same pointer that a future
+    /// multi-threaded implementation would pass to `futex_wait`
+    /// / `futex_wake_all`.
+    ///
+    /// # Safety
+    ///
+    /// The returned pointer is only valid while the caller holds
+    /// a reference to the underlying `pthread_once_t`.
+    pub fn init_executed_ptr(&mut self) -> *mut c_int {
+        ::core::ptr::addr_of_mut!(self.init_executed)
+    }
 }
 
 #[derive(Debug, Clone, Copy)]

src/libs/syscall/src/dlfcn/syscall/dlclose.rs

@@ -117,10 +117,27 @@ pub fn dlclose(handle: &DlHandle) -> Result<(), Error> {
             })
             .collect();
 
+        // `extract_if` may legitimately return zero entries when this
+        // library still has live dependents elsewhere in the dependency
+        // tree. This is the diamond `DT_NEEDED` case --
+        //
+        //   libdiamond.so -> libleft.so  -> libbase.so
+        //                 -> libright.so -> libbase.so
+        //
+        // While unloading libright, BFS pushes libbase onto the work
+        // list. By the time we pop libbase, libleft has not yet unloaded
+        // and still holds an Arc to libbase, so `strong_count` is 3
+        // (registry + libleft.dependencies + our pop). We skip libbase
+        // for now; a later iteration will pop libleft, drop its
+        // dependencies (releasing the extra libbase reference), push
+        // libbase a second time, and the subsequent pop will succeed.
+        if dep_dlfile.is_empty() {
+            continue;
+        }
         assert_eq!(
             dep_dlfile.len(),
             1,
-            "dlclose(): expected to remove exactly one dynamic library file"
+            "dlclose(): expected to remove at most one dynamic library file"
         );
 
         // Collect all dependencies of the dynamic library file.

src/libs/syscall/src/dlfcn/syscall/dlopen.rs

@@ -162,6 +162,7 @@ fn load_all_dependencies(
         dlfiles: &mut MutexGuard<'_, BTreeMap<DlHandle, Arc<Mutex<DynamicLibrary>>>>,
         new_dlhandle: &DlHandle,
         new_dlfile: &mut MutexGuard<'_, DynamicLibrary>,
+        ancestors: &mut BTreeSet<DlHandle>,
     ) -> Result<(), Error> {
         // Snapshot the loader's `DT_RUNPATH` entries so they are visible to
         // `resolve_library_path` while probing every dependency below. The
@@ -177,13 +178,31 @@ fn load_all_dependencies(
             .collect();
 
         // Bind to already loaded dependencies and remove them from the list.
+        //
+        // The closure below calls `dlfile.lock()` on every other entry in the
+        // registry while looking for a matching name. Those locks must skip
+        // BOTH the current library (`new_dlhandle`) AND every ancestor still
+        // held by an outer recursive frame — otherwise a diamond `DT_NEEDED`
+        // graph deadlocks, e.g.:
+        //
+        //   dlopen(libdiamond.so)                  // holds libdiamond lock
+        //     -> recurse into libright.so          // also holds libright lock
+        //          -> retain() iterates dlfiles    // tries to lock libdiamond
+        //             ^ blocks forever, libdiamond is still held by the
+        //               outer frame.
+        //
+        // The lookup is purely advisory (we want to know if a sibling already
+        // bound a dependency with the same name), so skipping ancestors is
+        // safe: an ancestor by definition cannot have been loaded by a prior
+        // iteration of the same frame's dependency list.
         dependencies.retain(|dependency| {
             // Resolve bare name so we can match against loaded libraries
             // that were opened with a full path.
             let resolved_dep: String = super::resolve_library_path(dependency, Some(&runpaths));
             for (dlhandle, dlfile) in dlfiles.iter() {
-                // Check if need to skip the dynamic library itself.
-                if dlhandle == new_dlhandle {
+                // Skip the dynamic library itself and any ancestor held by
+                // an outer frame's lock — locking them would deadlock.
+                if dlhandle == new_dlhandle || ancestors.contains(dlhandle) {
                     continue;
                 }
 
@@ -218,6 +237,45 @@ fn load_all_dependencies(
             // Resolve bare library names to full paths using search directories.
             let resolved_dep: String = super::resolve_library_path(&dependency, Some(&runpaths));
 
+            // Re-check the registry before opening: a prior iteration of
+            // this same loop may have already loaded this dependency
+            // transitively. This is the diamond case --
+            //
+            //   libdiamond.so -> libleft.so  -> libbase.so
+            //                 -> libright.so -> libbase.so
+            //
+            // After processing libleft (and recursing into it, which loads
+            // libbase), libbase is now in the registry. When the outer loop
+            // returns to process libright, the closure-based `retain` above
+            // never ran for libbase because it was not yet loaded at the
+            // time libright's frame started; we must catch the diamond
+            // here instead. Without this check the loader would either
+            // open libbase a second time (producing two distinct
+            // in-memory copies) or trip the `unreachable!()` below when
+            // the VFS reuses the underlying file descriptor.
+            let already_loaded: Option<Arc<Mutex<DynamicLibrary>>> =
+                dlfiles.iter().find_map(|(dlhandle, dlfile)| {
+                    if dlhandle == new_dlhandle || ancestors.contains(dlhandle) {
+                        return None;
+                    }
+                    let loaded_file: spin::MutexGuard<'_, DynamicLibrary> = dlfile.lock();
+                    let loaded_name: &str = loaded_file.name();
+                    if loaded_name == dependency.as_str() || loaded_name == resolved_dep {
+                        Some(dlfile.clone())
+                    } else {
+                        None
+                    }
+                });
+            if let Some(existing) = already_loaded {
+                ::syslog::debug!(
+                    "load_all_dependencies_recursive(): dependency '{}' loaded transitively \
+                     during this dlopen call; binding to existing copy",
+                    dependency
+                );
+                new_dlfile.bind_dependency(dependency, existing)?;
+                continue;
+            }
+
             // Open and pre-load the dynamic library file.
             let dep_dlfile: DynamicLibrary = DynamicLibrary::open(&resolved_dep)?;
             let handle: DlHandle = dep_dlfile.handle();
@@ -230,17 +288,24 @@ fn load_all_dependencies(
 
             new_dlfile.bind_dependency(dependency.clone(), dep_dlfile.clone())?;
 
-            // Load dependencies of the new dynamic library file.
+            // Load dependencies of the new dynamic library file. Mark the
+            // current library as an ancestor so the recursive frame does
+            // not try to lock our still-held mutex (would deadlock on
+            // diamond DT_NEEDED graphs).
+            ancestors.insert(*new_dlhandle);
             let mut dlfile: MutexGuard<'_, DynamicLibrary> = dep_dlfile.lock();
-            load_all_dependencies_recursive(dlfiles, &handle, &mut dlfile)?;
+            let result = load_all_dependencies_recursive(dlfiles, &handle, &mut dlfile, ancestors);
+            ancestors.remove(new_dlhandle);
+            result?;
         }
 
         Ok(())
     }
 
     let mut new_dlfile = new_dlfile.lock();
     let new_dlhandle = new_dlfile.handle();
-    load_all_dependencies_recursive(dlfiles, &new_dlhandle, &mut new_dlfile)?;
+    let mut ancestors: BTreeSet<DlHandle> = BTreeSet::new();
+    load_all_dependencies_recursive(dlfiles, &new_dlhandle, &mut new_dlfile, &mut ancestors)?;
 
     Ok(())
 }