Skip to content

rust: suppress wake notifications while destroying async tasks - #1740

Open
lthibault wants to merge 1 commit into
bytecodealliance:mainfrom
wetware:fix/cancelled-task-wake-state
Open

lthibault wants to merge 1 commit into
bytecodealliance:mainfrom
wetware:fix/cancelled-task-wake-state

Conversation

@lthibault

@lthibault lthibault commented Oct 9, 2026 •

Copy link
Copy Markdown

EVENT_CANCEL skips normal wake-state normalization. If the cancelled task remains SLEEP_STATE_SLEEPING, Rust destructors that wake registered waiters attempt normal notifications after the wake-stream read has been cancelled.

Set SLEEP_STATE_WOKEN at the start of TaskState::drop, before retiring the read and destroying futures. This suppresses destruction-time notifications without scheduling another poll.

tests/runtime/rust-cancel-wake now uses one destructor sender and one Rust oneshot waiter. The runner cancels a sleeping task through the component-model cancellation path, then checks that destruction completed and neither future was polled again. The redundant native test was removed.

Validation:

  • The focused component regression passes with the fix, fails at the wake-stream write when only the state store is removed, and passes after restoration. Verified on Wasmtime 48.0.1 and 49.0.2.
  • cargo test --locked -p wit-bindgen: default, inter-task-wakeup, async-spawn, combined, and all features; 13 unit tests and 15 doctests per configuration.
  • The standard Rust/C runtime suite passes 244 fixture builds and 173 composed executions on both Wasmtime versions.
  • Relevant crate tests, workspace Clippy, and formatting pass.

@alexcrichton alexcrichton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Would it be possible to simplify the runtime test to mostly just trying to test the bug being fixed here? As-is it seems pretty unwieldy and I'm having a tough time understanding it

Comment thread crates/guest-rust/src/rt/async_support.rs Outdated
Mark a TaskState as woken before retiring its wake stream and dropping
futures. Destructor-driven wakes then need no notification.

Add a composed-component regression that cancels a sleeping task whose
destructor wakes a registered Rust oneshot waiter. Check that destruction
completes and neither the root nor the waiter is polled again.
@lthibault
lthibault force-pushed the fix/cancelled-task-wake-state branch from 5a7d183 to ffd64e8 Compare October 10, 2026 11:29
@lthibault

lthibault commented Oct 10, 2026 •

Copy link
Copy Markdown
Author

@alexcrichton, thank you for the prompt reply! I've made the requested changes.

The runtime fixture now has one destructor sender and one oneshot waiter, addressing your review. I removed the sibling task, escaped/repeated wakes, extra gate, and wake graph. One checkpoint remains to let the task sleep before cancellation.

Dropping the import still delivers EVENT_CANCEL. The test checks that destruction completes and neither future is polled again.

@alexcrichton alexcrichton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Reading over this test now it looks like there's a more subtle bug here -- that the spawn_local'd task does not complete looks like a bug, that should still execute tho completion even if the original main future is cancelled/dropped. I'll try to take a closer look at this on Monday

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants