Skip to content

Only use futex_wait_busy on the main browser thread #18402

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

Merged
merged 1 commit into from
Dec 26, 2022
Merged

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Dec 19, 2022

When I ported this code from JS back in #15742 I mistakenly changed the semantics here. This change restores the previous behaviour of using atomic.wait except in on the main browser thread.

The futex_wait_busy can clearly only be used a single main thread and is not appropriate for using in cases where there are other threads in the system that also don't support atomic.wait (such as IIUC audio worklets). To support that case we would need a different busy wait function that didn't depend on some global singleton like _emscripten_main_thread_futex.

@sbc100 sbc100 requested review from juj and kripken December 19, 2022 14:49
@sbc100 sbc100 force-pushed the futex_wait_busy branch 3 times, most recently from 90e7cab to c28bd43 Compare December 19, 2022 20:38
@sbc100 sbc100 enabled auto-merge (squash) December 20, 2022 00:26
When I ported this code from JS back in #15742 I mistakenly changed
the semantics here.  This change restores the previous behaviour of
using atomic.wait except in on the main browser thread.

The `futex_wait_busy` can clearly only be used a single main thread and
is not appropriate for using in cases where there are other threads in
the system that also don't support `atomic.wait` (such as IIUC audio
worklets).  To support that case we would need a different busy wait
function that didn't depend on some global singleton like
_emscripten_main_thread_futex.
@sbc100 sbc100 merged commit bfabc4e into main Dec 26, 2022
@sbc100 sbc100 deleted the futex_wait_busy branch December 26, 2022 10:58
juj added a commit to juj/emscripten that referenced this pull request Feb 2, 2023
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.

2 participants