Stream: git-wasmtime

Topic: wasmtime / issue #7001 new: declare type wasi_ctx in c-api

view this post on Zulip Wasmtime GitHub notifications bot (Sep 11 2023 at 22:31):

gaukas commented on issue #7001:

fixing Rustfmt...

view this post on Zulip Wasmtime GitHub notifications bot (Sep 11 2023 at 22:43):

gaukas edited a comment on issue #7001:

fixing Rustfmt... Done.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 11 2023 at 22:43):

gaukas edited a comment on issue #7001:

Fixing Rustfmt... [DONE]

view this post on Zulip Wasmtime GitHub notifications bot (Sep 12 2023 at 00:16):

pchickey commented on issue #7001:

I missed the discussion in https://github.com/bytecodealliance/wasmtime-go/issues/187 and I am a little nervous about these changes - they can be supported today, but will have to change substantially in the future.

In our preview 2 implementation, we have eliminated all of the user-facing methods on WasiCtx. This is required due to new invariants that we need to maintain for preview 2 resources. You can still insert directories into a WasiCtx while in the builder. We also eliminated the WasiFile/WasiDir traits as a mechanism for extending the Wasi implementation. We do have extension traits for implementing streams (which maybe will be what you need, to expose pipes?) and a totally new interface and implementation for sockets. The story for how to expose these new interfaces to guests is itself a bit involved as well - we have adapters to transition modules consuming just Wasi preview 1, but it sounds like you are using preview 1 plus some other imports as well.

We expect the preview 2 builder/context (defined in wasmtime-wasi::preview2) will supplant the preview 1 implementation (defined in wasi-common) pretty soon, and preview 1 will be supported through the p2->p1 host compatibility layer (wasmtime-wasi::preview2::preview1). We will be supporting both the new and old Wasi implementations a little while for users to transition, but that is much easier to manage in Rust than in the C API. The existing C APIs which end up connecting to wasi-common are pretty easy to transition over wasmtime-wasi::preview2, but these methods will not be possible to transition.

One way we can bypass the many wrinkles in this story is if we can switch you over to using Wasi Preview 2 and the Component Model. That still has open questions about how to use the C FFI, but those are questions that it will benefit a lot of users to solve, whereas if we expose these methods for just you to use today, we'll also have to solve other problems for just you in a few months when it is time to transition to preview 2.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 12 2023 at 00:17):

pchickey edited a comment on issue #7001:

I missed the discussion in https://github.com/bytecodealliance/wasmtime-go/issues/187 and I am a little nervous about these changes - they can be supported today, but will have to change substantially in the future.

In our preview 2 implementation, we have eliminated all of the user-facing methods on WasiCtx. This is required due to new invariants that we need to maintain for preview 2 resources. You can still insert (host filesystem) directories into a WasiCtx while in the builder. We also eliminated the WasiFile/WasiDir traits as a mechanism for extending the Wasi implementation. We do have extension traits for implementing streams (which maybe will be what you need, to expose pipes?) and a totally new interface and implementation for sockets. The story for how to expose these new interfaces to guests is itself a bit involved as well - we have adapters to transition modules consuming just Wasi preview 1, but it sounds like you are using preview 1 plus some other imports as well.

We expect the preview 2 builder/context (defined in wasmtime-wasi::preview2) will supplant the preview 1 implementation (defined in wasi-common) pretty soon, and preview 1 will be supported through the p2->p1 host compatibility layer (wasmtime-wasi::preview2::preview1). We will be supporting both the new and old Wasi implementations a little while for users to transition, but that is much easier to manage in Rust than in the C API. The existing C APIs which end up connecting to wasi-common are pretty easy to transition over wasmtime-wasi::preview2, but these methods will not be possible to transition.

One way we can bypass the many wrinkles in this story is if we can switch you over to using Wasi Preview 2 and the Component Model. That still has open questions about how to use the C FFI, but those are questions that it will benefit a lot of users to solve, whereas if we expose these methods for just you to use today, we'll also have to solve other problems for just you in a few months when it is time to transition to preview 2.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 12 2023 at 00:21):

