Fix async stream cancel corrupting read/write state#12704
Conversation
dicej
left a comment
dicej
left a comment
There was a problem hiding this comment.
Thanks for catching and fixing this, @jellevandenhooff.
Currently, not many guest binding generators support cancellation, and while the Rust binding generator does, it uses the synchronous versions of cancel-read and cancel-write, allowing this to fly under the radar.
@alexcrichton's recent PR downgrades the panic to a trap, but it's great to have it fixed altogether.
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
jellevandenhooff
force-pushed
the
fix-async-stream-cancel-state-corruption
branch
from
14635c0 to
1b56f8a
Compare
|
Happy to contribute! Rebased on that change. (As shameless plug, I've been discovering these issues working on wasip3 go guest at https://github.com/jellevandenhooff/go/commits/wasip3-prototype/. I run the entire go test suite with |
|
The stress testing is very much appreciated, thank you so much for the reports here and fixes! The surface area of async is broad enough that it's tough to ensure comprehensive coverage so it's great to get a different runtime tickling different bits than JS/C/Rust/Python have all tickled so far |
When `stream.cancel-read` or `stream.cancel-write` is called with the `async` option and the cancel cannot complete immediately (returns BLOCKED), the code was unconditionally transitioning the read/write state from GuestReady to Open. This destroyed the buffer address/count info stored in GuestReady, causing incorrect behavior when the host producer/consumer later tried to access the stream state. Guard the GuestReady -> Open state transition with a check that the cancel did not return BLOCKED. When blocked, the cancel is still in-flight and the read/write state must be preserved until the cancel completes. Adds a regression test that creates a host StreamProducer, starts an async read (BLOCKED), then async-cancels (BLOCKED), and waits for cancel completion.
jellevandenhooff
force-pushed
the
fix-async-stream-cancel-state-corruption
branch
from
1b56f8a to
ece5372
Compare
3 participants
Ran into an issue with async stream.cancel-read causing a panic. The attached change makes my guest pass, and the test reproduces. However, it is LLM written and I am not 100% confident in either the diagnosis or the description of the bug in the test.