Skip to content

Conversation

juj
Copy link
Collaborator

@juj juj commented Oct 4, 2025

Delete the Atomics.waitAsync() polyfill, which is not good enough to polyfill over the usage pattern in Emscripten mailbox implementation. See #25449 (comment) . The mailbox implementation has its own postMessage() fallback, so does not need the polyfill. Fixes test browser.test_gl_textures_pthreads_main_module in Firefox."

This will be a breaking change to any users who successfully depended on the polyfill (i.e. only needed the partial support it brought), though it is uncertain if there are any.

Fixes test browser.test_gl_textures_pthreads_main_module on Firefox.

…polyfill over the usage pattern in Emscripten mailbox implementation. See emscripten-core#25449 (comment) . The mailbox implementation has its own postMessage() fallback, so does not need the polyfill. Fixes test browser.test_gl_textures_pthreads_main_module in Firefox."
@juj juj requested review from tlively and sbc100 October 4, 2025 13:17
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.

Nice! Always good to see code being deleted.

I'll let @tlively give the final approval.

@juj juj force-pushed the delete_waitasync_polyfill branch from 5eb6c1a to 29a1295 Compare October 4, 2025 19:28
Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

Oh yeah, wow, that's totally broken.

@juj juj merged commit d9e0690 into emscripten-core:main Oct 6, 2025
33 checks passed
juj added a commit to juj/emscripten that referenced this pull request Oct 6, 2025
juj added a commit that referenced this pull request 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.
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