wasmerio / wasmer

🚀 The leading Wasm Runtime supporting WASIX and WASI
https://wasmer.io
MIT License
19.04k stars 814 forks source link

WASIX: `__asyncify_light` ignores the timeout argument #4935

Open andy0130tw opened 4 months ago

andy0130tw commented 4 months ago

Describe the bug

I am implementing nonblocking stdin support for Wasmer and Wasmer JS. During exploring the code, I noticed that fd_read never timed out and is blocked indefinitely despite the detection of NONBLOCK status flag. __asyncify_light was supposed to handle that timeout parameter and errored out, just like what was done in __asyncify_with_deep_sleep, but it appeared to be ignored.

Given all the usage of __asyncify_light are either supplying zero timeout as a nonblocking mechanism or does not use timeout at all, the timeout parameter can be replaced with a simple bool. Most polling logic use the with_deep_sleep version. I guess that the distinction is that not all environment (e.g., Wasmer JS SDK without COEP/COOP) supports deep sleeping.

I use my own fork to reproduce. It is based on an old version, v4.2.5, used by Wasm.js, but this issue presents in the main branch.

wasmer -vV; rustc -vV

wasmer 4.2.5 (129428a 2024-07-16)
binary: wasmer-cli
commit-hash: 129428adb302626cf0ef8d12aec8485a8c102e93
commit-date: 2024-07-16
host: aarch64-apple-darwin
compiler: singlepass,cranelift

rustc 1.70.0 (90c541806 2023-05-31)
binary: rustc
commit-hash: 90c541806f23a127002de5b4038be731ba1458ca
commit-date: 2023-05-31
host: aarch64-apple-darwin
release: 1.70.0
LLVM version: 16.0.2

Steps to reproduce

N/A because nonblocking read is not available under normal circumstances, but it might be patched like above to allow one. It is actually better tested in Wasmer JS SDK since the blocking read from Tokio's stdin implementation cannot be cancelled. I can provide more context if required.

Expected behavior

Under nonblocking mode set to stdin (fd=0), fd_read returns immediately no matter whether any data is available.

Actual behavior

fd_read blocks until some data is available.

Additional context

To work toward a fix, I would like to first understand the distinction between the light version and the deep sleep one, and if a zero-or-infinite timeout makes sense, e.g., actually "lighter", provides more compatibility, etc.

syrusakbary commented 4 months ago

@john-sharratt @theduke can you provide some context on each of those? Thanks!