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

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

Open
cybersoulK opened this issue Sep 13, 2023 · 6 comments
Open

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

cybersoulK opened this issue Sep 13, 2023 · 6 comments
Labels

Comments

@cybersoulK
Copy link

cybersoulK commented Sep 13, 2023

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 cybersoulK added the bug label Sep 13, 2023
@cybersoulK cybersoulK changed the title Packet Loss When Using JsFuture with Web Worker in Chrome Futures are skipped When Using JsFuture with Web Worker in Chrome Sep 13, 2023
@cybersoulK
Copy link
Author

cybersoulK commented Sep 13, 2023

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
Copy link
Collaborator

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

@cybersoulK
Copy link
Author

cybersoulK commented Sep 13, 2023

@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
Copy link
Author

cybersoulK commented Sep 13, 2023

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
Copy link
Collaborator

Be careful about that, I'm not exactly aware of all the details, but: #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
Copy link
Author

cybersoulK commented Sep 13, 2023

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

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

No branches or pull requests

2 participants