audioWorklets and libpd !emscripten_is_main_browser_thread issue

[Jonathhhan]

audioWorklets with Open Frameworks work really well, as long as I do not use ofxOfelia as an addon: https://github.com/cuinjune/Ofelia If I use it with USE_PTHREADS, I get error messages like this:

Uncaught RuntimeError: Aborted(Assertion failed: !emscripten_is_main_browser_thread(), at: /mnt/c/Users/Jonat/emsdk/upstream/emscripten/system/lib/pthread/emscripten_futex_wake.c,35,emscripten_futex_wake)
    at abort (index.js:783:40)
    at ___assert_fail (index.js:4021:2)
    at index.wasm.emscripten_futex_wake (index.wasm:0xd8a614)
    at index.wasm.__wake.10 (index.wasm:0xd93fd7)
    at index.wasm.__pthread_mutex_unlock (index.wasm:0xd93fbe)
    at index.wasm.sys_unlock (index.wasm:0x3f1448)
    at index.wasm.ofxOfelia::mouseDragged(ofMouseEventArgs&) (index.wasm:0x1cc4b)
    at index.wasm.ofApp::mouseDragged(ofMouseEventArgs&) (index.wasm:0x1445c)
    at index.wasm.std::__2::__function::__func<std::__2::shared_ptr<of::priv::Function<ofMouseEventArgs, std::__2::recursive_mutex>> ofEvent<ofMouseEventArgs, std::__2::recursive_mutex>::make_function<ofBaseApp>(ofBaseApp*, void (ofBaseApp::*)(ofMouseEventArgs&), int)::'lambda'(void const*, ofMouseEventArgs&), std::__2::allocator<std::__2::shared_ptr<of::priv::Function<ofMouseEventArgs, std::__2::recursive_mutex>> ofEvent<ofMouseEventArgs, std::__2::recursive_mutex>::make_function<ofBaseApp>(ofBaseApp*, void (ofBaseApp::*)(ofMouseEventArgs&), int)::'lambda'(void const*, ofMouseEventArgs&)>, bool (void const*, ofMouseEventArgs&)>::operator()(void const*&&, ofMouseEventArgs&) (index.wasm:0x45d3f4)
    at index.wasm.ofEvent<ofMouseEventArgs, std::__2::recursive_mutex>::notify(ofMouseEventArgs&) (index.wasm:0x463418)

If I comment out line 35 in emscripten_futex_wake.c it seems to work without issues: https://github.com/emscripten-core/emscripten/blob/main/system/lib/pthread/emscripten_futex_wake.c But I wonder, if there is a cleaner way to resolve the assertion. Probably it has to do with libpd's/Pure Data's own locking system(because there is always a sys_unlock involved)? And maybe this issue is related: https://github.com/emscripten-core/emscripten/issues/14339?

[sbc100]

On which thread is that backtrace running? Since its a mount event then it seems like it would be main browser thread? Perhaps emscripten_is_main_browser_thread is not returning the correct value?

[Jonathhhan]

@sbc100 not really sure how to see or set where the backtrace is running, here is the config file: https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworksCompiled/project/emscripten/config.emscripten.default.mk ASSERTIONS is set here: PLATFORM_OPTIMIZATION_LDFLAGS_DEBUG = -g3 -s TOTAL_MEMORY=134217728 --memory-init-file 1 -s DEMANGLE_SUPPORT=1 -s ASSERTIONS=2 And here: PLATFORM_OPTIMIZATION_CFLAGS_DEBUG = -g3 -s DEMANGLE_SUPPORT=1 -s ASSERTIONS=2 The error is gone, if I remove ASSERTIONS from PLATFORM_OPTIMIZATION_LDFLAGS_DEBUG.

[sbc100]

Can you capture it in the debugger with the full backtrace. From the name of the function ofEvent<ofMouseEventArgs it sounds like a DOM mouse event...which seems to suggest it is running on the main thread.

[Jonathhhan]

@sbc100 I hope this is the full backtrace:

