return ERRNO_AGAIN from fd_{read,write} when appropriate

[dicej]

The WASIp1 fd_read and fd_write functions should return Err(ERRNO_AGAIN) rather Ok(0) if a non-blocking operation fails to transfer any bytes. The docs aren't particularly clear about this other than saying these functions are "similar to [readv,writev] in POSIX", but if any functions ought to ever return ERRNO_AGAIN, it's these.

I'm guessing this hasn't been noticed (or at least not reported) until now because most guests use blocking I/O for files, but Go uses non-blocking I/O to support concurrent I/O across multiple goroutines, and it only knows to call poll_oneoff (rather than e.g. assume EOF) if it gets an EAGAIN.

[alexcrichton]

Could you detail a bit more about how this surfaced and came here? I'm effectively not certain this is the best way to fix the underlying issue. The interaction with wasmtime-wasi, files, and nonblocking I/O is ... weird. IIRC wasmtime-wasi just ignores requests for nonblocking I/O on files and always does blocking I/O, and for example that might be the best fix here for the adapter. I think your conclusion here is correct that the adapter isn't doing the right thing with nonblocking I/O, but my thinking is that it's probably wrong for it to try at all.

One example is that nonblocking writes look pretty buggy on the surface:

• Closed streams return 0, which here with this PR would turn into EAGAIN
• Nonblocking writes do a blocking flush

And that may not be the full extent of possible issues here...

[dicej]

The symptom that prompted this PR is that a Go WASIp1 module which attempts to read a file immediately gets what it thinks is an EOF instead of the content when wrapped in a WASIp2 (or WASIp3+WASIp2) component using the p1 adapter. When I investigated, I found that Go explicitly sets all file descriptors to non-blocking using fd_fdstat_set_flags when opening them and expects errno == EAGAIN rather than errno == 0 to indicate non-blocking backpressure.

So we either need to ignore the FDFLAGS_NONBLOCK flag in the adapter's fd_fdstat_set_flags (i.e. force blocking always) or return ERRNO_AGAIN from fd_{read,write} when appropriate. I don't care which, personally.

[dicej]

Closed streams return 0, which here with this PR would turn into EAGAIN

Aren't they supposed to return StreamErr::Closed?

[dicej]

Closed streams return 0, which here with this PR would turn into EAGAIN

Aren't they supposed to return StreamErr::Closed?

Nevermind, I see what you're pointing to now. Yes, I'll need to fix that, but it's fixable.

[alexcrichton]

I suppose in the sake of being conservative it's probably best to apply a fix like this (returning the right codes) instead of behaviorally changing by ignoring nonblocking flags. Another risk to consider here though is that the adapter may have different behavior than the native implementation of WASIp1, which is something we've historically avoided. Could you add a test which exercises this at a higher level? That test should automatically be run by the various implementations with-and-without the adapter which can help validate the behavior in each situation.

If we end up going this route (e.g. a test behaves well across implementations), could you update the read path to look more like the write path though? For example could the BlockingMode::read function return a result-with-wasi-errno instead of the StreamError it does today? That would move the handling of the 0 return value there to look simliar to the write path.