From 0f4a82b5e8793a50d594da3464e33c7c1bfe1ece Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Fri, 23 Jan 2026 14:22:16 +1100 Subject: [PATCH] fix(pthread/tls): destructors Also added tests for it. Signed-off-by: Anhad Singh --- src/header/limits/mod.rs | 4 + src/header/pthread/tls.rs | 171 ++++++++++++++++++++------------------ tests/pthread/tls.c | 71 ++++++++++++++++ 3 files changed, 167 insertions(+), 79 deletions(-) create mode 100644 tests/pthread/tls.c diff --git a/src/header/limits/mod.rs b/src/header/limits/mod.rs index 295054b1f9..a3f7df9d94 100644 --- a/src/header/limits/mod.rs +++ b/src/header/limits/mod.rs @@ -115,3 +115,7 @@ pub const COLL_WEIGHTS_MAX: c_long = _POSIX2_COLL_WEIGHTS_MAX; pub const EXPR_NEST_MAX: c_long = _POSIX2_EXPR_NEST_MAX; pub const LINE_MAX: c_long = _POSIX2_LINE_MAX; pub const RE_DUP_MAX: c_long = _POSIX2_RE_DUP_MAX; + +pub const PTHREAD_DESTRUCTOR_ITERATIONS: c_long = _POSIX_THREAD_DESTRUCTOR_ITERATIONS; +// TODO: What should this limit be? Both glibc and musl have it as 1024 +pub const PTHREAD_KEYS_MAX: c_long = 4096 * 32; diff --git a/src/header/pthread/tls.rs b/src/header/pthread/tls.rs index 6ece3e9926..7da2882339 100644 --- a/src/header/pthread/tls.rs +++ b/src/header/pthread/tls.rs @@ -1,71 +1,47 @@ +// FIXME(andypython): remove this when #![allow(warnings, unused_variables)] is +// dropped from src/lib.rs. +#![warn(warnings, unused_variables)] + use super::*; // TODO: Hashmap? -use alloc::collections::BTreeMap; +use alloc::{collections::BTreeMap, vec::Vec}; use core::{ - cell::{Cell, RefCell}, + cell::RefCell, + ptr, sync::atomic::{AtomicUsize, Ordering}, }; -use crate::{header::errno::EINVAL, sync::Mutex}; +use crate::{ + header::{errno::EINVAL, limits::PTHREAD_DESTRUCTOR_ITERATIONS}, + sync::Mutex, +}; -// TODO: What should this limit be? -pub const PTHREAD_KEYS_MAX: u32 = 4096 * 32; +type Dtor = Option; + +struct Record { + data: *mut c_void, +} + +#[thread_local] +static VALUES: RefCell> = RefCell::new(BTreeMap::new()); +static KEYS: Mutex> = Mutex::new(BTreeMap::new()); +static NEXTKEY: AtomicUsize = AtomicUsize::new(1); #[unsafe(no_mangle)] pub unsafe extern "C" fn pthread_getspecific(key: pthread_key_t) -> *mut c_void { - //print!("pthread_getspecific key={:#0x}: ", key); - - // TODO: Right? - if !KEYS.lock().contains_key(&key) { - //println!("= not found"); - return core::ptr::null_mut(); - } - - let Some(&Record { data, .. }) = VALUES.borrow_mut().get(&key) else { - //println!("= NULL"); - return core::ptr::null_mut(); - }; - //println!("= {:p}", data); - - data -} -#[unsafe(no_mangle)] -pub unsafe extern "C" fn pthread_key_create( - key_ptr: *mut pthread_key_t, - destructor: Option, -) -> c_int { - let key = NEXTKEY.fetch_add(1, Ordering::SeqCst) as pthread_key_t; - //println!("pthread_key_create new key {:#0x}, dtor {:?}", key, destructor); - - // TODO - //if key >= PTHREAD_KEYS_MAX { - //} - - KEYS.lock().insert(key, Dtor { destructor }); - - VALUES.borrow_mut().insert( - key, - Record { - data: core::ptr::null_mut(), - }, - ); - - unsafe { key_ptr.write(key) }; - - 0 -} - -#[unsafe(no_mangle)] -pub unsafe extern "C" fn pthread_key_delete(key: pthread_key_t) -> c_int { - if KEYS.lock().remove(&key).is_none() || VALUES.borrow_mut().remove(&key).is_none() { - // We don't have to return anything, but it's not less expensive to ignore it. - return EINVAL; - } - //println!("pthread_key_delete {:#0x}", key); - - 0 + // According to POSIX (issue 8): Calling [`pthread_getspecific`] with a key + // that has been deleted with [`pthread_key_delete`] or not obtained from + // [`pthread_key_create`] results in undefined behaviour. Therefore, we only + // do this check when debug assertions are explicitly enabled to avoid + // acquiring the global [`KEYS`] lock when it is not necessary. + debug_assert!(KEYS.lock().contains_key(&key)); + VALUES + .borrow_mut() + .get(&key) + .map(|record| record.data) + .unwrap_or(ptr::null_mut()) } #[unsafe(no_mangle)] @@ -78,40 +54,77 @@ pub unsafe extern "C" fn pthread_setspecific(key: pthread_key_t, value: *const c let mut guard = VALUES.borrow_mut(); - let mut record = guard.entry(key).or_insert(Record { + let record = guard.entry(key).or_insert(Record { data: core::ptr::null_mut(), }); //println!("Valid key for pthread_setspecific key {:#0x} value {:p} (was {:p})", key, value, record.data); record.data = value as *mut c_void; + 0 +} + +#[unsafe(no_mangle)] +pub unsafe extern "C" fn pthread_key_create( + key_ptr: *mut pthread_key_t, + destructor: Dtor, +) -> c_int { + let key = NEXTKEY.fetch_add(1, Ordering::SeqCst) as pthread_key_t; + + // TODO + //if key >= PTHREAD_KEYS_MAX { + //} + + KEYS.lock().insert(key, destructor); + + unsafe { key_ptr.write(key) }; + 0 +} + +#[unsafe(no_mangle)] +pub unsafe extern "C" fn pthread_key_delete(key: pthread_key_t) -> c_int { + if KEYS.lock().remove(&key).is_none() || VALUES.borrow_mut().remove(&key).is_none() { + // We don't have to return anything, but it's not less expensive to ignore it. + return EINVAL; + } 0 } -static KEYS: Mutex> = Mutex::new(BTreeMap::new()); - -struct Dtor { - destructor: Option, -} - -#[thread_local] -static VALUES: RefCell> = RefCell::new(BTreeMap::new()); - -struct Record { - data: *mut c_void, -} - -static NEXTKEY: AtomicUsize = AtomicUsize::new(1); - pub(crate) unsafe fn run_all_destructors() { - for (key, Record { data }) in VALUES.take() { - let Some(&Dtor { - destructor: Some(dtor), - }) = KEYS.lock().get(&key) - else { - continue; + for _ in 0..PTHREAD_DESTRUCTOR_ITERATIONS { + let mut any_run = false; + let dtors = { + let keys = KEYS.lock(); + keys.iter() + .filter_map(|(&key, &dtor)| dtor.map(|dtor| (key, dtor))) + .collect::>() }; - dtor(data); + // According to POSIX (issue 8): There is no specific order in which we + // have to run the destructors. + for (key, dtor) in dtors { + let mut values = VALUES.borrow_mut(); + if let Some(record) = values.get_mut(&key) { + let val = record.data; + if val.is_null() { + continue; + } + record.data = ptr::null_mut(); + drop(values); + dtor(val); + any_run = true; + } + } + + if !any_run { + break; + } } + + // According to POSIX (issue 8): If even after + // [`PTHREAD_DESTRUCTOR_ITERATIONS`] there are still some non-NULL + // associated values with associated destructors, the behaviour is + // implementation-defined. We can choose to stop calling them or continue + // calling them until none are left. Both musl and glibc choose to stop + // calling them so we do the same. } diff --git a/tests/pthread/tls.c b/tests/pthread/tls.c new file mode 100644 index 0000000000..ab79f8b8c9 --- /dev/null +++ b/tests/pthread/tls.c @@ -0,0 +1,71 @@ +#include +// PTHREAD_KEYS_MAX +#include +#include +#include +#include +#include + +pthread_key_t key_a; +pthread_key_t key_b; +size_t total_destructor_runs = 0; + +void drop_key_a(void *val_a) { + assert(pthread_getspecific(key_a) == NULL && val_a == (void *)0xaaaa); + total_destructor_runs++; + + void *val_b = pthread_getspecific(key_b); + printf("drop_key_a(a=%p, b=%p)\n", val_a, val_b); + + if (total_destructor_runs <= 2) { + pthread_setspecific(key_a, val_a); + return; + } + + if (val_b != NULL) { + assert(val_b == (void *)0xbbbb); + } else { + pthread_setspecific(key_b, (void *)0xbbbb); + } +} + +void drop_key_b(void *val_b) { + assert(pthread_getspecific(key_b) == NULL && val_b == (void *)0xbbbb); + total_destructor_runs++; + + void *val_a = pthread_getspecific(key_a); + printf("drop_key_b(a=%p, b=%p)\n", val_a, val_b); + + if (total_destructor_runs <= 2) { + pthread_setspecific(key_b, val_b); + return; + } + + if (val_a != NULL) { + assert(val_a == (void *)0xaaaa); + } else { + pthread_setspecific(key_a, (void *)0xaaaa); + } +} + +void *test_tls_destructors(void *arg) { + (void)arg; + assert(!pthread_key_create(&key_a, drop_key_a)); + assert(!pthread_key_create(&key_b, drop_key_b)); + assert(!pthread_setspecific(key_a, (void *)0xaaaa)); + assert(!pthread_setspecific(key_b, (void *)0xbbbb)); + return NULL; +} + +int main(void) { + pthread_t t1; + assert(!pthread_create(&t1, NULL, test_tls_destructors, NULL)); + assert(!pthread_join(t1, NULL)); + printf("total_destructor_iterations: %zu\n", total_destructor_runs); + // There are two destructors (i.e., `drop_key_{a, b}`), so the total number + // of iterations will be `2 * PTHREAD_DESTRUCTOR_ITERATIONS`. + assert((total_destructor_runs / 2) == PTHREAD_DESTRUCTOR_ITERATIONS); + + assert(pthread_getspecific(PTHREAD_KEYS_MAX) == NULL); + return EXIT_SUCCESS; +}