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

Allow custom imports #369

Open
FranklinWaller opened this issue Dec 14, 2023 · 6 comments · May be fixed by #406
Open

Allow custom imports #369

FranklinWaller opened this issue Dec 14, 2023 · 6 comments · May be fixed by #406
Assignees

Comments

@FranklinWaller
Copy link

Our runtime is half based on WASI with a few custom functions that are specific for blockchains. We use Wasmer-js for the SDK
(And Wasmer-rust for the node). In previous versions of Wasmer-js we could specify extra custom imports (through passing WebAssembly.instantiate(module, customImports);). However in the latest Wasmer-js it doesn't seem like we can pass in custom imports anymore.
Is this something that will eventually? We are looking to upgrade due the WASIX support.

Copy link

linear bot commented Dec 14, 2023

@Michael-F-Bryan
Copy link
Contributor

This is a limitation of the current wasmer_wasix::WasiEnvBuilder API. We need to update it so you can provide your own custom imports, then add them to the imports object here.

Once that is in place, it should be possible to thread the imports through the wasmer_wasix::runners::WasiRunner, and then add an adapter to the @wasmer/sdk package which lets you use JavaScript functions as the imports.

@Michael-F-Bryan Michael-F-Bryan added the Feature label Dec 18, 2023 — with Linear
@Michael-F-Bryan Michael-F-Bryan self-assigned this Dec 18, 2023
@Michael-F-Bryan Michael-F-Bryan added the JS label Dec 19, 2023 — with Linear
@Michael-F-Bryan
Copy link
Contributor

It turns out custom imports aren't compatible with the way we do multi-threading.

A big part of the @wasmer/sdk API is how we'll execute WebAssembly modules on a background thread (Web Worker) and give users an Instance handle they can use to communicate with the actual instance (reading stdout/stderr, writing to stdin, waiting for it to exit, etc.). This causes issues with custom imports because the JavaScript values you pass in are tied to the JavaScript VM and can't be accessed from other threads.

I've worked around this in the past by setting up the API so values are passed between threads via postMessage() (see the Scheduler abstraction), but that's just not possible in this scenario because @wasmer/sdk doesn't have control over how the wasmer::Imports object is passed to workers.

Another approach I took in #337 was to create a proxy object which would implement virtual_fs::FileSystem by sending messages to a listener running on the FileSystemDirectoryHandle's original thread.

impl<A> Mailbox<A> {
/// Spawn an actor on the current thread.
pub(crate) fn spawn(mut actor: A) -> Self
where
A: 'static,
{
let (sender, mut receiver) = mpsc::channel::<Thunk<A>>(1);
let original_thread = wasmer::current_thread_id();
wasm_bindgen_futures::spawn_local(
async move {
while let Some(thunk) = receiver.next().await {
thunk(&mut actor).await;
}
}
.in_current_span(),
);
Mailbox {
original_thread,
sender,
}
}

However, this approach won't work for custom imports because sending messages is an async operation and functions passed to wasmer::Imports need to be synchronous. It also runs into problems when dealing with imports like WebAssembly.Memory or externref because you need the actual WebAssembly.Memory value when instantiating - a proxy or copy isn't good enough.

Unfortunately, I think we'll need to close this issue because it doesn't appear possible within the constraints of our current architecture.

@Michael-F-Bryan Michael-F-Bryan closed this as not planned Won't fix, can't repro, duplicate, stale Jan 17, 2024
@Michael-F-Bryan Michael-F-Bryan linked a pull request Jan 17, 2024 that will close this issue
@FranklinWaller
Copy link
Author

That's ok, thank you for taking a look @Michael-F-Bryan
Maybe with #370 we can do something similiar and less rely on custom imports and more on something that is also part of WASI(X)

@theduke theduke reopened this May 13, 2024
@theduke
Copy link

theduke commented May 13, 2024

Re-opening because this is still a valid feature request.

@mfcekirdek
Copy link

I also need this feature for my use case

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

Successfully merging a pull request may close this issue.

4 participants