Use wasmtime-wasi APIs instead of wasi-common#448
Conversation
| } | ||
| } | ||
|
|
||
| impl StdinStream for FileStdinStream { |
There was a problem hiding this comment.
The wasmtime-wasi API requires types passed to stdin to implement StdinStream and I couldn't find an existing type that implementing this trait for file input.
| use wasmtime_wasi::{FileInputStream, StdinStream}; | ||
|
|
||
| pub struct FileStdinStream { | ||
| file: Arc<File>, |
There was a problem hiding this comment.
I don't actually know what fields this struct should have. I tried making a similar implementation to how OutputFile works but the types used by the constructor functions for are different.
|
|
||
| impl io::Write for OutputLimitedBuffer { | ||
| fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> { | ||
| impl OutputLimitedBuffer { |
There was a problem hiding this comment.
We still have to use a custom type since the default in-memory implementation will trap when its capacity is exceeded and it looks like we don't want to trap when the capacity for our implementation is exceeded
| use std::sync::{Arc, Mutex}; | ||
| use wasmtime_wasi::{OutputStream, Pollable, StdoutStream, StreamError, StreamResult}; | ||
|
|
||
| pub struct WritePipe { |
There was a problem hiding this comment.
We need a custom type so we can implement StdoutStream which effectively requires wrapping all fields in OutputLimitedBuffer in the same Arc.
| fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> { | ||
| impl OutputLimitedBuffer { | ||
| fn check_write(&mut self) -> StreamResult<usize> { | ||
| Ok(usize::MAX) |
There was a problem hiding this comment.
The caller will invoke check_write and then invoke write with a buffer that fits in the usize returned. If it repeatedly returns 0, then the caller will hang indefinitely so this shouldn't return 0.
| if wasi { | ||
| wasi_common::sync::add_to_linker(&mut inner, |s| s.wasi_ctx_mut()) | ||
| .map_err(|e| error!("{}", e))? | ||
| wasmtime_wasi::preview0::add_to_linker_sync(&mut inner, |s| s.wasi_ctx_mut()) |
There was a problem hiding this comment.
The old implementation added both preview 0 and preview 1 to the linker so copied that here. There's an argument to be made this should only add preview 1 and not preview 0.
| /// @return [WasiCtx] | ||
| pub fn deterministic() -> Self { | ||
| Self { | ||
| inner: RefCell::new(wasi_deterministic_ctx()), |
There was a problem hiding this comment.
There is no equivalent for this call with wasmtime-wasi. Instead there's a method that accepts a WasiCtxBuilder as input.
| /// @def set_stdin_file(path) | ||
| /// @param path [String] The path of the file to read from. | ||
| /// @return [WasiCtxBuilder] +self+ | ||
| fn set_stdin_file(rb_self: RbSelf, path: RString) -> RbSelf { |
There was a problem hiding this comment.
The rest of the deleted calls have no equivalent calls on WasiP1Ctx.
| /// Make the context deterministic. See See https://github.com/Shopify/deterministic-wasi-ctx for more details | ||
| /// @def add_determinism() | ||
| /// @return [WasiCtxBuilder] +self+ | ||
| pub fn add_determinism(rb_self: RbSelf) -> RbSelf { |
There was a problem hiding this comment.
Note that we still need to call replace_scheduling_functions (and the preview 0 equivalent) with deterministic_wasi_ctx passing the linker as an argument if there's a desire to prevent modules from being able to sleep.
| } | ||
| ReadStream::Path(path) => builder.stdin( | ||
| file_r(ruby.get_inner(*path)) | ||
| .map(|path| FileStdinStream::new(FileInputStream::new(&path, 0)))?, |
There was a problem hiding this comment.
I cannot figure out how to construct FileInputStream. The link for the &File type in https://docs.rs/wasmtime-wasi/31.0.0/wasmtime_wasi/struct.FileInputStream.html#method.new doesn't exist. Looking at the underlying code, the File type used does not appear to be exported by wasmtime-wasi. We may want to write our own equivalent of FileInputStream.
| async-trait = "*" | ||
| bytes = "*" |
There was a problem hiding this comment.
These two are required by wasmtime-wasi traits we have to implement.
|
There's an additional compilation failure with |
1 participant
This is not ready, it will not compile.
This is my work in progress for switching
wasmtime-rbto usewasmtime-wasiAPIs instead ofwasi-common. This will involve having to make breaking API changes because thewasmtime-wasiAPIs are quite different fromwasi-common's. Particularly around not being able to change thestdiostreams after creating aWasiP1Ctx.