Same AudioContext for two Modules causes memory corruption

[lindell]

Version of emscripten/emsdk: 4.0.13 (but verified in code this should still exist on HEAD)

Description:

Using the web audio API with one AudioContext but with two or more modules does not work, as the second one will try silently the memory of the first one, causing memory failures.

Example:

  const audioContext = new AudioContext();

  const mainModule = await createModule();
  const audioContextId = mainModule.emscriptenRegisterAudioObject(audioContext);
  mainModule.functionThatStartsAWorklet(audioContextId);

  const mainModule2 = await createModule();
  const audioContextId2 =
    mainModule2.emscriptenRegisterAudioObject(audioContext);
  mainModule2.functionThatStartsAnotherWorklet(audioContextId2);

Where functionThatStartsAWorklet and functionThatStartsAnotherWorklet starts AudioWorklets.

---

addModule will succeed, at least if the same URL is used.

But since it is set up with the wasmModule and memory only once for the AudioContext. When starting another worklet, from another module, both will use the same memory even though they should be using separate ones. This causes memory corruptions, and usually crashes for one or both of the worklets.

[lindell]

@cwoffenden

[cwoffenden]

Are you passing in the same stack or are you allocating one per audio worklet like this:

https://github.com/emscripten-core/emscripten/blob/80269bf4e700951847b4ef350605b7052ec05aa3/test/webaudio/audioworklet_test_shared.inc#L107

[lindell]

I'm using uint8_t audioThreadStack[4096]; But that should not really matter? Since the two emscripten Module instances have different memories.

[cwoffenden]

Are modules not sharing the same shared array? From what you describe as running using memory of the other, I'd first look at allocating the stack instead of passing in the array.

[lindell]

They are not using the same memory, that is the core of this issue. With the same instance/memory you should be able to register/create as many Worklets as you want without any problem. createModule() is the factory function which create another instance.

But just to be sure. I change the stack creation to this

  void* const workletStack = memalign(16, AUDIO_STACK_SIZE);

  emscripten_start_wasm_audio_worklet_thread_async(
      audio_context, workletStack, AUDIO_STACK_SIZE, &AudioThreadInitialized,
      user_data);

And I still get Uncaught RuntimeError: memory access out of bounds

[cwoffenden]

Now I understand, the errors are because the second instance can't see the memory of the first (I understood it to be writing over the same space).

[lindell]

I understood it to be writing over the same space

That is what I think is happening.

Let me added some additional context. For simplicity, I assume that global scope message ports is not enabled in the following example.

What is done:

1. An audio context is created
2. An Module instance is created by calling the factory method. This will create the SharedArrayBuffer to be used for this instance only.
3. A function is called on the Module from 2. which
   1. Calls emscripten_start_wasm_audio_worklet_thread_async to setup everything in the AudioContext.
   2. emscripten_create_wasm_audio_worklet_processor_async is called with a unique name for the AudioContext to register a new Worklet Processor.
   3. We can now initialize the Worker, but it is technically not necessary.
4. Repeat 2
5. Repeat 3

Expectation: We now have two AudioWorkletProcessors registered (and started?) in the AudioContext. Each with its own memory since they are created by different instances from the Main Thread.

What is actually happening

In 5.1, the em-bootstrap AudioWorkletProcessor will be the same as for 3.1. This includes setting the wasmMemory/wasmModule blocked scoped here:

• https://github.com/emscripten-core/emscripten/blob/80269bf4e700951847b4ef350605b7052ec05aa3/src/audio_worklet.js#L276C5-L276C20
• https://github.com/emscripten-core/emscripten/blob/80269bf4e700951847b4ef350605b7052ec05aa3/src/wasm_worker.js#L14

In 5.2, when registering the new processor, any call into Wasm will now be made into the module created in 2. instead of the one created in 4. which the code assume it will do. This is the case both for Emscripten code in the Worklet like this. But also any user code that will be run within the registered process function.

Since the second instance within the AudioContext will try to use the other instances memory, will cause out of bound reads / memory corruptions / etc.

[brendandahl]

Do you have a minimal example you could share? I tried the following, but I must be doing something different since I get

emscripten_create_wasm_audio_worklet() was already called for AudioContext 1! Only call this function once per AudioContext!)

#include <emscripten/em_math.h>
#include <emscripten/webaudio.h>

uint8_t audioThreadStack[4096];

bool OnCanvasClick(int eventType,
                   const EmscriptenMouseEvent* mouseEvent,
                   void* userData) {
  EMSCRIPTEN_WEBAUDIO_T audioContext = (EMSCRIPTEN_WEBAUDIO_T)userData;
  if (emscripten_audio_context_state(audioContext) !=
      AUDIO_CONTEXT_STATE_RUNNING) {
    emscripten_resume_audio_context_sync(audioContext);
  }
  return false;
}

bool GenerateNoise(int numInputs,
                   const AudioSampleFrame* inputs,
                   int numOutputs,
                   AudioSampleFrame* outputs,
                   int numParams,
                   const AudioParamFrame* params,
                   void* userData) {
  for (int i = 0; i < numOutputs; ++i)
    for (int j = 0;
         j < outputs[i].samplesPerChannel * outputs[i].numberOfChannels;
         ++j)
      outputs[i].data[j] = emscripten_random() * 0.2 -
                           0.1; // Warning: scale down audio volume by factor of
                                // 0.2, raw noise can be really loud otherwise

  return true; // Keep the graph output going
}

