Skip to content
This repository has been archived by the owner on Mar 14, 2024. It is now read-only.

Snippet in article about setSinkId() may cause a memory leak #4894

Closed
chrisguttandin opened this issue Jan 14, 2023 · 1 comment · Fixed by #4982
Closed

Snippet in article about setSinkId() may cause a memory leak #4894

chrisguttandin opened this issue Jan 14, 2023 · 1 comment · Fixed by #4982
Assignees
Labels
content P2 A normal priority task. This is the default for most issues.

Comments

@chrisguttandin
Copy link

Describe the bug
The article about setSinkId() (https://developer.chrome.com/en/blog/audiocontext-setsinkid/) contains a snippet which may cause a memory leak.

The article recommends to trigger the permission to view all audio output devices by calling getUserMedia() like this:

await navigator.mediaDevices.getUserMedia({ audio: true });

As far as I can tell this will keep the returned stream alive indefinitely in Chrome (tested in v109 and v111). Firefox for example will garbage collect the stream.

To Reproduce
Steps to reproduce the behavior:

  1. Run the line of code from above.
  2. Observe that the microphone indicator of the operating system (tested on macOS) will never go away again.

Expected behavior
I think the stream should be stopped immediately when getUserMedia() is only used to trigger the permission to view more devices.

const mediaStream = await navigator.mediaDevices.getUserMedia({ audio: true });

mediaStream.getTracks().forEach((track) => track.stop());

I'm happy to provide a PR to update the article.

Additional context
By coincidence there is currently a spec discussion if garbage collecting an unused stream like that should be allowed or not. w3c/mediacapture-main#910

@chrisguttandin chrisguttandin added bug Something on the site is broken! P2 A normal priority task. This is the default for most issues. labels Jan 14, 2023
@matthiasrohmer matthiasrohmer added content and removed bug Something on the site is broken! labels Jan 19, 2023
@beaufortfrancois
Copy link
Member

You're absolutely right @chrisguttandin
Thank you for raising this issue. I've started #4982 to address it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
content P2 A normal priority task. This is the default for most issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants