Skip to content

Commit

Permalink
Rollup merge of #122088 - ChrisDenton:fixme, r=workingjubilee
Browse files Browse the repository at this point in the history
Remove unnecessary fixme on new thread stack size

As the FIXME itself notes, there's nothing to fix here.

And as the documentation for [`CreateThread`] says of `dwStackSize`, the value is rounded up to the nearest page. A 4kb stack is very small but perfectly usable if you're careful. Of course it will be very limited but there's no reason to add artificial limits. We don't know what the user is doing.

[`CreateThread`]: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createthread
  • Loading branch information
matthiaskrgr authored Mar 6, 2024
2 parents f1354ed + 8718317 commit 7220e52
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 5 deletions.
7 changes: 2 additions & 5 deletions library/std/src/sys/pal/windows/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,8 @@ impl Thread {
pub unsafe fn new(stack: usize, p: Box<dyn FnOnce()>) -> io::Result<Thread> {
let p = Box::into_raw(Box::new(p));

// FIXME On UNIX, we guard against stack sizes that are too small but
// that's because pthreads enforces that stacks are at least
// PTHREAD_STACK_MIN bytes big. Windows has no such lower limit, it's
// just that below a certain threshold you can't do anything useful.
// That threshold is application and architecture-specific, however.
// CreateThread rounds up values for the stack size to the nearest page size (at least 4kb).
// If a value of zero is given then the default stack size is used instead.
let ret = c::CreateThread(
ptr::null_mut(),
stack,
Expand Down
13 changes: 13 additions & 0 deletions library/std/src/thread/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,3 +423,16 @@ fn scope_join_race() {
});
}
}

// Test that the smallest value for stack_size works on Windows.
#[cfg(windows)]
#[test]
fn test_minimal_thread_stack() {
use crate::sync::atomic::AtomicU8;
static COUNT: AtomicU8 = AtomicU8::new(0);

let builder = thread::Builder::new().stack_size(1);
let before = builder.spawn(|| COUNT.fetch_add(1, Ordering::Relaxed)).unwrap().join().unwrap();
assert_eq!(before, 0);
assert_eq!(COUNT.load(Ordering::Relaxed), 1);
}

0 comments on commit 7220e52

Please sign in to comment.