Fix a few memory leaks while tearing down the player #5698
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
absidue
requested review from
PikachuEXE,
efb4f5ff-1298-471a-8973-3d47447115dc,
ChunkyProgrammer,
kommunarr and
MarmadileManteater
github-actions
bot
added
the
PR: waiting for review
PikachuEXE
previously approved these changes
Sep 15, 2024
This comment was marked as resolved.
This comment was marked as resolved.
github-actions
bot
removed
the
PR: waiting for review
absidue
force-pushed
the
player-memory-leaks
branch
from
dc01a07
to
d9a27b8
Compare
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
efb4f5ff-1298-471a-8973-3d47447115dc
approved these changes
Sep 15, 2024
PikachuEXE
approved these changes
Sep 15, 2024
ChunkyProgrammer
approved these changes
Sep 15, 2024
PikachuEXE
added a commit
to PikachuEXE/FreeTube
that referenced
this pull request
Sep 16, 2024
* development: (29 commits) Translated using Weblate (Flemish (West)) Added translation using Weblate (Flemish (West)) Translated using Weblate (Slovak) Improve history import performance and fix some bugs (FreeTubeApp#5666) Bump electron from 32.0.2 to 32.1.0 (FreeTubeApp#5710) Local API: Use IOS HLS manifest for livestreams (FreeTubeApp#5705) Translated using Weblate (Slovak) Translated using Weblate (Japanese) Bump the stylelint group with 2 updates (FreeTubeApp#5706) Bump swiper from 11.1.12 to 11.1.14 (FreeTubeApp#5709) Fix a few memory leaks while tearing down the player (FreeTubeApp#5698) Fix audio track selection (FreeTubeApp#5697) Bump shaka-player from 4.10.12 to 4.11.1 (FreeTubeApp#5677) Translated using Weblate (Czech) Translated using Weblate (Serbian) Translated using Weblate (Polish) Translated using Weblate (German) Translated using Weblate (Chinese (Simplified Han script)) Bump express from 4.19.2 to 4.20.0 (FreeTubeApp#5687) Translated using Weblate (Turkish) ...
PikachuEXE
added a commit
to PikachuEXE/FreeTube
that referenced
this pull request
Sep 16, 2024
* development: (74 commits) Translated using Weblate (Flemish (West)) Added translation using Weblate (Flemish (West)) Translated using Weblate (Slovak) Improve history import performance and fix some bugs (FreeTubeApp#5666) Bump electron from 32.0.2 to 32.1.0 (FreeTubeApp#5710) Local API: Use IOS HLS manifest for livestreams (FreeTubeApp#5705) Translated using Weblate (Slovak) Translated using Weblate (Japanese) Bump the stylelint group with 2 updates (FreeTubeApp#5706) Bump swiper from 11.1.12 to 11.1.14 (FreeTubeApp#5709) Fix a few memory leaks while tearing down the player (FreeTubeApp#5698) Fix audio track selection (FreeTubeApp#5697) Bump shaka-player from 4.10.12 to 4.11.1 (FreeTubeApp#5677) Translated using Weblate (Czech) Translated using Weblate (Serbian) Translated using Weblate (Polish) Translated using Weblate (German) Translated using Weblate (Chinese (Simplified Han script)) Bump express from 4.19.2 to 4.20.0 (FreeTubeApp#5687) Translated using Weblate (Turkish) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix a few memory leaks while tearing down the player
Pull Request Type
Description
This pull request addresses two current memory leaks that occur while tearing down the player or more specifically two things that don't get torn down at the moment.
The first issue is that shaka-player's component registrations are global, however as we create our custom components with references to variables that are only meant to be used for the current player session, we need to clean those registrations up otherwise it will prevent the variables in question from being garbage collected until you play another video. Unfortunately shaka-player doesn't have a way to unregister a component, so I decided to overwrite the registration with
null.The second issue is that shaka-player's
destroy()method is asynchronous, however as Vue's lifecycle hooks are synchronous, it means that thedestroy()method isn't finished by the time FreeTube and Vue consider the player to be destroyed. To solve that I decided to introduce a method to theft-shaka-video-playercomponent which calls the destroy method on shaka-player. That way we can destroy the player first, wait for it to be finished and then prodeed with unmounting the Vue component.This doesn't fix all of the problems, as shaka-player doesn't fully clean up after itself, hopefully some of their recent pull requests will help with that and I've also opened an issue about some of the UI elements not getting removed properly. Additionally there are some other things that seem to hang around (according to the Memory tab in the devtools), but I'm not sure why, my guess is that it is a Vue 2 bug, if that is the case then the only solution is updating to Vue 3.
Desktop
Additional context
There may be code conflicts between this pull request and #5677, in which case please make sure that pull request is merged first.