w:0,t:0x003aaf80: Aborted(Assertion failed: !emscripten_is_main_browser_thread(), at: /mnt/c/Users/Jonat/emsdk/upstream/emscripten/system/lib/pthread/emscripten_futex_wake.c,35,emscripten_futex_wake) printErr @ (index):1 err @ index.js:3777 err @ index.js:12091 abort @ index.js:784 _assert_fail @ index.js:4125 $emscripten_futex_wake @ index.wasm:0xd8a4c9 $wake.10 @ index.wasm:0xd93e8c $pthread_mutex_unlock @ index.wasm:0xd93e73 $sys_unlock @ index.wasm:0x3f186a $ofxOfelia::mouseDragged(ofMouseEventArgs&) @ index.wasm:0x1d1dc $ofApp::mouseDragged(ofMouseEventArgs&) @ index.wasm:0x14a0b $std::2::function::func<std::2::shared_ptr<of::priv::Function<ofMouseEventArgs, std::2::recursive_mutex>> ofEvent<ofMouseEventArgs, std::2::recursive_mutex>::make_function(ofBaseApp, void (ofBaseApp::)(ofMouseEventArgs&), int)::'lambda'(void const*, ofMouseEventArgs&), std::2::allocator<std::2::shared_ptr<of::priv::Function<ofMouseEventArgs, std::__2::recursive_mutex>> ofEvent<ofMouseEventArgs, std::2::recursive_mutex>::make_function(ofBaseApp, void (ofBaseApp::)(ofMouseEventArgs&), int)::'lambda'(void const, ofMouseEventArgs&)>, bool (void const, ofMouseEventArgs&)>::operator()(void const&&, ofMouseEventArgs&) @ index.wasm:0x45d5ae $ofEvent<ofMouseEventArgs, std::__2::recursive_mutex>::notify(ofMouseEventArgs&) @ index.wasm:0x4635d2 $ofCoreEvents::notifyMouseEvent(ofMouseEventArgs&) @ index.wasm:0x46343c $ofCoreEvents::notifyMouseDragged(int, int, int) @ index.wasm:0x463807 $ofxAppEmscriptenWindow::mousemoved_cb(int, EmscriptenMouseEvent const, void*) @ index.wasm:0x1620d mouseEventHandlerFunc @ index.js:8389 jsEventHandler

[sbc100]

So that assertion is firing on the main thread, and its correctly detecting some kind of misconfiguration.

Perhaps you are trying use sys_lock/sys_unlock from your audio worklet and those APIs depend on pthread APIs. Pthread APIs don't work on audio worklets or at least they are not supposed to. Perhaps they are partially working and emscripten_is_main_browser_thread is incorrectly returning true for them.

[sbc100]

@juj, what currently happens if you try to use pthread API from wasm workers? Do we have assertions for that of will things fail in odd ways like this?

[Jonathhhan]

Perhaps you are trying use sys_lock/sys_unlock from your audio worklet

Yes, that's the case. Thanks for helping.

[juj]

@juj, what currently happens if you try to use pthread API from wasm workers? Do we have assertions for that of will things fail in odd ways like this?

Sorry, I missed if this bug relates to Wasm Workers. I believe emscripten_futex_wake() and emscripten_futex_wait() should not be linked to programs that use Wasm Workers, so won't be available there.

If building with the hybrid mode Wasm Workers + pthreads, then I believe they will work, although that is a spot in testing.

