input-stream pollable read-readiness guarantees · wasi

Stream: wasi

Topic: input-stream pollable read-readiness guarantees

Pat Hickey (Jan 22 2025 at 19:59):

want to bring more visibility to https://github.com/bytecodealliance/wasmtime/issues/9667#issuecomment-2608134946: Alex pointed out that, basically, the guarantees I wrote down for input-stream read readiness are bogus and cant reasonably be met on top of posix. They've never been met by wasmtime, so I think we should just change the spec to weaken the guarantees provided, and accommodate that in e.g. wstd and any other places we depended on it.

wasmtime-wasi: Unexpected tcp stream break (Returning 0 for `std::io::ErrorKind::WouldBlock`) · Issue #9667 · bytecodealliance/wasmtime

wasmtime/crates/wasi/src/tcp.rs Lines 719 to 723 in 6767488 // Failing with `EWOULDBLOCK` is how we differentiate between a closed channel and no // data to read right now. Err(e) if e.kind() == st...

Pat Hickey (Jan 22 2025 at 19:59):

@Guy Bedford do you know if jco has the same behavior as wasmtime here?

Pat Hickey (Jan 22 2025 at 19:59):

(cc @Dan Gohman @Yosh Wuyts @Joel Dice )

Guy Bedford (Jan 22 2025 at 21:20):

I'm pretty sure Node.js stream reads after poll can return zero bytes in reality as well, so this wouldn't be a problem for JS if it's already happening there too

Yosh Wuyts (Jan 23 2025 at 16:01):

@Pat Hickey I read over the issue - what's the proposed mapping back to Rust from this? Is it something like:

wasi: StreamError::Closed -> AsyncRead::read: Ok(0)
wasi: Ok(0) -> AsyncRead::read: [pending future]

Yosh Wuyts (Jan 23 2025 at 16:04):

Heh, okay what I wrote is less clear than I imagined it would be. I guess I'm wondering what the concrete change we're proposing here is?

Dave Bakker (badeend) (Jan 23 2025 at 16:07):

I presume slightly changing the documentation from

The pollable given by subscribe will be ready when more bytes are available.

to something along the lines of:

The pollable given by subscribe will be ready when the read should be attempted again.

Yosh Wuyts (Jan 23 2025 at 16:10):

ohhh, I see - and that gives us room to schedule a wake to signal EOF, right?

Dave Bakker (badeend) (Jan 23 2025 at 16:11):

Hmm, not sure what you mean, but I don't think EOF has anything to do with this

Dave Bakker (badeend) (Jan 23 2025 at 16:12):

More about EWOULDBLOCK (a.k.a. Poll:Pending in rust)

Yosh Wuyts (Jan 23 2025 at 16:12):

Oh interesting; I meant EOF as "end of stream" - so like, we can signal "ready" to say "there are no more bytes here". The previous wording read like that was disallowed?

Yosh Wuyts (Jan 23 2025 at 16:12):

ahhh, okay - that's also interesting

Yosh Wuyts (Jan 23 2025 at 16:14):

either way tho: I now understand that the current wording is too narrow, ty!

Joel Dice (Jan 23 2025 at 16:14):

Basically we want to communicate to users that they should always use a loop when pollables and streams are involved since a pollable being ready doesn't guarantee there the next read or write will produce/consume more than zero bytes.

Dave Bakker (badeend) (Jan 23 2025 at 16:15):

Pollable::ready() has more to do with EWOULDBLOCK.

The current docs mention that: If a pollable signals readiness, the next operation MUST NOT return "EWOULDBLOCK" (or however the API in question has WIT-ified that response code).

The proposal is to relax that condition to effectively say: If a pollable signals readiness, the next operation _likely won't_ return "EWOULDBLOCK". But no promises.. #Yolo :P

Alex Crichton (Jan 23 2025 at 16:37):

What Dave just said is what I was thinking for a rewording yeah. Including #yolo too!

Last updated: Jan 24 2025 at 00:11 UTC