Make fd_write unbuffered; fixes CraneStation/wasmtime#255#55
Make fd_write unbuffered; fixes CraneStation/wasmtime#255#55sunfishcode merged 1 commit intoCraneStation:masterfrom kubkon:unbuffered
Conversation
| let nwritten = stderr.write_vectored(&iovs); | ||
| stderr | ||
| .flush() | ||
| .map_err(|err| err.raw_os_error().map_or(host::__WASI_EIO, errno_from_host))?; |
There was a problem hiding this comment.
Unlike io::stdout, io::stderr is not buffered, so we don't need to flush it.
src/hostcalls_impl/fs.rs
Outdated
| Descriptor::File(f) => { | ||
| let nwritten = f.write_vectored(&iovs); | ||
| f.flush() | ||
| .map_err(|err| err.raw_os_error().map_or(host::__WASI_EIO, errno_from_host))?; |
There was a problem hiding this comment.
On the other hand, it may be a good idea to add the currently-no-op flush in case the Rust implementation decides to do some buffering. Or to clarify the docs.
There was a problem hiding this comment.
@sunfishcode I think you're spot on! @marmistrz I'm partial to leaving flushing out of this one, and clarifying Rust's docs instead. Then, if anything changes we should in principle be able to track it down through the changes in the docs. :-)
There was a problem hiding this comment.
Also, if Rust ever does decide to make File's write etc. buffered, it'd be better to switch to a lower-level API without buffering, rather than use buffering and flushing.
It's unfortunate that we have to deal with stdout buffering, but it is useful to coordinate with io::stdout so that we interop with Rust code in the host.
| let nwritten = stdout.write_vectored(&iovs); | ||
| stdout | ||
| .flush() | ||
| .map_err(|err| err.raw_os_error().map_or(host::__WASI_EIO, errno_from_host))?; |
There was a problem hiding this comment.
This is really subtle, but the ordering here means that errors from the flush call are reported before errors from the write_vectored call. I admittedly can't think of a case where it would matter offhand, but it feels untidy to check errors out of order.
There was a problem hiding this comment.
Ah, of course! Well spotted, thanks!
|
@kubkon ok |
|
Looks good! |
This PR fixes bytecodealliance/wasmtime#255 by making
fd_writeunbuffered. AFAIK,std::io::Readandstd::io::Writeall do buffered reads and writes respectively. I'm not sure whether the fact that our implementation offd_readuses buffered reads is a problem?@marmistrz I've not re-used your fn
errno_from_ioerroras I didn't want to create additional conflicts for your PR #46.