Address review comments

This commit is contained in:
Orson Peters 2025-10-01 15:01:20 +02:00
parent 58830def36
commit 0b7cdf2435
3 changed files with 18 additions and 29 deletions

View file

@ -7,7 +7,9 @@ static DTORS: RefCell<Vec<(*mut u8, unsafe extern "C" fn(*mut u8)), System>> =
RefCell::new(Vec::new_in(System));
pub unsafe fn register(t: *mut u8, dtor: unsafe extern "C" fn(*mut u8)) {
let Ok(mut dtors) = DTORS.try_borrow_mut() else { rtabort!("unreachable") };
let Ok(mut dtors) = DTORS.try_borrow_mut() else {
rtabort!("the System allocator may not use TLS with destructors")
};
guard::enable();
dtors.push((t, dtor));
}

View file

@ -269,23 +269,13 @@ fn init_current(current: *mut ()) -> Thread {
// BUSY exists solely for this check, but as it is in the slow path, the
// extra TLS write above shouldn't matter. The alternative is nearly always
// a stack overflow.
// If you came across this message, contact the author of your
// allocator. If you are said author: A surprising amount of functions
// inside the standard library (e.g. `Mutex`, `File` when using long
// paths, even `panic!` when using unwinding), need memory allocation,
// so you'll get circular dependencies all over the place when using
// them. I (joboet) highly recommend using only APIs from core in your
// allocator and implementing your own system abstractions. Still, if
// you feel that a particular API should be entirely allocation-free,
// feel free to open an issue on the Rust repository, we'll see what we
// can do.
//
// If we reach this point it means our initialization routine ended up
// calling current() either directly, or indirectly through the global
// allocator, which is a bug either way as we may not call the global
// allocator in current().
rtabort!(
"\n\
Attempted to access thread-local data while allocating said data.\n\
Do not access functions that allocate in the global allocator!\n\
This is a bug in the global allocator.\n\
"
"init_current() was re-entrant, which indicates a bug in the Rust threading implementation"
)
} else {
debug_assert_eq!(current, DESTROYED);

View file

@ -1305,20 +1305,17 @@ impl ThreadId {
spin += 1;
}
let id;
// SAFETY: we have an exclusive lock on the counter.
unsafe {
id = (*COUNTER.get()).saturating_add(1);
(*COUNTER.get()) = id;
};
// Release the lock.
COUNTER_LOCKED.store(false, Ordering::Release);
if id == u64::MAX {
exhausted()
} else {
ThreadId(NonZero::new(id).unwrap())
if *COUNTER.get() == u64::MAX {
COUNTER_LOCKED.store(false, Ordering::Release);
exhausted()
} else {
let id = *COUNTER.get() + 1;
*COUNTER.get() = id;
COUNTER_LOCKED.store(false, Ordering::Release);
ThreadId(NonZero::new(id).unwrap())
}
}
}
}