diff --git a/src/libs/posix/src/pthread/mod.rs b/src/libs/posix/src/pthread/mod.rs index 8e84bc97cb..d0cd3b10aa 100644 --- a/src/libs/posix/src/pthread/mod.rs +++ b/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, ) -> 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 } diff --git a/src/libs/sysapi/src/sys_types.rs b/src/libs/sysapi/src/sys_types.rs index 163e4da858..8202c9b4cc 100644 --- a/src/libs/sysapi/src/sys_types.rs +++ b/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)] diff --git a/src/libs/syscall/src/dlfcn/syscall/dlclose.rs b/src/libs/syscall/src/dlfcn/syscall/dlclose.rs index 864efe6b2c..f38f19d1dd 100644 --- a/src/libs/syscall/src/dlfcn/syscall/dlclose.rs +++ b/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. diff --git a/src/libs/syscall/src/dlfcn/syscall/dlopen.rs b/src/libs/syscall/src/dlfcn/syscall/dlopen.rs index 0281624206..04337e9e19 100644 --- a/src/libs/syscall/src/dlfcn/syscall/dlopen.rs +++ b/src/libs/syscall/src/dlfcn/syscall/dlopen.rs @@ -162,6 +162,7 @@ fn load_all_dependencies( dlfiles: &mut MutexGuard<'_, BTreeMap>>>, new_dlhandle: &DlHandle, new_dlfile: &mut MutexGuard<'_, DynamicLibrary>, + ancestors: &mut BTreeSet, ) -> 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>> = + 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,9 +288,15 @@ 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(()) @@ -240,7 +304,8 @@ fn load_all_dependencies( 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 = BTreeSet::new(); + load_all_dependencies_recursive(dlfiles, &new_dlhandle, &mut new_dlfile, &mut ancestors)?; Ok(()) } diff --git a/src/libs/syscall/src/pthread/bindings/pthread_key_create.rs b/src/libs/syscall/src/pthread/bindings/pthread_key_create.rs index 7e397cff4b..c29e6fb569 100644 --- a/src/libs/syscall/src/pthread/bindings/pthread_key_create.rs +++ b/src/libs/syscall/src/pthread/bindings/pthread_key_create.rs @@ -32,10 +32,18 @@ pub unsafe extern "C" fn pthread_key_create( return ErrorCode::InvalidArgument.get(); } - // Check if destructor is not null. + // Destructors are not yet supported (would require per-thread + // cleanup on thread exit, which Nanvix's single-threaded + // user-space model has no use for). Rather than refuse the + // call -- which breaks libraries like OpenSSL that always pass + // a non-null destructor as a defensive measure -- we accept + // the call, log a one-line warning, and silently drop the + // destructor. Worst case is a per-thread resource leak at + // thread exit, which is benign in a process-lifetime model. if destructor.is_some() { - ::syslog::warn!("pthread_key_create(): destructors are not supported"); - return ErrorCode::OperationNotSupported.get(); + ::syslog::warn!( + "pthread_key_create(): destructor ignored (not yet supported)" + ); } // Create key.