Interactive audio worklet futex test fails

[cwoffenden]

With the latest Emscripten:

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.73-git (299be0bbfbbadfa926cb69380f0fffa3703b26c7)

This interactive audio worklet futex test fails:

emscripten/test/test_interactive.py

Line 298 in 299be0b

def test_audio_worklet_emscripten_futex_wake(self):

Running:

test/runner interactive.test_audio_worklet_emscripten_futex_wake

It fails to compile due to _emscripten_thread_supports_atomics_wait() being internal to the pthread implementation:

test_audio_worklet_emscripten_futex_wake (test_interactive.interactive) ... Opening in existing browser session.
/Volumes/Work/Tools/Emscripten/emscripten/main/test/webaudio/audioworklet_emscripten_futex_wake.cpp:17:28: error: use of undeclared identifier '_emscripten_thread_supports_atomics_wait'
   17 |   int supportsAtomicWait = _emscripten_thread_supports_atomics_wait();
      |                            ^
1 error generated.

Further, the comment here:

emscripten/system/lib/pthread/emscripten_thread_state.S

Line 56 in 299be0b

# Semantically the same as testing "!ENVIRONMENT_IS_WEB" in JS

Is incorrect since !ENVIRONMENT_IS_WEB is true for Audio Worklets but Atomics.wait is not supported.

[cwoffenden]

There's more to this. If I comment out _emscripten_thread_supports_atomics_wait() then the emscripten_futex_wait() call fails with:

test.js:899 Uncaught RuntimeError: Aborted(Assertion failed: emscripten_is_main_browser_thread(), at: ../../../system/lib/pthread/emscripten_futex_wait.c,26,futex_wait_main_browser_thread)

Because when the audio worklet code hits here:

emscripten/system/lib/pthread/emscripten_futex_wait.c

Line 132 in 299be0b

if (!_emscripten_thread_supports_atomics_wait()) {

As the comment says, an audio worklet will take the same path as the main thread:

  // For the main browser thread and audio worklets we can't use
  // __builtin_wasm_memory_atomic_wait32 so we have busy wait instead.

Which then fails when it arrives here:

emscripten/system/lib/pthread/emscripten_futex_wait.c

Line 26 in 299be0b

assert(emscripten_is_main_browser_thread());

(And in testing, it's correct that __builtin_wasm_memory_atomic_wait32() fails: Atomics.wait cannot be called in this context.)

[cwoffenden]

And adding more: since Atomics.wait isn't allowed in Audio Worklets, anything that uses a futex, including a mutex fails when assertions are enabled:

Uncaught RuntimeError: Aborted(Assertion failed: emscripten_is_main_browser_thread(), at: ../../../system/lib/pthread/emscripten_futex_wait.c,26,futex_wait_main_browser_thread)
    at abort (index.js:796:11)
    at ___assert_fail (index.js:1859:7)
    at index.wasm.futex_wait_main_browser_thread (index.wasm:0x2b19)
    at index.wasm.emscripten_futex_wait (index.wasm:0x29bc)
    at index.wasm.__timedwait_cp (index.wasm:0x2cb0)
    at index.wasm.__timedwait (index.wasm:0x2d16)
    at index.wasm.__pthread_mutex_timedlock (index.wasm:0x2e37)
    at index.wasm.mtx_timedlock (index.wasm:0xf4d)
    at index.wasm.mtx_lock (index.wasm:0xf33)
    at index.wasm.process (index.wasm:0xa67)

But are fine without assertions. So the questions comes to this:

  // Atomics.wait is not available in the main browser thread, so simulate it
  // via busy spinning. Only the main browser thread is allowed to call into
  // this function. It is not thread-safe to be called from any other thread.

Is still true and Audio Worklets will need this fixing or another implementation?

Bit of a rabbit hole...

[sbc100]

IIRC @juj also looked into this recently? And maybe had a fix in flight? I couldn't find one, but I do find this which looks like the same issue: #20617

[cwoffenden]

It turns out @tklajnscek (writer of the other implementation mentioned in the issue and with whom I work) exposed a separate futex to the worklet. I'll create an additional PR later.

[cwoffenden]

Having discussed this over here, the consensus being the existing working API spinlocks should be used, so emscripten_lock_busyspin_wait_acquire() to get some equivalence with emscripten_futex_wait(), but these also fail due to the emscripten_performance_now() call:

emscripten/system/lib/wasm_worker/library_wasm_worker.c

Line 119 in 119a427

double t = emscripten_performance_now();

But this could be changed to use emscripten_get_now(), which defers to performance.now() where available:

emscripten/src/library.js

Line 1378 in 119a427

emscripten_get_now: `;

And since these ms calls don't need to resolution the ns calls do, is probably a good compromise and will only affect the Audio Worklet for little overhead (either _now() call goes out to JS). What do you think, @sbc100?

[tklajnscek]

For what it's worth, I don't see any other way except if we only allow infinite timeout...

[sbc100]

But this could be changed to use emscripten_get_now(), which defers to performance.now() where available:

Is this because performance.now() is not available in audio worklets? I guess emscripten_get_now() uses Date.now() instead in that case? That seems rather strange that Data.now() is available but not performance.now()..

[cwoffenden]

Exactly that, performance.now() isn't available, and emscripten_get_now() uses Date.now() in that case. And yeah, an odd choice that it's missing for what should be high-precision processing. The AW does have 'currentTime` but I'll assume it pauses when the audio does.

The low precision of the Date.now() is preferable to infinite timeouts, for code that really, really needs to block.

[tklajnscek]

The relevant WebAudio issue here: WebAudio/web-audio-api#2413

With comments from @juj :)

[cwoffenden]

Fix #23393 ready.