I posted a comment atWebAudio/web-audio-api#2658 (comment) to involve the spec folks.

This indeed looks like a very difficult problem.

We definitely do want a synchronousemscripten_destroy_web_audio_node(), which guarantees that it is safe and possible tofree() any and all shared data control structures that might have been used between an Audio Worklet node and the main thread, synchronously after theemscripten_destroy_web_audio_node() call returns.

Otherwise would be madness.

The only way I can see to implement that mechanism today is tomalloc() a short control block that contains abool shouldShutDown; boolean, and have the Audio Worklet side poll for that boolean in theprocess() function. WhenshouldShutDown becomes true, then the Audio Worklet willfree() the control block andreturn false;

This will unfortunately lead to a memory leak if the Web Audio graph never resumes, but that is not the end of the world. The way bigger problem here seems to be that this will unconditionally require involving the dynamic memory allocator into Audio Worklets (which the current design of Audio Worklets has not required so far)

@juj It does also not take into account that the process callback can currently run when theshouldShutDown is set. This would still allow for use after frees.

Here's my attempt to fix. Iiuc this should give a synchronous guarantee that the audio worklet will no longer process after being shut down:main...juj:emscripten:fix_emscripten_destroy_web_audio_node

Did not give it a test yet though, wrote it blind. So at this point just to give an idea.

@juj Unfortunetely that implementation will not work ifemscripten_destroy_web_audio_node is called while the the process callback is running and we free resources connected to in on the main thread right afteremscripten_destroy_web_audio_node is called.

Hmm yeah, I was missing a semaphore to prevent the main thread from proceeding while the callback is live. Added one now, I think that should help synchronize the main thread better.

@juj This approach looks good to me 👍 Since we will might spinlock in rare cases, but that will not be dependent on the scheduling, just that the worklet execution will complete. And it keeps the API sync.

🚀

@sbc100@cwoffenden What do you think about@juj's approach? If you like it, I close this one and Juj can create another PR.

What do you think about@juj's approach?

I think it's a good way of achieving this. I would wrap the atomics calls with Emscripten's existing code where possible, then they're function calls rather than adding more code (and that atomic exchange, at first glance, isn't quite right).

And to add: whilst I said I wouldn't personally use such a call, I do think it's needed, and I'm 50/50 on thinking that not shutting down is why the AW CI tests hang.

There are some issues with my branch proposal that I don't like:

• it adds unconditional dependency tomalloc() from Audio Worklets. I took a lot of care with the original Audio Worklets implementation to ensure that it wouldn't fundamentally have ties to malloc/free, to provide memory management flexibility and small code size.
• it adds code size that is not easy to recover. This is a textbook scenario of "only fixes an issue some people have, but at the cost of regressing code size for every project that utilizes Audio Worklets." Ideally, we'd codegen the fix in only ifemscripten_destroy_web_audio_node() is ever called, but that is not easy to achieve in our current system. I am leaning towards adding a -sAUDIO_WORKLET_SUPPORT_DESTROY_NODE linker flag to opt out from this code (defaulting true of course)
• current code creates a memory leak in scenarios where a node is not added to the graph at all (e.g. if one callsemscripten_create_wasm_audio_worklet_node() +emscripten_destroy_web_audio_node() without ever connecting the created node in the graph in between. Will need to fix that.
• there was that use case somewhere that asked supporting Audio Worklets without SharedArrayBuffer - this code will veer farther from that being a possibility.

I would wrap the atomics calls with Emscripten's existing code where possible, then they're function calls rather than adding more code

There unfortunately does not exist JS code in Emscripten to do JS-side atomics calls - we only have wasm side atomics. (Calling from JS to wasm to do the atomics wouldn't be useful)

Dedicating JS functions for the atomics would have outlining benefit to avoid repeating the long stringAtomics.exchange, although two occurrences of that function is barely a size win. I could try giving that a go though.

(and that atomic exchange, at first glance, isn't quite right).

Yeh, that needs to be tested in detail - didn't run the code yet.

There unfortunately does not exist JS code in Emscripten to do JS-side atomics calls

True, I wrote them recently for testing pure JS Emscripten-like code, and I had to just remind myself:

https://github.com/cwoffenden/chrome-lock-bug/blob/main/lock-bug-js.html#L62

Dedicating JS functions for the atomics would have outlining benefit to avoid repeating

You might be able to use the JS lock functions directly.

• it adds unconditional dependency tomalloc() from Audio Worklets. I took a lot of care with the original Audio Worklets implementation to ensure that it wouldn't fundamentally have ties to malloc/free, to provide memory management flexibility and small code size.

Given that the control blocks being allocated are only 8 bytes, can we consider simply statically allocating a fixed amount of them? i.e. would it be reasonable to set and upper limit on the number of active audio worklets to say, 16, or 32? ~100 bytes of static BSS data is way cheaper than malloc/free.

Given that the control blocks being allocated are only 8 bytes, can we consider simply statically allocating a fixed amount of them? i.e. would it be reasonable to set and upper limit on the number of active audio worklets to say, 16, or 32? ~100 bytes of static BSS data is way cheaper than malloc/free.

I considered that as well, though there will definitely be some use case that allocates Audio Worklet Nodes to implement some massive audio graph synthesis, that would then have an issue of not having the ability to allocate 50, 100, 1000 or whatever number of nodes they need.