Fix thread::available_parallelism on WASI targets with threads#153604
Fix thread::available_parallelism on WASI targets with threads#153604surban wants to merge 1 commit intorust-lang:mainfrom
Conversation
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
There was a problem hiding this comment.
Pull request overview
This PR restores thread::available_parallelism() support for WASI targets compiled with the atomics target feature. A prior refactoring (ba462864f11) unified WASI into the shared unix.rs thread module but failed to migrate the WASI+atomics implementation of available_parallelism(), causing it to silently fall through to the unsupported catch-all.
Changes:
- Adds a dedicated
cfg_selectarm forall(target_os = "wasi", target_feature = "atomics")inavailable_parallelism()that callslibc::sysconfwith a hardcoded constant_SC_NPROCESSORS_ONLN = 84(since thelibccrate does not export this constant for WASI targets). - Plain wasip1 without threads continues to fall through to the
_arm and return an unsupported error, matching pre-regression behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6ca9426 to
0fcb2b0
Compare
|
Given that |
|
Personally I think that this is a reasonable implementation, although I think it'd be best to lump this in with the implementation for For the correctness of this function for single-threaded wasm 1 is the correct answer, and otherwise the only other target would be for |
0fcb2b0 to
a41f6b5
Compare
The refactoring in ba46286 ("std: Use more unix.rs code on WASI targets") moved WASI from its own thread module into the shared unix.rs module. However, it did not carry over the available_parallelism() implementation for WASI with threads, causing it to fall through to the unsupported catch-all. This silently regressed the support originally added in f0b7008. Fix this by: - Adding WASI with atomics (i.e. wasm32-wasip1-threads) to the sysconf-based cfg_select arm alongside other platforms that use libc::sysconf(_SC_NPROCESSORS_ONLN). - For single-threaded WASI (without atomics), returning Ok(1) since there is always exactly one thread of execution. This requires libc to export _SC_NPROCESSORS_ONLN for WASI targets, which is tracked in a companion PR to the libc crate.
a41f6b5 to
00f08b0
Compare
Yes, I have opened PR rust-lang/libc#5023 for all
I have now changed it to always return |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| all(target_os = "wasi", not(target_feature = "atomics")) => { | ||
| // Single-threaded WASM always has exactly one thread of execution. | ||
| Ok(NonZero::new(1).unwrap()) | ||
| } |
| #[cfg_attr( | ||
| any( | ||
| target_os = "redox", | ||
| target_os = "l4re", | ||
| target_env = "sgx", | ||
| target_os = "solid_asp3", | ||
| target_os = "teeos", | ||
| target_os = "wasi" | ||
| ), | ||
| should_panic | ||
| )] |
The refactoring in ba46286 ("std: Use more unix.rs code on WASI targets") moved WASI from its own thread module into the shared unix.rs module. However, it did not carry over the available_parallelism() implementation for WASI with threads, causing it to fall through to the unsupported catch-all. This silently regressed the support originally added in f0b7008.
Fix this by adding a dedicated cfg_select arm for WASI with atomics.
Plain wasip1 without threads continues to return unsupported, matching the previous behavior.