From 842d508aeeeecddeaef662ff6a2be7c2eb0fe8ec Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 24 Dec 2021 18:38:17 +0900 Subject: [PATCH] Fix incorrect delgate capture leading to slow leak of audio tracks During profile, it was found that the `Completed` delegate was incorrectly also capturing `lastTrack`, leading to an unexpected reference chain that led to a memory leak over a long period of time. This solves the issue by moving the delegate construction to its own method, where it won't capture the other variables. --- osu.Game/Overlays/MusicController.cs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 76358fca3f..3346c6d97d 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -342,11 +342,9 @@ namespace osu.Game.Overlays private void changeTrack() { + var queuedTrack = getQueuedTrack(); + var lastTrack = CurrentTrack; - - var queuedTrack = new DrawableTrack(current.LoadTrack()); - queuedTrack.Completed += () => onTrackCompleted(current); - CurrentTrack = queuedTrack; // At this point we may potentially be in an async context from tests. This is extremely dangerous but we have to make do for now. @@ -370,6 +368,15 @@ namespace osu.Game.Overlays }); } + private DrawableTrack getQueuedTrack() + { + // Important to keep this in its own method to avoid inadvertently capturing unnecessary variables in the callback. + // Can lead to leaks. + var queuedTrack = new DrawableTrack(current.LoadTrack()); + queuedTrack.Completed += () => onTrackCompleted(current); + return queuedTrack; + } + private void onTrackCompleted(WorkingBeatmap workingBeatmap) { // the source of track completion is the audio thread, so the beatmap may have changed before firing.