Wasmtime GitHub notifications bot (Dec 03 2024 at 22:36): Wasmtime GitHub notifications bot (Dec 03 2024 at 22:36):jeffcharles added the bug label to Issue #9723.
Wasmtime GitHub notifications bot (Dec 03 2024 at 22:36):jeffcharles opened issue #9723:
Thanks for filing a bug report! Please fill out the TODOs below.
Note: if you want to report a security issue, please read our security policy!
Test Case
The source code for the Wasm module looks like:
use std::io::{self, Read}; #[export_name = "wizer.initialize"] pub fn initializer() { let mut buf = vec![]; io::stdin().read_to_end(&mut buf).unwrap(); }The source code for the host looks like:
use wasmtime::{Engine, Linker, Module, Store}; use wasmtime_wasi::{pipe::MemoryInputPipe, preview1::WasiP1Ctx, WasiCtxBuilder}; fn main() { let engine = Engine::default(); let module = Module::from_file( &engine, "check_read_to_end.wasm", ) .unwrap(); let mut linker = Linker::new(&engine); let mut store = Store::new(&engine, Data { wasi: None }); wasmtime_wasi::preview1::add_to_linker_sync(&mut linker, |cx: &mut Data| { cx.wasi = Some( WasiCtxBuilder::new() .inherit_stdout() .inherit_stderr() .stdin(MemoryInputPipe::new(vec![1])) .build_p1(), ); cx.wasi.as_mut().unwrap() }) .unwrap(); let instance = linker.instantiate(&mut store, &module).unwrap(); instance .get_typed_func::<(), ()>(&mut store, "wizer.initialize") .unwrap() .call(&mut store, ()) .unwrap(); } struct Data { wasi: Option<WasiP1Ctx>, }Steps to Reproduce
- Ungzip the Wasm file (or compile it) and move it to
check_read_to_end.wasm.cargo runthe main program for the host.Expected Results
The function call to run and exit.
Actual Results
The function call hangs.
It looks like the
wasmtime_wasi::preview1::add_to_linker_sync's closure is run infinitely and in my debugger I can see theMemoryInputPipe'sreadbeing called infinitely.Versions and Environment
Wasmtime version or commit: 27.0.0 and also 23.0.3
Operating system: MacOS
Architecture: AArch64
Extra Info
Anything else you'd like to add?
Normally I'd define the WASI context on the store but I'm using Wizer which doesn't expose a public API that I can use to do that so this is a workaround I tried. However, I found it very unexpected that this causes a hang.
Wasmtime GitHub notifications bot (Dec 04 2024 at 04:27):pchickey commented on issue #9723:
I think the bug here is that you are creating a new WasiP1Ctx in the closure to add_to_linker, which is a “getter” function into your Ctx type, when the usual place to create it is in Store::new - if you change that does the behavior resolve?
Wasmtime GitHub notifications bot (Dec 04 2024 at 14:15):jeffcharles commented on issue #9723:
Yes it does resolve. However, as I mentioned in the issue:
Normally I'd define the WASI context on the store but I'm using Wizer which doesn't expose a public API that I can use to do that so this is a workaround I tried.
I did file https://github.com/bytecodealliance/wizer/issues/123 to ask for an example of how to define a custom WASI context when using Wizer.
It might be worth augmenting the documentation around
add_to_linker_syncand similar methods to clarify the closure may execute one or more times during an invocation of an exported Wasm method. That was definitely missing from my mental model and surprised me.
Wasmtime GitHub notifications bot (Dec 04 2024 at 16:07):pchickey commented on issue #9723:
Sorry, I missed that context at the end of your post. Yes, we should improve the docs to describe that those linker closures shouldn’t perform side effects.
Wasmtime GitHub notifications bot (Dec 04 2024 at 21:50):alexcrichton commented on issue #9723:
I'll admit I'm a bit lost about what to do about this. The problem here is that you're re-initializing
WasiCtxso the guest itself is repeatedly reading and getting more data. The guest is waiting for a 0 return value signaling "eof" but every time it reads the context is reinitialized which provides more data.We could certainly provide more notes in
add_to_linkerand friends about the intent of the closure, but would that really help here? The documentation already says that the closure should be a "projection" which isn't as clear as it could be, but regardless that's a pretty niche location to document this and I suspect when the app here "hangs" you probably wouldn't go consultadd_to_linker_syncas the first thing to see what went wrong.So all-in-all I agree that the docs of
add_to_linker_synccould be improved, but would that really meaningfully change the experience here?
Wasmtime GitHub notifications bot (Dec 04 2024 at 22:49):jeffcharles commented on issue #9723:
The problem here is that you're re-initializing WasiCtx so the guest itself is repeatedly reading and getting more data. The guest is waiting for a 0 return value signaling "eof" but every time it reads the context is reinitialized which provides more data.
So technically it's the
MemoryInputPipe::new(...)inside the closure that's introducing the problem for me, not the context reinitialization (even if it's not ideal).would that really help here?
That's admittedly subjective. In my case, it would have. I actually was looking at how this function was called first when my program started hanging because my initial implementation was locking a mutex inside this closure to retrieve the byte array I wanted to pass to
stdinand I wanted to check that I didn't introduce a deadlock which is what I thought was happening initially. And then I got confused when I noticed the closure was being called more than once because that didn't align with my prior mental model.Don't get me wrong, I would rather not re-initialize a
WasiCtxin that closure, that's just a result of me using Wizer and Wizer seemingly not letting me set the stdin for itsWasiCtx. But, it really wasn't clear to me thatadd_to_linker_syncwould be called while inside a Wasm call.
Last updated: Dec 23 2024 at 13:07 UTC