rustwasm / wasm-bindgen

Facilitating high-level interactions between Wasm modules and JavaScript
https://rustwasm.github.io/docs/wasm-bindgen/
Apache License 2.0
7.69k stars 1.06k forks source link

Futures are skipped When Using JsFuture with Web Worker in Chrome #3609

Open cybersoulK opened 1 year ago

cybersoulK commented 1 year ago

We are experiencing an issue where JsFuture::from(reader.read()).await in a Web Worker drops over 80% of packets in Chrome. This issue does not occur in Firefox or when using native JavaScript in Chrome.

Code Snippet: The following Rust code runs in a Web Worker and awaits the web-transport datagrams reader, and experiences packet loss in Chrome:

while let Ok(obj) = JsFuture::from(reader.read()).await {
    // ... handle packets
}

For comparison, the JavaScript version that works correctly:

while (true) {
    const { value, done } = await reader.read();
    if (done) {
        break;
    }
    rustFunction(value);
}

i called the javacript code above using wasm-bindgen, with the reader as argument. The js function is located in the worker. So this is the best isolation i could do for JsFuture::from(reader.read()).await.

The observed behaviour is that awaiting when there is a group of them ready to be received, the in-between as skipped.

cybersoulK commented 1 year ago

more info about my setup:

cargo 1.73.0-nightly (45782b6b8 2023-07-05) wasm-bindgen-cli v0.2.87

how i build:

cargo build --release --target wasm32-unknown-unknown          

wasm-bindgen PATH --target web --out-dir PATH_OUT

my config.toml:

[unstable]
build-std = ["std", "panic_abort"]

[target.wasm32-unknown-unknown]
rustflags = ["-Ctarget-feature=+atomics,+bulk-memory,+mutable-globals", "--cfg=web_sys_unstable_apis"]

how i call the module:

<script type="module">
        import init from './client.js';

        document.addEventListener("play", async function(){
            await init();
        }, false);
    </script>

worker code:


import init, {wasm_thread_entry_point} from "WASM_BINDGEN_SHIM_URL";

self.onmessage = event => {
    let [ module, memory, context ] = event.data;

    init(module, memory).catch(err => {
        console.log(err);

        // Propagate to main `onerror`:
        setTimeout(() => { throw err; });
        throw err;

    }).then(() => {
        wasm_thread_entry_point(context);
    });
};

note: I tried recursively calling a Closure on the reader.read(), and it works like normal. So JsFuture might need to be investigated

daxpedda commented 1 year ago

A minimal reproducible example would go a long way helping you debug this.

cybersoulK commented 1 year ago

@daxpedda

I was able to test JsFuture in a non-worker environment, and the issue happens as well! So it's not worker related.

I kept doing A/B testing about 30 times on my codebase, until added and removed the following: -Ctarget-feature=+atomics,+bulk-memory" specifically the +atomics flag.

WIth atomics flag, JsFutures has the "skipping behaviour" that i observed in chrome (but not firefox)

hopefully we can figure out how atomics is affecting it. for now i will use a recursive closure instead of JsFuture, since i need +atomics for shared memory with the worker

cybersoulK commented 1 year ago

i digged a bit into the codes, and i saw the specific atomic flag and branch I forked wasm_bindgen_futures and disabled them, and now it works 100%.

note: i use a singlethreaded async runtime inside webworker, and the multithreading branch was not working for me due to the bug

daxpedda commented 1 year ago

Be careful about that, I'm not exactly aware of all the details, but: https://github.com/rustwasm/wasm-bindgen/issues/1379. If I'm interpreting this correctly, the version of wasm-bindgen-futures you are using now isn't thread-safe. But if you are sure you aren't involving any other threads in your futures, I guess it should be fine.

cybersoulK commented 1 year ago

uhm actually after further testing, the single threaded branch didn't await inside the worker properly: JsFuture::from(connection.ready()).await (so js complained get_Object functions were missing)

i will just use the closure solution for now then