Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reset() does not completely destroy MediaPlayer (memory leak) #3431

Closed
5 tasks done
andrewagain opened this issue Oct 15, 2020 · 0 comments · Fixed by #3432
Closed
5 tasks done

reset() does not completely destroy MediaPlayer (memory leak) #3431

andrewagain opened this issue Oct 15, 2020 · 0 comments · Fixed by #3432

Comments

@andrewagain
Copy link
Contributor

Environment
Steps to reproduce
  1. Create and initialize a MediaPlayer.
  2. Call '.reset()' on MediaPlayer
  3. Notice that many resources are not freed.
See the Leak in Devtools

The above steps will reproduce the leak but you have to look in chrome dev tools at the memory tab. You'll see something like this:

dash-singleton-leak

Notice that it looks like getSingletonInstance is retaining a reference indirectly to a VideoTrackList which will retain the HTMLVideoElement as well.

See the Leak more Easily

I created another way to see the leak here: https://github.com/ahfarmer/dash-mem-leak

Run these commands:

git clone git@github.com:ahfarmer/dash-mem-leak.git
cd dash-mem-leak
yarn install
node bin/modify-dash.js
yarn start

( bin/modify-dash.js will modify FactoryMaker.js, adding two console log lines that make it easier to see the issue. )

Then open http://localhost:3000/ and open your dev console. Press the "Recreate" button. This will call reset() on the current media player and then create a new one.

You will see in the console that I am printing the length of singletonContexts from FactoryMaker.js. The count just keeps going up and up, even though I reset my media players before I stop using them, and there is no longer any reference to previous players.

I believe this clearly shows the leak in FactoryMaker.js. It retains the 'context' for every MediaPlayer ever created. This context indirectly references many other objects, including the HTMLVideoElement that was used. On my site I create & delete MediaPlayers quite frequently (every 5-10 seconds) and our users have reported that their browsers eventually crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant