Support async thread entry points

[chemicstry]

Async entry points would allow using async channels inside web worker threads.

This feature would probably need a major rewrite as discussed in https://github.com/chemicstry/wasm_thread/issues/8

[Davidster]

Hello! I am currently encountering the same issue as #6 and am wondering if there might be a way to fix the issue without the need for supporting postMessage? Currently I'm planning on doing something like this:

fn spawn_thread<F, T>(f: F)
where
    F: FnOnce() -> T,
    F: Send + 'static,
    T: Send + 'static,
{
    if cfg!(target_arch = "wasm32") {
        thread::spawn(|| {
            f();
            wasm_bindgen::throw_str("Cursed hack to keep workers alive. See https://github.com/rustwasm/wasm-bindgen/issues/2945");
        });
    } else {
        thread::spawn(f);
    }
}

At a minimum it is possible to make it work with the error. I don't see what exactly is stopping wasm_thead from being able to block in worker threads, although I admit I haven't read through the crate's code.

[chemicstry]

@Davidster I would rather avoid hacks with throws.

So I looked more into this issue and found something stupid simple, the web worker script has a close call after executing closure: https://github.com/chemicstry/wasm_thread/blob/main/src/web_worker.js#L25C11-L25C11, could you test if removing it makes your usecase work? You might need to also remove it from web_worker_module.js if you are using ES modules.

I tested it locally and it seems to work, maybe everyone missed this close() call when discussing the previous issue

[Davidster]

Yes, after posting my comment I read more code and found the close() call. Removing it might indeed fix the issue and I will test it in a few days when I get the chance. I was worried that the close() call was needed to prevent a leak or something, but the browser should be able to clean up workers automatically when they have no more pending tasks, right? I wonder if there's a way I can test it, maybe by spawning a lot of wasm_thread's and checking in the dev tools window to see how many active workers stay alive (not sure if such a tool exists).

Thank you for your reply!

[chemicstry]

In theory, web workers should terminate automatically when there are no items left in worker's event queue (pending async promises) and main thread has dropped worker handle (can no longer postMessage()). However, I could not replicate this behavior yet. Maybe there is still some dangling reference left that prevents garbage collector cleanup.

[adamgerhant]

I just ran into a very similar issue when running async code with futures_lite::future::block_on in a web worker. The web worker would close before a promise could resolve. I think it might be worth removing the .close(), or at least making it an option and documenting the fact that it not work with any async code in the web worker.

[chemicstry]

@adamgerhant could you test it on a new refactored v0.3.0 release? We now have a test for this behavior: https://github.com/chemicstry/wasm_thread/blob/49ae81d67601bdc2e64d8ef96e14c7e62107aa4c/tests/wasm.rs#L102

[adamgerhant]

I tried with v0.3.0 and found that the close() function has to be removed, and spawn_local has to be used instead of block_on. If the close() is not removed then the thread will be closed before the future can be executed since spawn_local returns immediately.

Wasm_thread not working: "after" never gets printed

pub fn create_promise() -> Promise {
    let promise = Promise::new(&mut |resolve, reject| {
        log::info!("promise");
        let success = true;
        if success {
            resolve.call0(&JsValue::null()).unwrap();
        } else {
            let error_msg = JsValue::from_str("An error occurred");
            reject.call1(&JsValue::null(), &error_msg).unwrap();
        }
    });

    promise
}

 wasm_thread::spawn(|| {
     futures::executor::block_on(async move {
        log::info!("before");
        let js_future = wasm_bindgen_futures::JsFuture::from(create_promise());
        js_future.await;
        log::info!("after");
     })
 });

Wasm_thread fixed: "after" gets printed

thread::spawn(|| {
    wasm_bindgen_futures::spawn_local(async move {
        log::info!("before");
        let js_future = wasm_bindgen_futures::JsFuture::from(create_promise());
        js_future.await;
        log::info!("after");
    });
});

This is because block_on does not work with jsFutures, while spawn_local does. This also applies for libraries where a jsFuture is internally awaited, such as with wgpu::Instance::request_adapter, which returns a standard Future, but internally awaits a jsFuture so spawn_local must be used.

Ultimately I think the easiest solution is to make the thread close() based on a flag, and document that it will not work with jsFutures if it is enabled. With the current method I dont see any way to tell when the async function finishes, since it is created and run by the user, not wasm_thread. Ideally I think it would be best for wasm_thread to natively support async closures. Maybe this could be done by treating all functions as async futures and running them with spawn_local. This would also allow channels to communicate when the function finishes, and then close the thread at the correct time.

[chemicstry]

Yeah the problem here is that block_on parks the thread (possibly on atomic wait) and javascript event loop cannot run while the thread is parked. The close call is needed in order to not leak memory when threads are actively being created/detroyed. I will think how this could be fixed without deviating too much from the std thread API.

[adamgerhant]

I added a spawn_async method that spawns the future with spawn_local, and uses a static arc flag that is updated when the function is complete and then used to close the thread. It works well so far, and I'll try create a pull request today. The main problem so far is that I wasnt able to get the join handle properly implemented, but I think that should be possible.

Edit: I realized that a single static boolean flag for if the thread should close will not work when there are multiple threads running. I am going to try a new approach with arc's, but it will change the internals a bit.

Edit 2: I think I got it fully functional with a static hashmap. I will clean up the code, write a test, and make a pull request