fix(wasix): Interrupt atomic waiters on signals#6536
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a WASIX shutdown/signal-delivery issue where threads blocked in memory.atomic.wait could remain indefinitely stuck (including on terminating signals), by waking atomic waiters when relevant signals are delivered and adding regression tests to cover the behavior.
Changes:
- Register the process shared memory during WASIX environment initialization so the process can wake atomic waiters.
- On selected signals, call
SharedMemory::wake_all_atomic_waiters()before delivering signals to threads/processes. - Add two WASIX C-based regression tests (plus build flags and runner scripts) covering signal interruption of atomic waiters, including child-process scenarios.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/wasix/atomic-wait-signal/run.sh | Adds a regression runner ensuring the process doesn’t hang in atomic.wait and exits non-zero on SIGKILL. |
| tests/wasix/atomic-wait-signal/main.c | Minimal repro: parent blocks in atomic.wait, child sends SIGKILL. |
| tests/wasix/atomic-wait-signal/.flags | Enables pthreads for the test build. |
| tests/wasix/atomic-wait-signal-children/run.sh | Adds regression runner validating signal behavior with child processes and forwarding. |
| tests/wasix/atomic-wait-signal-children/main.c | Multi-process repros for targeted kill and forwarding-to-children cases while blocked in atomic.wait. |
| tests/wasix/atomic-wait-signal-children/.flags | Enables pthreads for the test build. |
| lib/wasix/src/state/func_env.rs | Registers shared memory with the process during initialization. |
| lib/wasix/src/os/task/process.rs | Stores shared memory on the process and wakes atomic waiters on selected signals. |
Comment on lines
+606
to
+610
| /// Registers the shared memory used by this process. | ||
| pub fn register_memory(&self, memory: SharedMemory) { | ||
| let mut inner = self.inner.0.lock().unwrap(); | ||
| inner.memory = Some(memory); | ||
| } |
| puts("waiting"); | ||
| fflush(stdout); | ||
|
|
||
| __builtin_wasm_memory_atomic_wait32((int *)&wait_word, 0, -1LL); |
Comment on lines
+10
to
+16
| static _Atomic uint32_t wait_word = 0; | ||
|
|
||
| static void wait_forever(const char *name) { | ||
| printf("%s waiting\n", name); | ||
| fflush(stdout); | ||
|
|
||
| __builtin_wasm_memory_atomic_wait32((int *)&wait_word, 0, -1LL); |
Arshia001
requested changes
May 4, 2026
| fn should_wake_atomic_waiters_for_signal(signal: Signal) -> bool { | ||
| // Atomic wait wakeups are memory-wide, so only use them for signals | ||
| // that should interrupt or terminate execution anyway. | ||
| matches!( |
Member
There was a problem hiding this comment.
We definitely also need SIGWAKEUP here.
| fn wake_atomic_waiters(process: &WasiProcessInner) { | ||
| let pid = process.pid; | ||
| if let Some(memory) = &process.memory { | ||
| if let Err(err) = memory.wake_all_atomic_waiters() { |
Member
There was a problem hiding this comment.
Does this API both exist and work properly for all backends? Or are we doing a sys-only thing here?
Collaborator
Author
There was a problem hiding this comment.
yeah, sys only unfortunately.
theduke
force-pushed
the
wake-atomics-on-signals
branch
from
f74e9ac to
44f3d01
Compare
theduke
force-pushed
the
wake-atomics-on-signals
branch
from
44f3d01 to
8ca4ebc
Compare
Previously threads could get stuck idefinitely , even on KILL signals, because they would be stuck in atomic waiting. The downside of this implementation is that it can lead to spurious wakeups, but most data structures building on top of atomics can usually handle these!
theduke
force-pushed
the
wake-atomics-on-signals
branch
from
8ca4ebc to
151b3d9
Compare
Arshia001
reviewed
May 5, 2026
| @@ -934,7 +934,7 @@ impl Instance { | |||
| Err(_err) => { | |||
| // ret is None if there is more than 2^32 waiter in queue or some other error | |||
| // TODO: why THIS specific trap code tho? -.- | |||
Member
There was a problem hiding this comment.
@theduke feel free to remove this piece of art from yours truly 😄
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously threads could get stuck idefinitely , even on KILL signals,
because they would be stuck in atomic waiting.
The downside of this implementation is that it can lead to spurious
wakeups, but most data structures building on top of atomics can usually
handle these!