Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Equivalence for WasiCtx.insert_file from wasmtime_wasi Rust crate #187

Open
gaukas opened this issue Aug 22, 2023 · 14 comments
Open

Equivalence for WasiCtx.insert_file from wasmtime_wasi Rust crate #187

gaukas opened this issue Aug 22, 2023 · 14 comments

Comments

@gaukas
Copy link

gaukas commented Aug 22, 2023

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
Copy link
Author

gaukas commented Aug 22, 2023

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
Copy link
Member

Thanks for the report! I believe this is similar and/or the same as #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
Copy link
Author

gaukas commented Aug 22, 2023

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

@alexcrichton
Copy link
Member

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
Copy link
Author

gaukas commented Aug 23, 2023

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
Copy link
Author

gaukas commented Aug 23, 2023

/**
 * \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
Copy link
Member

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
Copy link
Author

gaukas commented Sep 6, 2023

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
Copy link
Member

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
Copy link
Author

gaukas commented Sep 6, 2023

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
Copy link
Member

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
Copy link
Author

gaukas commented Sep 6, 2023

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
Copy link
Member

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
Copy link
Author

gaukas commented Sep 12, 2023

@alexcrichton Please see #190. Which is still pending on bytecodealliance/wasmtime#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants