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

create_resource panics when server is compiled to wasm32-wasi #2056

Closed
itowlson opened this issue Nov 22, 2023 · 8 comments · Fixed by #2066
Closed

create_resource panics when server is compiled to wasm32-wasi #2056

itowlson opened this issue Nov 22, 2023 · 8 comments · Fixed by #2066
Labels
bug Something isn't working

Comments

@itowlson
Copy link
Contributor

Describe the bug

I am using SSR and building the server as a Spin application. This means the server is compiled to wasm32-wasi rather than native code.

When I tried to run a variant of the counter_isomorphic example, the server panicked on the create_resource call (based on

let counter = create_resource(
), with the message "cannot call wasm-bindgen imported functions on non-wasm targets', /home/ivan/.cargo/registry/src/index.crates.io-6f17d22bba15001f/js-sys-0.3.65/src/lib.rs:5970:9".

The relevant part of the backtrace is:

           6: 0x1fe6d0 - <unknown>!js_sys::global::get_global_object::h905e1441bf8294a8
           7: 0x1fe6b7 - <unknown>!js_sys::global::h98e3a07765e5d726
           8: 0x1fe3a0 - <unknown>!wasm_bindgen_futures::queue::Queue::new::h6d3bb2f62074b61c
           9: 0x1fe462 - <unknown>!wasm_bindgen_futures::task::singlethread::Task::spawn::hf198b6ecb05f055e
          10: 0x18dae - <unknown>!std::thread::local::LocalKey<T>::with::h472ebb6959128837
          11: 0x2ba3b - <unknown>!<leptos_reactive::memo::Memo<T> as leptos_reactive::signal::SignalWith>::try_with::hb826f2fa273592e0
          12: 0x33bff - <unknown>!leptos_reactive::resource::ResourceState<S,T>::load::hcb00091d53adc33a
          13: 0x1e295 - <unknown>!<leptos_reactive::effect::EffectState<T,F> as leptos_reactive::effect::AnyComputation>::run::h7382e9f0cd086cde
          14: 0x1f2326 - <unknown>!leptos_reactive::runtime::Runtime::update_if_necessary::h7ec72211ebe3fde0
          15: 0x1a035 - <unknown>!std::thread::local::LocalKey<T>::with::h7e93c221acbbee39
          16: 0x354f1 - <unknown>!leptos_reactive::resource::create_resource_helper::h909cdad642bfb8f2
          17: 0x341cb - <unknown>!leptos_reactive::resource::create_resource_with_initial_value::hf6fa4f40ea300293
          18: 0x33eda - <unknown>!leptos_reactive::resource::create_resource::h28393832763b21dd

The issue appears to arise in the leptos-reactive spawn_local function (

if #[cfg(target_arch = "wasm32")] {
), where if the target architecture is wasm32 it uses wasm_bindgen_futures::spawn_local, which depends on access to JavaScript globals (https://github.com/rustwasm/wasm-bindgen/blob/bb12058e1facd26c5c647282a125088535be577e/crates/futures/src/queue.rs#L88). In Spin SSR, the server code is Wasm but JavaScript is not present, hence, panic.

Leptos Dependencies

leptos = { version = "0.5" }
leptos_integration_utils = { version = "0.5.2", optional = true }
leptos_meta = { version = "0.5" }
leptos_router = { version = "0.5" }

The server environment was Spin 2.0.1, although this is of course not an official integration, and is very much experimental at this point!

To Reproduce
Steps to reproduce the behavior:

  1. Install the Spin CLI.
  2. If you don't already have it, install the Rust wasm32-wasi target
  3. Clone https://github.com/itowlson/spin-leptos-investigation and checkout out the counter-isomorphic branch
  4. Run cargo leptos build --release (this builds the client side Wasm and the assets; we ignore the native binary)
  5. Run spin up --build (this builds the app with the ssr feature, and runs it)
  6. Open localhost:3000 in the browser

Expected behavior

Expected to see Incr and Decr buttons and a counter that went up and down

Additional context

  1. I understand that what I am doing is completely experimental, and I am very much finding my way. It's entirely possible I've gone about this the wrong way and the problem is all on my end!

  2. I had a crack at fixing it by adding the following clause at the top of the spawn_local function:

        if #[cfg(all(target_arch = "wasm32", target_os = "wasi"))] {
            futures::executor::block_on(fut)
        }
        else if #[cfg(target_arch = "wasm32")] {
        // ... rest as before ...

This appeared to work and I am now seeing a working page. I don't know if it's a reasonable approach or if it could cause problems in more sophisticated apps, and I appreciate you may be reluctant to take a fix that is presumably not hit by testing. But I'd be delighted to send a PR if you're open to it.

@itowlson
Copy link
Contributor Author

I re-read the Hydration Bugs part of the book and now wonder if the correct fix is to enclose the create_resource in a create_effect. But I am not sure what the scope of that create_effect should be, because the counter resource is used later on the page.

@gbj
Copy link
Collaborator

gbj commented Nov 22, 2023

Excellent, thanks for raising this point! Initially wasm-bindgen-futures was only used for the browser, then a PR was made to use it for all wasm targets so that wasm32-unknown-unknown running in a JS environment on the server(less?) (Deno Deploy, Cloudflare Workers, etc.) would work. But obviously this then doesn't work for wasm32-wasi, which I definitely want to move toward full support for.

But I have a question: Is this really the recommended way to do async in Spin?

        if #[cfg(all(target_arch = "wasm32", target_os = "wasi"))] {
            futures::executor::block_on(fut)
        }
        else if #[cfg(target_arch = "wasm32")] {
        // ... rest as before ...

I don't know enough about the architecture: Does Spin handle separate requests as separate threads? i.e., is it safe to block on async like this? Or is there some other async support?

@gbj gbj added the bug Something isn't working label Nov 22, 2023
@itowlson
Copy link
Contributor Author

@gbj Spin handles separate requests as separate WebAssembly instances: that is, each request instantiates a request handler but that handler runs entirely separately from any other handlers. The handler is, I believe, single-threaded.

That said, Spin handlers do have a limited level of async/await. For example, if a handler makes an outbound HTTP call, it gets a future back, and can await that. But it isn't general-purpose async/await. We can't use tokio or other concurrent runtimes. It is, as far as I know, strictly bound to implementations of WASI pseudo-futures in the Spin host.

That said, I don't know if it's possible to have a long-running WASI host - one where a single instance handles all requests within the same module. I don't think WASI threading would enable that yet, but it could be a future risk with my fix.

I'll follow up with the Spin WASI experts to see what they think.

@gbj
Copy link
Collaborator

gbj commented Nov 22, 2023

In that case your solution sounds correct and I'd accept it as a PR in a heartbeat.

@itowlson
Copy link
Contributor Author

Thanks for the encouragement @gbj. I ran this past my colleague @dicej who is more knowledgeable about WASI and Spin internals than me, and he flagged a concern that the implementation of futures::executor::block_on would block forever if the future was pending when first polled. There's no great solution here, but Joel's advice was that we should poll the future in the same way as futures::executor::block_on, but if the future was not ready, we should panic rather than looping and polling again.

I've tested this and it Works On My Machine for the counter_isomorphic sample. I'll send a PR and can cover off some of the considerations there.

Longer term there is some stuff in the Spin SDK that we might be able to extract and generalise into a more robust poll_list-based executor for WASI. I'm not sure what the effort and risks there are though...!

@diversable
Copy link
Contributor

... We can't use tokio or other concurrent runtimes. ....

quick note about tokio, since I've spent some time trying to get Axum & Tokio to work properly with Leptos in a wasm / wasi context: Tokio does actually have a 'single-threaded' mode if you add the right 'flavor' to your tokio main fn.

ie.

#[tokio::main(flavor = "current_thread")]
async fn main() {
...
}

& in your cargo.toml, tokio supports the following features:

tokio = { version = "1.25.0", features = [
    "rt",
    "sync",
    "macros",
    "io-util",
], optional = true }

In a wasm32-wasi context, you can use the "time" feature in tokio too.

See here for more info: https://docs.rs/tokio/latest/tokio/#wasm-support

I still don't have it sorted out yet, but I'm working on putting together a wasm/wasi template for Leptos+Axum - great to see your work here as well!

@itowlson
Copy link
Contributor Author

@diversable Hmm, interesting. A Spin handler can't declare a tokio::main (it's a wasi-http incoming-handler) but presumably we could create a Tokio single-threaded runtime by hand at the start of the request. I'm not sure whether that would add overhead or how it would interact with host async features, but definitely another angle to investigate. Thanks!

@diversable
Copy link
Contributor

diversable commented Nov 24, 2023

FYI:
there seems to be an issue with running the full Leptos+Axum stack in a wasi context - it looks like hyper hasn't accepted the pull request that adds wasi support (yet)..

here it is for reference: hyperium/hyper#2900

tokio does have wasi support, though, and apparently there aren't any blockers from axum itself.

I'm not familiar enough with Spin to know if it would even support running hyper .. i'm still learning the finer details of wasi rt's

there is this example with hyper client wasi support, though: https://github.com/WasmEdge/wasmedge_hyper_demo

@gbj gbj closed this as completed in #2066 Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants