-
Notifications
You must be signed in to change notification settings - Fork 35
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
Underlying Wasm runtime, wasmer -> wasmtime #336
Comments
Thanks for asking! 💜 The only problem is my limited free time. Do you have the time to take over and/or help here? |
@tessi would love to help out here, especially if you're close! Would you be willing to publish that fork and give a quick TL;DR on what you've tried and what's not working? |
Hey @brooksmtownsend 👋 It turned out to be a way bigger refactoring than anticipated and, unfortunately, it is not fully working at the moment. This is where I could really use some of your help :)
|
Regarding the store-deadlock problem, I just realized that our wrapped functions receive a Caller which happens to implement I think we need to find a way to extract this caller into the elixir fn callback_context that we create here.If we have that done, we could pass the caller (acting as a store) instead of the original deadlocked store. |
Maybe we can get inspiration from wasmtime-py, they interface the C API, but they do exactly what I proposed above. They take the caller and pass them as a reference to the python function. I guess they can get around lifetime issues since C doesn't have lifetimes :D but maybe there is a (unsafe?) Rust way to do similar. |
Thank you for the great updates @tessi, I'm following along here but haven't had much free time to look into it yet |
no worries. I'm in the same boat :) |
I think i can report progress. I just pushed another (still very broken) version of wasmex-wasmtime which introduces a new concept
So far for the theory :D In practice, when doing the above, wasmtime panics because it thinks we're using a different store somehow and I don't know why yet. I added some debug code here which illustrates the problem and pointer-hack, but simplifies it as much as possible. Within a wrapped function, I can read memory using the originally provided let mut buffer = [0];
memory.read(&caller, 0, &mut buffer).unwrap();
println!("buffer 1: {:?}", buffer); however, after doing that pointer hack - which should give us the same caller because we read from the same memory address - we get a panic: let raw_ptr: *mut Caller<StoreData> = &mut caller as *mut Caller<StoreData>;
let the_other_caller: &mut Caller<StoreData> = unsafe { &mut *raw_ptr as &mut Caller<StoreData> };
memory.read(the_other_caller, 0, &mut buffer).unwrap();
println!("buffer 2: {:?}", buffer);
// thread '<unnamed>' panicked at 'object used with the wrong store',
// /Users/tessi/.cargo/registry/src/github.com-1ecc6299db9ec823/wasmtime-0.39.1/src/store/data.rs:258:5 you can play with that scenario, when checking out I hope someone reading this can help me find out why wasmtime thinks this is a new/different store and maybe even fix the problem 🙏 |
Good news! together with @hamptokr we could fix the memory problem and reading/writing memory from within function calls now fully works. In fact, all tests - with the exception of WASI tests as I didn't look at WASI yet - work \o/ Next steps: Fixing WASI and cleaning things up |
Keeping you updated here: WASI seems to work roughly - I still have to fix file access permissions, but 5/10 wasi tests work. This is getting closer :) |
Perfect timing here, Wasmtime just announced 1.0! Very exciting 😄 |
super exciting indeed! :) I worked some more on the wasmtime fork and all test are green now 🎉 However, it works only on my wasmtime fork. Next task is to contribute my wasmtime changes back so we can have proper access permissions for preopened directories. I'm currently asking the wasmtime folks how to contribute my patch back here. |
However, @brooksmtownsend it would be great if you could give that fork a try and see if it works for you. Some things (especially the WASI API) changed, but I hope the changes are easy to spot reading the source. But if not, please, ask :) Writing docs and a migration guide is another open TODO. |
Turns out that the wasmtime folks recommended to not have access permissions for WASI preopens yet but rather wait it to be implemented "proper" in the future (ticket). So I removed use of my custom wasmtime fork and instead upgraded to wasmtime 4.0. Any preopened directory will come with full access permissions until wasmtime allows us to restrict access. In addition, I moved all WASI opts (but especially the directory preopen options) into Elixir structs. This makes it easier to upgrade them in the future because we will get proper compile-time errors whith missing or changed keys (e.g. for those future access permissions). This probably even improves usability. |
@brooksmtownsend did you have the time to try things out yet? No worries if not :) I'll soon merge my work back into this repo (in some dev-branch, I guess) to prepare docs, polish, and release the new version |
The MR that brings the changes back is here and looks good. Docs are updated, changelog is written. |
Alright, merged the MR 🚢 |
Slightly related to #282
So wasmer is the underlying runtime at the moment for wasmex, would you consider switching the underlying runtime of this project to wasmtime like https://github.com/viniarck/wasmtime-ex has?
We've loved using this project and are looking to take advantage of the features that come first to wasmtime, and in a perfect world this would be done through swappable runtimes like #282. In the shorter term, is there any reservation to changing the runtime?
Thanks @tessi 🚀
The text was updated successfully, but these errors were encountered: