bytecodealliance / wasmtime-go

Go WebAssembly runtime powered by Wasmtime
https://pkg.go.dev/github.com/bytecodealliance/wasmtime-go
Apache License 2.0
777 stars 75 forks source link

Equivalence for `WasiCtx.insert_file` from wasmtime_wasi Rust crate #187

Open gaukas opened 1 year ago

gaukas commented 1 year ago

Hi, thanks for this amazing project.

I came across the wasmtime_wasi Rust crate and found the WasiCtx struct really handy, and especially for its insert_file function.

While not being able to identify the equivalence of WasiCtx from wasmtime-go, I see WasiConfig implements some similar functions, including SetStderrFile, SetStdinFile, and SetStdoutFile.

I was wondering if I missed anything or is it not yet supported? If so, are there such a plan to support some more (if not all) APIs available to the rust crates?

gaukas commented 1 year ago

What I am trying to achieve is basically passing a socket (say, net.TCPConn) into the wasi. Since wasi-socket is not yet ready, passing fd was the best solution I have for now. But if there are other convenient ways to do so, it also works.

alexcrichton commented 1 year ago

Thanks for the report! I believe this is similar and/or the same as https://github.com/bytecodealliance/wasmtime-go/issues/34 though where the C API doesn't support this yet so the first step here would be to add support there which these bindings could then use.

gaukas commented 1 year ago

Thanks for getting back to me. Is the support for such C APIs an officially planed feature to be implemented?

alexcrichton commented 1 year ago

Just waiting on a PR! No one is currently signed up to make such a PR if that's what you're asking though.

gaukas commented 1 year ago

Thanks for the clarification. May I ask why wasi_config_preopen_socket from wasi.h is not linked to any Go func, while all other functions are properly linked to WasiConfig? Was there ever a discussion on that?

gaukas commented 1 year ago
/**
 * \brief Configures a "preopened" listen socket to be available to WASI APIs.
 *
 * By default WASI programs do not have access to open up network sockets on
 * the host. This API can be used to grant WASI programs access to a network
 * socket file descriptor on the host.
 *
 * The fd_num argument is the number of the file descriptor by which it will be
 * known in WASM and the host_port is the IP address and port (e.g.
 * "127.0.0.1:8080") requested to listen on.
 */
WASI_API_EXTERN bool wasi_config_preopen_socket(wasi_config_t* config, uint32_t fd_num, const char* host_port);

https://github.com/bytecodealliance/wasmtime/blob/2f6e9777cbd0577f9d8a6f236c970c8a8a355dfc/crates/c-api/include/wasi.h#L160-L171

alexcrichton commented 1 year ago

No reason other than someone hasn't gotten around to adding it yet, if you'd like to do that that'd be welcome!

gaukas commented 1 year ago

We are planning on perhaps implementing and linking some of the missing C-APIs related to this. Before a PR is available, are there any compelling reason for wasi_ctx is not a thing in C-API but only built from wasm_config, or will you entertain a Pull Request exporting it in C-API and binding it to Go? Thanks.

alexcrichton commented 1 year ago

That'd be great, and thank you!

I'm not sure I understand what you mean though about wasi_ctx and wasm_config, could you elaborate?

gaukas commented 1 year ago

If I understood correctly, the current flow in wasmtime-go of setting PreopenDir is done by setting them in a wasm_config_t then call wasm_config_t.into_wasi_ctx() to build the WasiCtx in wasmtime_context_set_wasi(). However the pointer to this WasiCtx is not available to the C-API (and there's no such wasi_ctx C-binding for WasiCtx Rust stuct either).

#[cfg(feature = "wasi")]
#[no_mangle]
pub extern "C" fn wasmtime_context_set_wasi(
    mut context: CStoreContextMut<'_>,
    wasi: Box<crate::wasi_config_t>,
) -> Option<Box<wasmtime_error_t>> {
    crate::handle_result(wasi.into_wasi_ctx(), |wasi| {
        context.data_mut().wasi = Some(wasi);
    })
}

https://github.com/bytecodealliance/wasmtime/blob/e4fbf9767673bf055be2dabf4a9dd3e6a05bc64d/crates/c-api/src/store.rs#L195-L204

A downside of this is that while wasmtime (Rust) could update the WasiCtx bound to a Store after the instance has been created/running, wasmtime-go and others who rely on the C-API will not be able to (since they don't have access to the underlying WasiCtx).

alexcrichton commented 1 year ago

Ah ok I think I understand now, and in that case I think it's ok to skip this for now unless needed. AFAIK even in Rust it's intended that you don't do much with WasiCtx after it's created and the WasiCtxBuilder, which is akin to wasi_config_t is where everything happens primarily.

Do you have a use case though for accessing WasiCtx after it's created?

gaukas commented 1 year ago

One possible use case is to allow WASI modules dynamically requesting new sockets/pipes to be made available from the host program (wasmtime), e.g., dialing/listening for TCP.

alexcrichton commented 1 year ago

Good point! For something like this I think I'd recommend exposing it as wasmtime_store_* APIs since it's not going to be easy to separate these into two objects in the C API, but that should be fine otherwise to add.

gaukas commented 1 year ago

@alexcrichton Please see #190. Which is still pending on https://github.com/bytecodealliance/wasmtime/pull/7001.

I actually also implemented a few tests for WasiCtx but they look dirty (basically spin up an instance and call a few exported functions). So I am a bit skeptical about it and didn't include them.