misc(pthread): remove the need for tid_mutex

Signed-off-by: Anhad Singh <andypython@protonmail.com>

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<FdGuardUpper> {
+pub unsafe fn rlct_clone_impl(stack: *mut usize, tcb: &RtTcb) -> Result<usize> {
     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<FdGuardUpper> {
 
         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<FdGuardUpper> {
     // 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) -> ! {

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")]

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),
                 );
             }
 

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")]
         {

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<redox_rt::proc::FdGuardUpper>,
     ) {
         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<redox_rt::proc::FdGuardUpper>,
     ) {
         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)
         }
     }

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<crate::pthread::OsTid> {
+    unsafe fn rlct_clone(
+        stack: *mut usize,
+        _os_specific: &mut OsSpecific,
+    ) -> Result<crate::pthread::OsTid> {
         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<crate::pthread::OsTid> {
+    unsafe fn rlct_clone(
+        stack: *mut usize,
+        _os_specific: &mut OsSpecific,
+    ) -> Result<crate::pthread::OsTid> {
         todo!("rlct_clone not implemented for aarch64 yet")
     }
 

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<usize>;
 
-    unsafe fn rlct_clone(stack: *mut usize) -> Result<pthread::OsTid, Errno>;
+    unsafe fn rlct_clone(
+        stack: *mut usize,
+        os_specific: &mut OsSpecific,
+    ) -> Result<pthread::OsTid, Errno>;
     unsafe fn rlct_kill(os_tid: pthread::OsTid, signal: usize) -> Result<()>;
 
     fn current_os_tid() -> pthread::OsTid;

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<crate::pthread::OsTid> {
+    unsafe fn rlct_clone(
+        stack: *mut usize,
+        os_specific: &mut OsSpecific,
+    ) -> Result<crate::pthread::OsTid> {
         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(())

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::<MaybeUninit<OsTid>>::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<MaybeUninit<OsTid>>,
-    mutex2: *const Mutex<u64>,
+    synchronization_mutex: *const Mutex<u64>,
 ) -> ! {
-    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")]
     {