1878516 - `BaseAudioContext` `closed` state doesn't free audio worklet resources

Status: RESOLVED MOVED

(Core :: Web Audio, enhancement)

Description

[daxpedda]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:122.0) Gecko/20100101 Firefox/122.0

Steps to reproduce:

In index.html:

<script type=module>
    onclick = async () => {
        const audioContext = new AudioContext();
        await audioContext.audioWorklet.addModule('worklet.js')

        const port = new MessageChannel
        const worklet = new AudioWorkletNode(audioContext, 'test')
        worklet.port.postMessage(undefined, [port.port2])
        port.port1.postMessage('hello from main')
        audioContext.addEventListener(
            'statechange',
            () => audioContext.state == 'closed' ? port.port1.postMessage('hello from main after being closed') : {},
        )
        await audioContext.close()

        onclick = () => audioContext.state == 'closed' ? port.port1.postMessage('hello from main after being closed') : {}
    }
</script>

In worklet.js:

registerProcessor('test', class extends AudioWorkletProcessor {
    constructor() {
        super()
        this.port.onmessage = event => event.ports[0].onmessage = event => console.log(event.data)
    }

    process() {}
})

Actual results:

The audio worklet continued to receive and handle messages.

Expected results:

The audio worklet and all it's resources should have been freed according to the spec.

This seems to work correctly on Chrome.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Web Audio' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Paul Adenot (:padenot)]

Reporter, thanks for filing. The spec isn't exactly clear on whether message are working when a context is suspended, or closed, because the way the event loop is defined is not very precise right now, this is being worked on. It's quite useful to be able to send messages to suspended context (e.g. to set things up), and potentially closed (to close things up).

Karl, is there audio-thread side things we could clean up here?

Comment 3

[daxpedda]

For context:
The issue originated from trying to deal with Wasm threads: Wasm needs to know when it can clean up a thread.
For dedicated workers this isn't an issue, because we have Worker.terminate(); so a worker can be terminated and the Wasm thread safely cleaned up.

Wasm would need something similar for audio worklets, I thought that AudioContext.close() (or just waiting for the closed state) would be a sufficient equivalent. But if Wasm cleans up the thread and it can still be called into, e.g. an event handler on a MessagePort, it could lead to undefined behavior in Wasm.

I opened an issue at Web Audio to ask for clarification on the event loop: https://github.com/WebAudio/web-audio-api/issues/2568.

Comment 4

[Paul Adenot (:padenot)]

Thanks for opening an issue there, all very reasonable question and requests, let's figure out the best behavior, spec it, and then we can follow up here.

Comment 5

[Karl Tomlinson (:karlt)]

"closed" indicates that

This context has been released, and can no longer be used to process audio. All system audio resources have been released.

I don't think there is any affect specified for close() on the MessagePorts, which means that a conforming user agent would leave these open.

It sounds like Wasm might provide a use case for some way to explicitly terminate the WorkletGlobalScope and perhaps prevent creating further scopes. Thank you for opening the spec issue on that.

(In reply to Paul Adenot (:padenot) from comment #2)

It's quite useful to be able to send messages to suspended context (e.g. to set things up), and potentially closed (to close things up).

Yes, definitely the former. The latter could be useful too, but I guess a suspend(), finalize in worklet scope, close() ordering would suffice.

Karl, is there audio-thread side things we could clean up here?

The MessagePorts are hard to GC because of interacting threads (and we don't yet even do that in the simple cases - bug 1612997), so a way to close all MessagePorts would be convenient.

Comment 6

[daxpedda]

(In reply to Karl Tomlinson (:karlt) from comment #5)

It sounds like Wasm might provide a use case for some way to explicitly terminate the WorkletGlobalScope and perhaps prevent creating further scopes. Thank you for opening the spec issue on that.

I opened https://github.com/whatwg/html/issues/10169 for this.

---

It seems to me like Firefox is actually following the spec here and this issue can be closed?

Comment 7

[Karl Tomlinson (:karlt)]

Thank you for posting your investigations and details here and in the spec issues.

If there is something making GC difficult for browsers (such as concurrent threads), then some explicit shut down API would be beneficial.
(That's intentionally a vague statement because I don't have a great understanding.)

But, yes, I think Firefox is following the spec, and it sounds like changes to the spec would differ from what was initially proposed here, so this bug has passed its useful life, and it'll be clearer to have a fresh bug when we have a better sense of direction.