Skip to content

Conversation

juj
Copy link
Collaborator

@juj juj commented Oct 6, 2025

This reverts commit d9e0690.

Oops, it looks like we do have several tests that depended on the polyfill:

http://clbri.com:8010/api/v2/logs/89393/raw_inline

In particular:

browser.test_wasm_worker_cancel_all_wait_asyncs
browser.test_wasm_worker_cancel_all_wait_asyncs_at_address
browser.test_wasm_worker_cancel_all_wait_asyncs_at_address_minimal_runtime
browser.test_wasm_worker_cancel_all_wait_asyncs_minimal_runtime
browser.test_wasm_worker_cancel_wait_async
browser.test_wasm_worker_cancel_wait_async_minimal_runtime
browser.test_wasm_worker_wait_async
browser.test_wasm_worker_wait_async_minimal_runtime

CircleCI just didn't have coverage of these functions on Firefox.

Revert this PR until we can reinstate it in a form that applies the polyfill to only these functions.

@juj juj requested review from sbc100 and tlively October 6, 2025 18:59
Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Feel free to land pure reverts like this as TBR if its breaking tests.

@juj juj merged commit d15011e into emscripten-core:main Oct 6, 2025
24 of 33 checks passed
@juj
Copy link
Collaborator Author

juj commented Oct 6, 2025

Thanks. I am fairly certain that the polyfill should be usable for scenarios that implement a lock.. We'll either have to selectively apply the polyfill to those locations, or conclude that they won't be safe, and add test disables/MIN_x_VERSION checks selectively for browsers that don't support the real thing.

@tlively
Copy link
Member

tlively commented Oct 6, 2025

It might be nice to add a way to detect the polyfill so the mailbox mechanism can fall back to postMessage when waitAsync is polyfilled.

@juj
Copy link
Collaborator Author

juj commented Oct 6, 2025

Yeah, that's what I was thinking.. to demote the polyfill to a regular library function, which the select Atomics.waitAsync()-utilizing library functions will fall back on, but the mailbox would then get the postMessage().

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

Successfully merging this pull request may close these issues.

3 participants