pchickey edited a comment on issue #7001:

I missed the discussion in https://github.com/bytecodealliance/wasmtime-go/issues/187 and I am a little nervous about these changes - they can be supported today, but will have to change substantially in the future.

In our preview 2 implementation, we have eliminated all of the user-facing methods on WasiCtx. This is required due to new invariants that we need to maintain for preview 2 resources. You can still insert (host filesystem) directories into a WasiCtx while in the builder. We also eliminated the WasiFile/WasiDir traits as a mechanism for extending the Wasi implementation. We do have extension traits for implementing streams (which maybe will be what you need, to expose pipes?) and a totally new interface and implementation for sockets. The story for how to expose these new interfaces to guests is itself a bit involved as well - we have adapters to transition modules consuming just Wasi preview 1, but it sounds like you are using preview 1 plus some other imports as well.

We expect the preview 2 builder/context (defined in wasmtime-wasi::preview2) will supplant the preview 1 implementation (defined in wasi-common) pretty soon, and preview 1 will be supported through the p2->p1 host compatibility layer (wasmtime-wasi::preview2::preview1). We will be supporting both the new and old Wasi implementations a little while for users to transition, but that is much easier to manage in Rust than in the C API. The existing C APIs which end up connecting to wasi-common are pretty easy to transition over wasmtime-wasi::preview2, but these methods will not be possible to transition.

One way we can bypass the many wrinkles in this story is if we can switch you over to using Wasi Preview 2 and the Component Model. That still has open questions about how to use the C FFI, but those are questions that it will benefit a lot of users to solve, whereas if we expose these methods for just you to use today, we'll also have to solve other problems for just you in a few months when it is time to transition to preview 2.

This is a pretty complex design space which doesn't have much by the way of docs, so if I can help you understand this problem better by discussing it, we can talk on the bytecode alliance zulip or setup a call sometime.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 12 2023 at 00:42):

gaukas commented on issue #7001:

I understand your concern. The reason for this PR to exist is simply that the Go's implementation of wasmtime (as wasmtime-go) has been missing way too many features from its Rust counterpart, and we really need BOTH to work on the same page. There's a set of projects we are working on right now and part of its job can ONLY be done in Go. However the gap between two implementation due to the absence of relevant C-API is irritating, and it feels like it is never a part of plan to allow the users with Go to do as much as Rust users can. If so, it is pretty sad, as I believe Go has its significance as much as Rust do.

It is not a bad idea to switch over to wasi preview2, but I am concerned about the amount of work needed to fill the gap in wasmtime-go, given it looks like not as popular and already fell short on features in preview 1. In the long run I believe it is definitely worth considering, though, if it implements a much more consistent feature set in across all ports. Specifically, the ability to inject sockets AFTER instance has been created/running.

A discussion will be nice to have, we can start by setting up a covert communication channel. Please feel free to shoot me an email if you wish.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 12 2023 at 01:56):

github-actions[bot] commented on issue #7001:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Sep 12 2023 at 17:55):

pchickey commented on issue #7001:

Wasmtime's C FFI isn't fully fleshed out, not because we don't want it to be, but because we have had other priorities. We are happy to help contributors add C FFI support for the parts of wasmtime where it is missing right now.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 12 2023 at 21:28):

pchickey commented on issue #7001:

I looked at this again and talked to @alexcrichton about how we can move this over to preview 2 and I'm comfortable with it once he signs it off.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 14 2023 at 15:26):

alexcrichton commented on issue #7001:

Personally I don't think that void* should be used here as a placeholder for HANDLE-or-int. I realize this is what Go seems to do with uintptr as the return value from the Fd() method, but that's not how I'd personally do it. That being said I also don't have a lot of prior experience to draw from. Everything in Rust is "just use File" which isn't an option in C and otherwise I'm not aware of any C libraries that handle the Windows/Unix distinction as they generally take an fd and then don't work on Windows. To me though casting int in C to void* just to pass it to this function feels a bit too weird.