The main thing that does not get initialized for Wasm Workers is the pthread_self() pointer structure and the proxying queues, so as a guideline anything that uses those will not work. (and shouldn't be getting linked in pure wasm workers builds either)

[juj]

If Audio Worklets are involved in this scenario, then I am guessing that the issue might have something to do with these code lines:

https://github.com/emscripten-core/emscripten/blob/1375c03ad476dba8f20c9c3919ef57f1af43e36b/system/lib/pthread/emscripten_futex_wait.c#L127-L129

In the above code, Audio Worklets were guided to do a spinwait like the main thread, since they both don't have real futex wait capabilities as per browser specifications.

But the assert check in

https://github.com/emscripten-core/emscripten/blob/1375c03ad476dba8f20c9c3919ef57f1af43e36b/system/lib/pthread/emscripten_futex_wake.c#L34-L35

is intended to do a data corruption/programming error check: the special memory location _emscripten_main_thread_futex is to store the location where main thread is currently waiting on. If that location is nonzero, then the main thread is supposed to be spinwaiting there, so if main thread is simultaneously waking a waiter, while it should also be spinwaiting, something should have gone wrong: it can't be in two places at the same time!

The bug here seems to be that Audio Worklets should not be directed to also take the same behavior as the main thread, since they now literally get treated like the main thread as they call futex_wait_main_browser_thread() as well. As result, firing the above definitely would happen.

This is something that we should distill a minimal failing code example for, since these synchronization scenarios are quite complex. I think a good fix will be to duplicate the extern void* _emscripten_main_thread_futex; into a separate extern void* _emscripten_audio_worklet_thread_futex; and check both the locations when waking.

While that sounds a bit hacky initially, the Web Audio specification guarantees that there will be only one audio thread (I asked to get more in https://github.com/WebAudio/web-audio-api/issues/2500 , where the consensus was that there will only ever be one). So we should be pretty good to think that audio worklet and main thread would get this special treatment on their own.

[sbc100]

Just to be clear what is the expected behavior in "wasm workers + pthread" build if a pthread function is called from a wasm worker? Presumably that is undefined behaviour.. but maybe we don't currently detect this an report the error.

[juj]

If I comment out line 35 in emscripten_futex_wake.c it seems to work without issues

My impression of the situation is that if both audio worklet and main thread happen to do a mutex wait at the same time, then eventually there will be a breakage here, so we won't be good by only removing the assert.

[juj]

Just to be clear what is the expected behavior in "wasm workers + pthread" build if a pthread function is called from a wasm worker? Presumably that is undefined behaviour.. but maybe we don't currently detect this an report the error.

Correct, this would run into a crash likely, if calling any pthread function that attempted to invoke pthread_self() and maybe other functions that require pthread specific state. This is an area that we definitely should proof.

[sbc100]

Alternatively I suppose we could say that emscripten_futex_wait is simply not allowed in audio worklets?

I suppose the reason that __builtin_wasm_memory_atomic_wait32 is not supported there is because the designers of the API didn't want any blocking. Perhaps the rational for adding a busy-wait version of waiting does not apply in this case like it did for the main browser thread?

[juj]

I suppose the reason that __builtin_wasm_memory_atomic_wait32 is not supported there is because the designers of the API didn't want any blocking.

This is true, this was the very rationale. It is the same rationale as the main browser thread not having a futex wait.

Perhaps the rational for adding a busy-wait version of waiting does not apply in this case like it did for the main browser thread?

In my opinion both main thread and audio thread should have gotten futex wait abilities directly in the browser APIs. The root issue that I was never able to convince people writing the specs is the composability problem: it is not feasible to develop shared data structures in Wasm code where only one side accessing the data structure would use "real" waiting, and the other one (the main thread/audio thread) would somehow never need to wait (be 'wait-free' in the exact technical meaning of the term). Such data structures really don't exist, or have unrealistic overhead from requiring transactional principles.

In the main thread case, the situation was attempted to be resolved with the async wait operation that never became feasible to use due to spec problems, and I don't think even today anyone is using that (or if they are, they have subtle garbage collection and memory free() bugs that prevents them from implementing dynamic site loading experiences).

Instead, users need to write data structures where every side can equally wait, just that the programmer will provide the guarantee that whenever main thread, or audio worklet waits, they with their programming experience that these waiters will make progress quickly. The same thing as a programmer who was writing a for(;;) loop, they do guarantee that it won't hang the browser.

People will be doing mutex locking and spinwaiting in the audio worklet thread, and I think that is ok, as long as they know their shared data structure access performance landscape.

[sbc100]

Sounds reasonable. Perhaps we can parameterize futex_wait_main_browser_thread with the location of the _emscripten_main_thread_futex value (and rename it):

  // For the main browser thread and audio worklets we can't use                 
  // __builtin_wasm_memory_atomic_wait32 so we have busy wait instead.           
  if (!_emscripten_thread_supports_atomics_wait()) {
    assert(is_main_browser_thread() || is_audio_worklet());
    void* global_location = is_main_browser_thread ?  _main_thread_futex : _worklet_futex                 
    ret = futex_wait_main_browser_thread(addr, val, max_wait_ms, global_location);                
    emscripten_conditional_set_current_thread_status(EM_THREAD_STATUS_WAITFUTEX, EM_THREAD_STATUS_RUNNING);
    return ret;                                                                  
  } 

[Jonathhhan]

The audioworklet implementation from @tklajnscek also had a dedicated worklet_futex (and I did not had those issues with that): https://github.com/emscripten-core/emscripten/blob/8a2f1b7c7d7e4249e70fc108eb848a3a3bdccb08/src/library_pthread.js // For audio worklets we use the same global address since they all run on // the audio thread. It's all very similar to the main thread case, except we // don't have to do any nested call special casing.

#if ENVIRONMENT_MAY_BE_AUDIOWORKLET
    var spinFutexes = [__emscripten_main_thread_futex, __emscripten_audio_worklet_futex];
    for (var i = 0; i < spinFutexes.length; i++) {
      var theFutex = spinFutexes[i];
#else
    var theFutex = __emscripten_main_thread_futex;
#endif
#if ASSERTIONS
    assert(theFutex > 0);
#endif
    var waitAddress = Atomics.load(HEAP32, theFutex >> 2);

    if (waitAddress == addr) {
#if ASSERTIONS
      // We only use __emscripten_main_thread_futex on the main browser thread, where we
      // cannot block while we wait. Therefore we should only see it set from
      // other threads, and not on the main thread itself. In other words, the
      // main thread must never try to wake itself up!
      assert(theFutex != __emscripten_main_thread_futex || !ENVIRONMENT_IS_WEB);
#endif
      var loadedAddr = Atomics.compareExchange(HEAP32, theFutex >> 2, waitAddress, 0);
      if (loadedAddr == waitAddress) {
        --count;
        spinFutexesWoken += 1;
        if (count <= 0) return 1;
      }
    }

[sbc100]

Oh nice! Great minds think alike.

[Jonathhhan]

People will be doing mutex locking and spinwaiting in the audio worklet thread, and I think that is ok, as long as they know their shared data structure access performance landscape.

In my case it is this audiocallback, which writes the audio data into the inbuffer/outbuffer (which was called from the ScriptProcessorNode before):

stream->settings.inCallback(stream->inbuffer);
stream->settings.outCallback(stream->outbuffer);

That's my ProcessAudio method:

EM_BOOL ProcessAudio(int numInputs, const AudioSampleFrame *inputs, int numOutputs, AudioSampleFrame *outputs, int numParams, const AudioParamFrame *params, void *userData) {
    ofxEmscriptenSoundStream * stream = (ofxEmscriptenSoundStream *)userData;
    ++stream->audioProcessedCount;
    if (stream->settings.numInputChannels > 0 && stream->settings.inCallback) {
        for (int o = 0; o < numInputs; ++o) {
            for (int i = 0; i < 128; ++i) {
                for (int ch = 0; ch < inputs[o].numberOfChannels; ++ch) {
                    stream->inbuffer[i * inputs[o].numberOfChannels + ch] = inputs[o].data[ch * 128 + i];
                }
            }
        }
        stream->settings.inCallback(stream->inbuffer);
    }
    if (stream->settings.numOutputChannels > 0 && stream->settings.outCallback) {
        stream->settings.outCallback(stream->outbuffer);
        for (int o = 0; o < numOutputs; ++o) {
            for (int i = 0; i < 128; ++i) {
                for (int ch = 0; ch < outputs[o].numberOfChannels; ++ch) {
                    outputs[o].data[ch * 128 + i] = stream->outbuffer[i * outputs[o].numberOfChannels + ch];
                }
            }
        }
    }
    return EM_TRUE;
}

[Jonathhhan]

I just wanted to emphasize that it would be really nice, to have an emscripten_audio_worklet_futex...

[Jonathhhan]

It seems they improved Pure Data (the audio processing software I use with Emscripten) and the issue does not occure anymore.