void AudioWorkletProcessorCreated(EMSCRIPTEN_WEBAUDIO_T audioContext,
                                  bool success,
                                  void* userData) {
  if (!success)
    return; // Check browser console in a debug build for detailed errors

  int outputChannelCounts[1] = {1};
  EmscriptenAudioWorkletNodeCreateOptions options = {.numberOfInputs = 0,
                                                     .numberOfOutputs = 1,
                                                     .outputChannelCounts =
                                                       outputChannelCounts};

  // Create node
  EMSCRIPTEN_AUDIO_WORKLET_NODE_T wasmAudioWorklet =
    emscripten_create_wasm_audio_worklet_node(
      audioContext, "noise-generator", &options, &GenerateNoise, 0);

  // Connect it to audio context destination
  emscripten_audio_node_connect(wasmAudioWorklet, audioContext, 0, 0);

  // Resume context on mouse click
  emscripten_set_click_callback(
    "canvas", (void*)audioContext, 0, OnCanvasClick);
}

void AudioThreadInitialized(EMSCRIPTEN_WEBAUDIO_T audioContext,
                            bool success,
                            void* userData) {
  if (!success)
    return; // Check browser console in a debug build for detailed errors
  WebAudioWorkletProcessorCreateOptions opts = {
    .name = "noise-generator",
  };
  emscripten_create_wasm_audio_worklet_processor_async(
    audioContext, &opts, &AudioWorkletProcessorCreated, 0);
}

void functionThatStartsAWorklet(EMSCRIPTEN_WEBAUDIO_T context) {
  emscripten_start_wasm_audio_worklet_thread_async(context,
                                                   audioThreadStack,
                                                   sizeof(audioThreadStack),
                                                   &AudioThreadInitialized,
                                                   0);
}

void functionThatStartsAnotherWorklet(EMSCRIPTEN_WEBAUDIO_T context) {
  emscripten_start_wasm_audio_worklet_thread_async(context,
                                                   audioThreadStack,
                                                   sizeof(audioThreadStack),
                                                   &AudioThreadInitialized,
                                                   0);
}

int main() {}

<!doctype html>
<html lang="en-us">
  <head>
    <meta charset="utf-8">
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
    <title>Emscripten-Generated Code</title>
  </head>
  <body>
      <canvas class="emscripten" id="canvas" oncontextmenu="event.preventDefault()" tabindex=-1></canvas>
    <script type='text/javascript'>
      var canvasElement = document.getElementById('canvas');
    </script>
    <script type="text/javascript" src="a.out.js"></script>
    <script type="text/javascript">
      console.log(Module);
      (async function() {
          const audioContext = new AudioContext();

      const mainModule = await Module();
      const audioContextId = mainModule.emscriptenRegisterAudioObject(audioContext);
      mainModule._functionThatStartsAWorklet(audioContextId);

      const mainModule2 = await Module();
      const audioContextId2 =
        mainModule2.emscriptenRegisterAudioObject(audioContext);
        mainModule2._functionThatStartsAnotherWorklet(audioContextId2);
      })();
    </script>
  </body>
</html>

emcc -sEXPORTED_FUNCTIONS=_functionThatStartsAnotherWorklet,_functionThatStartsAWorklet,emscriptenRegisterAudioObject,_main -sMODULARIZE -sAUDIO_WORKLET -sWASM_WORKERS -g main.c -o a.out.js`

[lindell]

Thanks for the response @brendandahl.

This assertion should indeed be hit. Looking closer, my repro was with -sASSERTIONS=0. Since the assertion exist, this should probably be considered intended behavior for now. The then is if this is something that should be supported. I would argue it is reasonable, since you are allowed to (with modularize) use multiple Modules on the main thread. That the AudioContext should also allow for this.

The AudioContext is intended to be used as a graph with different nodes. To force all Worklet Nodes to use the same Module might not always be desirable.

[sbc100]

Yes, it certainly looks like this is something that API current API explicitly does not support. @juj originally designed this API so he would be the one who might know how likely this to be possible as an extension of the existing API.

[juj]

It is unfortunately currently not possible to have a single Audio Worklet thread operate against two different WebAssembly Modules.

The WebAssembly code in a Wasm Audio Worklet is compiled in a static manner against a single WebAssembly Module, and the implementation of Wasm Workers assumes that an individual Web Worker is always associated with/hosting a single Module.

To achieve this, the Audio Worklet Global Scope should instantiate into itself multiple copies of Wasm Worker control structures. There would need to exist multiple copies of the Module object, once for each instance, and the wwParams field at least.

Gut feeling says it should be possible to "modularize" also the Audio Worklet side, but it does seem super complex: it seems like it would require also modularizing/keying the audio worklet message passing, and keeping a match of those instantiated modules in sync with the main thread side.

.. the way that AudioWorkletGlobalScope is a forced singleton per audio context really doesn't lend to making this easier. If someone wants to give this a stab, go for it.. but I can't really say if the end result will be going to be easy to maintain/co-exist with other build modes.

[lindell]

Will try to workaround this for now.

We have a class, let's call it TheGenerator, that needs to create a Module to be used in an already existing AudioContext. We have to destroy TheGenerator at some point. Normally this is the end of it. But there are scenarios where another TheGenerators needs to be created, using the same AudioContext.

Our workaround will be to keep a map between AudioContext, and the Emscripten Module, and never destroy the Emscripten module as long as the AudioContext is alive, even if we almost always will never use it again.

[juj]

This is definitely a bit tricky situation.

Would using multiple AudioContexts resolve this as a workaround? Or does that run into other problems on its own..

If you might want to give a stab a modularizing the audio worklet JS code, it does seem like something that could be doable. Although it seems like a bit of a complex challenge to implement.