Repository navigation
Conversation
alexcrichton
left a comment
There was a problem hiding this comment.
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
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.
5a7d183 to
ffd64e8
Compare
|
@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 |
alexcrichton
left a comment
There was a problem hiding this comment.
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
EVENT_CANCELskips normal wake-state normalization. If the cancelled task remainsSLEEP_STATE_SLEEPING, Rust destructors that wake registered waiters attempt normal notifications after the wake-stream read has been cancelled.Set
SLEEP_STATE_WOKENat the start ofTaskState::drop, before retiring the read and destroying futures. This suppresses destruction-time notifications without scheduling another poll.tests/runtime/rust-cancel-wakenow 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:
cargo test --locked -p wit-bindgen: default,inter-task-wakeup,async-spawn, combined, and all features; 13 unit tests and 15 doctests per configuration.