From df647941c50fb54fc670e559e0a617c38ccdfdaa Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Fri, 9 Jan 2026 17:21:37 +1100 Subject: [PATCH] misc(pthread): remove the need for `tid_mutex` Signed-off-by: Anhad Singh --- redox-rt/src/thread.rs | 14 ++++++--- src/ld_so/linker.rs | 2 +- src/ld_so/mod.rs | 2 +- src/ld_so/start.rs | 10 +++--- src/ld_so/tcb.rs | 12 +++++--- src/platform/linux/mod.rs | 12 ++++++-- src/platform/pal/mod.rs | 6 +++- src/platform/redox/mod.rs | 15 +++++---- src/pthread/mod.rs | 64 ++++++++++++++------------------------- 9 files changed, 72 insertions(+), 65 deletions(-) diff --git a/redox-rt/src/thread.rs b/redox-rt/src/thread.rs index 81d46c31f9..91ef61929e 100644 --- a/redox-rt/src/thread.rs +++ b/redox-rt/src/thread.rs @@ -5,7 +5,7 @@ use syscall::Result; use crate::{RtTcb, arch::*, proc::*, signal::tmp_disable_signals, static_proc_info}; /// Spawns a new context sharing the same address space as the current one (i.e. a new thread). -pub unsafe fn rlct_clone_impl(stack: *mut usize) -> Result { +pub unsafe fn rlct_clone_impl(stack: *mut usize, tcb: &RtTcb) -> Result { let proc_info = static_proc_info(); assert!(proc_info.has_proc_fd); let cur_proc_fd = unsafe { proc_info.proc_fd.assume_init_ref() }; @@ -20,7 +20,7 @@ pub unsafe fn rlct_clone_impl(stack: *mut usize) -> Result { let buf = create_set_addr_space_buf( cur_addr_space_fd.as_raw_fd(), - __relibc_internal_rlct_clone_ret as usize, + __relibc_internal_rlct_clone_ret as *const () as usize, stack as usize, ); new_addr_space_sel_fd.write(&buf)?; @@ -39,11 +39,17 @@ pub unsafe fn rlct_clone_impl(stack: *mut usize) -> Result { // the same process) will be discarded. Process-specific signals will ignore this new thread, // until it has initialized its own signal handler. - // Unblock context. let start_fd = new_thr_fd.dup(b"start")?; + + let fd = new_thr_fd.as_raw_fd(); + unsafe { + tcb.thr_fd.get().write(Some(new_thr_fd)); + } + + // Unblock context. start_fd.write(&[0])?; - Ok(new_thr_fd) + Ok(fd) } pub unsafe fn exit_this_thread(stack_base: *mut (), stack_size: usize) -> ! { diff --git a/src/ld_so/linker.rs b/src/ld_so/linker.rs index ad9b15346e..69db3aa1a0 100644 --- a/src/ld_so/linker.rs +++ b/src/ld_so/linker.rs @@ -698,7 +698,7 @@ impl Linker { tcb.copy_masters().map_err(|_| DlError::Malformed)?; tcb.activate( #[cfg(target_os = "redox")] - thr_fd, + Some(thr_fd), ); #[cfg(target_os = "redox")] diff --git a/src/ld_so/mod.rs b/src/ld_so/mod.rs index d16913edf9..12532e9c38 100644 --- a/src/ld_so/mod.rs +++ b/src/ld_so/mod.rs @@ -123,7 +123,7 @@ pub fn static_init( .expect_notls("failed to copy TLS master data"); tcb.activate( #[cfg(target_os = "redox")] - thr_fd, + Some(thr_fd), ); } diff --git a/src/ld_so/start.rs b/src/ld_so/start.rs index 938a09b820..eed04d81a4 100644 --- a/src/ld_so/start.rs +++ b/src/ld_so/start.rs @@ -160,12 +160,14 @@ pub unsafe extern "C" fn relibc_ld_so_start(sp: &'static mut Stack, ld_entry: us crate::platform::get_auxv_raw(sp.auxv().cast(), redox_rt::auxv_defs::AT_REDOX_THR_FD) .expect_notls("no thread fd present"); - let tcb = Tcb::new(0).expect_notls("ld.so: failed to allocate bootstrap TCB"); + let tcb = Tcb::new(0).expect_notls("[ld.so]: failed to allocate bootstrap TCB"); tcb.activate( #[cfg(target_os = "redox")] - redox_rt::proc::FdGuard::new(thr_fd) - .to_upper() - .expect_notls("failed to move thread fd to upper table"), + Some( + redox_rt::proc::FdGuard::new(thr_fd) + .to_upper() + .expect_notls("failed to move thread fd to upper table"), + ), ); #[cfg(target_os = "redox")] { diff --git a/src/ld_so/tcb.rs b/src/ld_so/tcb.rs index c3ea48ac33..7451fa4f9a 100644 --- a/src/ld_so/tcb.rs +++ b/src/ld_so/tcb.rs @@ -40,10 +40,10 @@ impl Master { } #[cfg(target_os = "linux")] -type OsSpecific = (); +pub type OsSpecific = (); #[cfg(target_os = "redox")] -type OsSpecific = redox_rt::signal::RtSigarea; +pub type OsSpecific = redox_rt::signal::RtSigarea; #[derive(Debug)] #[repr(C)] @@ -209,7 +209,7 @@ impl Tcb { /// Activate TLS pub unsafe fn activate( &mut self, - #[cfg(target_os = "redox")] thr_fd: redox_rt::proc::FdGuardUpper, + #[cfg(target_os = "redox")] thr_fd: Option, ) { unsafe { Self::os_arch_activate( @@ -363,10 +363,12 @@ impl Tcb { os: &OsSpecific, tls_end: usize, tls_len: usize, - thr_fd: redox_rt::proc::FdGuardUpper, + thr_fd: Option, ) { unsafe { - os.thr_fd.get().write(Some(thr_fd)); + if let Some(thr_fd) = thr_fd { + os.thr_fd.get().write(Some(thr_fd)); + } redox_rt::tcb_activate(os, tls_end, tls_len) } } diff --git a/src/platform/linux/mod.rs b/src/platform/linux/mod.rs index 9659a57298..9bc285d11b 100644 --- a/src/platform/linux/mod.rs +++ b/src/platform/linux/mod.rs @@ -1,6 +1,8 @@ use core::{arch::asm, num::NonZeroU64, ptr}; use super::{ERRNO, Pal, types::*}; +#[cfg(target_arch = "x86_64")] +use crate::ld_so::tcb::OsSpecific; use crate::{ c_str::CStr, header::{ @@ -623,7 +625,10 @@ impl Pal for Sys { } #[cfg(target_arch = "x86_64")] - unsafe fn rlct_clone(stack: *mut usize) -> Result { + unsafe fn rlct_clone( + stack: *mut usize, + _os_specific: &mut OsSpecific, + ) -> Result { let flags = CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND | CLONE_THREAD; let pid; asm!(" @@ -678,7 +683,10 @@ impl Pal for Sys { } #[cfg(target_arch = "aarch64")] - unsafe fn rlct_clone(stack: *mut usize) -> Result { + unsafe fn rlct_clone( + stack: *mut usize, + _os_specific: &mut OsSpecific, + ) -> Result { todo!("rlct_clone not implemented for aarch64 yet") } diff --git a/src/platform/pal/mod.rs b/src/platform/pal/mod.rs index b10b8053c0..c0ed81c666 100644 --- a/src/platform/pal/mod.rs +++ b/src/platform/pal/mod.rs @@ -13,6 +13,7 @@ use crate::{ sys_utsname::utsname, time::{itimerspec, timespec}, }, + ld_so::tcb::OsSpecific, out::Out, pthread, }; @@ -221,7 +222,10 @@ pub trait Pal { fn posix_getdents(fildes: c_int, buf: &mut [u8]) -> Result; - unsafe fn rlct_clone(stack: *mut usize) -> Result; + unsafe fn rlct_clone( + stack: *mut usize, + os_specific: &mut OsSpecific, + ) -> Result; unsafe fn rlct_kill(os_tid: pthread::OsTid, signal: usize) -> Result<()>; fn current_os_tid() -> pthread::OsTid; diff --git a/src/platform/redox/mod.rs b/src/platform/redox/mod.rs index 9338a97508..2f59dc9002 100644 --- a/src/platform/redox/mod.rs +++ b/src/platform/redox/mod.rs @@ -48,6 +48,7 @@ use crate::{ unistd::{F_OK, R_OK, SEEK_CUR, SEEK_SET, W_OK, X_OK}, }, io::{self, BufReader, prelude::*}, + ld_so::tcb::{OsSpecific, Tcb}, out::Out, platform::sys::{ libredox::RawResult, @@ -926,15 +927,17 @@ impl Pal for Sys { Ok(bytes_read) } - unsafe fn rlct_clone(stack: *mut usize) -> Result { + unsafe fn rlct_clone( + stack: *mut usize, + os_specific: &mut OsSpecific, + ) -> Result { let _guard = CLONE_LOCK.read(); - let res = clone::rlct_clone_impl(stack); + let res = clone::rlct_clone_impl(stack, os_specific); - res.map(|mut fd| crate::pthread::OsTid { - thread_fd: fd.take(), - }) - .map_err(|error| Errno(error.errno)) + res.map(|thread_fd| crate::pthread::OsTid { thread_fd }) + .map_err(|error| Errno(error.errno)) } + unsafe fn rlct_kill(os_tid: crate::pthread::OsTid, signal: usize) -> Result<()> { redox_rt::sys::posix_kill_thread(os_tid.thread_fd, signal as u32)?; Ok(()) diff --git a/src/pthread/mod.rs b/src/pthread/mod.rs index 415b5ec9b2..0f86adfd67 100644 --- a/src/pthread/mod.rs +++ b/src/pthread/mod.rs @@ -1,22 +1,18 @@ //! Relibc Threads, or RLCT. use core::{ - cell::{Cell, UnsafeCell}, - mem::{MaybeUninit, offset_of}, - ptr::{self, NonNull, addr_of}, + cell::UnsafeCell, + mem::MaybeUninit, + ptr::{self, addr_of}, sync::atomic::{AtomicBool, AtomicUsize, Ordering}, }; -use alloc::{boxed::Box, collections::BTreeMap}; +use alloc::collections::BTreeMap; use crate::{ error::Errno, header::{errno::*, pthread as header, sched::sched_param, sys_mman}, - ld_so::{ - ExpectTlsFree, - linker::Linker, - tcb::{Master, Tcb}, - }, + ld_so::{ExpectTlsFree, tcb::Tcb}, platform::{Pal, Sys, types::*}, }; @@ -124,9 +120,6 @@ pub(crate) unsafe fn create( let synchronization_mutex = Mutex::locked(current_sigmask); let synchronization_mutex = &synchronization_mutex; - let tid_mutex = spin::Mutex::>::new(MaybeUninit::uninit()); - let mut tid_guard = tid_mutex.lock(); - let stack_size = attrs.stacksize.next_multiple_of(Sys::getpagesize()); let stack_base = if attrs.stack != 0 { @@ -187,8 +180,8 @@ pub(crate) unsafe fn create( push(0); } push(0); + push(0); push(synchronization_mutex as *const _ as usize); - push(addr_of!(tid_mutex) as usize); push(new_tcb as *mut _ as usize); push(arg as usize); @@ -197,13 +190,11 @@ pub(crate) unsafe fn create( push(new_thread_shim as usize); } - let Ok(os_tid) = Sys::rlct_clone(stack) else { + let Ok(os_tid) = Sys::rlct_clone(stack, &mut new_tcb.os_specific) else { return Err(Errno(EAGAIN)); }; core::mem::forget(stack_raii); - tid_guard.write(os_tid); - drop(tid_guard); let _ = synchronization_mutex.lock(); OS_TID_TO_PTHREAD @@ -218,38 +209,29 @@ unsafe extern "C" fn new_thread_shim( entry_point: unsafe extern "C" fn(*mut c_void) -> *mut c_void, arg: *mut c_void, tcb: *mut Tcb, - // XXX: [`OsTid`] is required to activate the TCB. [`sync::Mutex`] may call - // [`futex_wait`], which goes through [`redox_rt::sync::wrapper`] and - // therefore requires the TCB to be initialised. If the new thread starts - // before [`create`] has released the lock, calling [`futex_wait`] would - // result in a `SIGSEGV`. Hence, we use a spinlock. - mutex1: *const spin::Mutex>, - mutex2: *const Mutex, + synchronization_mutex: *const Mutex, ) -> ! { - let tid = (*(&*mutex1).lock()).assume_init(); - let tcb = tcb.as_mut().unwrap(); + let tcb = tcb.as_mut().expect_notls("non-null TLS is required"); - #[cfg(not(target_os = "redox"))] - { - tcb.activate(); - } - #[cfg(target_os = "redox")] - { - tcb.activate( - redox_rt::proc::FdGuard::new(tid.thread_fd) - .to_upper() - .unwrap(), - ); - redox_rt::signal::setup_sighandler(&tcb.os_specific, false); - } + #[cfg(not(target_os = "redox"))] + { + tcb.activate(); + } + #[cfg(target_os = "redox")] + { + // `thr_fd` in `tcb` is set by [`Sys::rlct_clone`] *before* jumping to + // the entry point of the new thread. + tcb.activate(None); + redox_rt::signal::setup_sighandler(&tcb.os_specific, false); + } - let procmask = (&*mutex2).as_ptr().read(); + let procmask = (&*synchronization_mutex).as_ptr().read(); - tcb.copy_masters().unwrap(); + tcb.copy_masters().unwrap(); (*tcb).pthread.os_tid.get().write(Sys::current_os_tid()); - (&*mutex2).manual_unlock(); + (&*synchronization_mutex).manual_unlock(); #[cfg(target_os = "redox")] {