Otherwise though another comment I would have is that #[cfg] can be applied on a much more granular level than the entire function, so I'd recommend avoiding duplication of the entire function bodies in Rust with something like:

#[cfg(unix)]
let file = File::from_raw_fd(the_argument);
#[cfg(windows)]
let file = File::from_raw_handle(the_argumet);

(or something like that)

view this post on Zulip Wasmtime GitHub notifications bot (Sep 14 2023 at 16:08):

gaukas commented on issue #7001:

So do we have a conclusion here, shall we include multiple functions for Unix and Windows to have explicit types (int32/i32 and void*/HANDLE), or do you think it is okay to save them for later and keep it as you described 

#[cfg(unix)]
let file = File::from_raw_fd(the_argument);
#[cfg(windows)]
let file = File::from_raw_handle(the_argumet);

view this post on Zulip Wasmtime GitHub notifications bot (Sep 14 2023 at 19:34):

alexcrichton commented on issue #7001:

All I can really offer here is my own personal opinion and preference. My preference is that the types of the argument reflect what the system uses, which in C is int on Unix (not int32_t) and HANDLE on Windows. The Rust version of those is Raw* in the standard library. I don't have a strong preference on whether there's two different names for the function, but I think it's reasonable to have the same symbol with a platform-specific signature, meaning just one function with different types depending on the platform.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 15 2023 at 06:28):

gaukas commented on issue #7001:

Now using RawFd and RawHandle.

On C's side:

Unix: 

WASM_API_EXTERN void wasmtime_context_insert_file(wasmtime_context_t *context, uint32_t guest_fd, int host_fd, uint32_t access_mode);
WASM_API_EXTERN wasmtime_error_t *wasmtime_context_push_file(wasmtime_context_t *context, int host_fd, uint32_t access_mode, uint32_t *guest_fd);

Windows: 

WASM_API_EXTERN void wasmtime_context_insert_file(wasmtime_context_t *context, uint32_t guest_fd, void *host_handle, uint32_t access_mode);
WASM_API_EXTERN wasmtime_error_t *wasmtime_context_push_file(wasmtime_context_t *context, void *host_handle, uint32_t access_mode, uint32_t *guest_fd);

Tried to use HANDLE from <winnt.h> but it turns out CGO won't even work, throws a bunch of errors such as 

error: unknown type name 'DWORD'
error: unknown type name 'WORD'

Defining typedef void *HANDLE in store.h didn't work out for me either, I failed to figure out a way to convert the uintptr into the self-defined C.HANDLE type. Casting into C.HANDLE() simply does not work. Therefore, here I choose to stick with void* in C-API, since it is just its true identity. Please feel free to push any further changes, but I guess that will be all I have for this.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 16 2023 at 01:56):

gaukas commented on issue #7001:

I guess it is time for someone from bytecodealliance to deal with the PR, if you want to make a change, do it. I allowed changes by maintainers.

I already got bored of this low communication efficiency and have much more prioritized work to do. And all I wanted could already be achieved with my fork, and for code style issue, please, no one can reach the agreement with others so push changes rather than suggesting one. I'm pissed and I hope I made that obvious.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 16 2023 at 02:01):

gaukas commented on issue #7001:

![image](https://github.com/bytecodealliance/wasmtime/assets/9084527/bfad862e-3209-4cae-931f-92128357dcc0)

view this post on Zulip Wasmtime GitHub notifications bot (Sep 18 2023 at 14:05):

alexcrichton commented on issue #7001:

Ok thanks for the initial effort here. I'm going to close this and others can pick this up if they're interseted. If you feel inspired in the future feel free to reopen and/or resubmit as well.

Last updated: Nov 22 2024 at 16:03 UTC