-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
wasmtime-wasi: Fix spurious wake-ups in blocking_
of InputStream
and OutputStream
#10113
wasmtime-wasi: Fix spurious wake-ups in blocking_
of InputStream
and OutputStream
#10113
Conversation
Thanks. As you said in the other thread, there's no real way to test this operation from inside because it blocks. I am fine landing this without a test. |
The change seems fine indeed. Aside from |
loop {
// This `ready` call may return early due to `io::ErrorKind::WouldBlock`.
self.ready().await;
let data = self.read(size)?;
if data.is_empty() {
continue;
}
return Ok(data);
} Can we ensure that there won't be infinite or excessive invalid loops here? |
As a fail-safe, you could keep track of the number of iterations and return a trap if it exceeds {some number}. The reason I suggest to trap as opposed to return an empty buffer, is because after a certain threshold it's more likely to be a bug in the |
wasmtime/crates/wasi/src/tcp.rs Lines 844 to 850 in 5dfccc0
I noticed that |
I believe this requires further discussion before implementation. |
I think it'd be reasonable to pick a small arbitrary number, for example 10, as the limit of turns of the loop. If something is spuriously readable/writable 10 times in a row that seems indicative of a bug |
InputStream
blocking_read
blocking_
of InputStream
and OutputStream
I believe the test failure can be fixed by checking if the blocking_read size is zero, and in such a case avoiding the loop entirely or basically doing the read once and returning the result instead of turning the loop again. |
issue #9667
The
InputStream::ready
can be prematurely triggered byio::ErrorKind::WouldBlock
. This PR ensure thatblocking_
functions return a valid non-empty result.