From 32d56fe3a959a40082094b3d9399e178039f8766 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Mon, 26 Sep 2022 16:02:33 +0900
Subject: [PATCH] Use request cancellation and `IsDisposed` guard instead of
 more `Schedule`

---
 osu.Game/Online/Leaderboards/Leaderboard.cs     | 13 +++++++++----
 .../Select/Leaderboards/BeatmapLeaderboard.cs   | 17 ++++++++++-------
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/osu.Game/Online/Leaderboards/Leaderboard.cs b/osu.Game/Online/Leaderboards/Leaderboard.cs
index 206c15afde..0f444a368a 100644
--- a/osu.Game/Online/Leaderboards/Leaderboard.cs
+++ b/osu.Game/Online/Leaderboards/Leaderboard.cs
@@ -185,10 +185,15 @@ namespace osu.Game.Online.Leaderboards
             if (scores != null)
                 this.scores.AddRange(scores);
 
-            // Schedule needs to be non-delayed here for the weird logic in refetchScores to work.
-            // If it is removed, the placeholder will be incorrectly updated to "no scores" rather than "retrieving".
-            // This whole flow should be refactored in the future.
-            Scheduler.Add(applyNewScores, false);
+            // Non-delayed schedule may potentially run inline (due to IsMainThread check passing) after leaderboard  is disposed.
+            // This is guarded against in BeatmapLeaderboard via web request cancellation, but let's be extra safe.
+            if (!IsDisposed)
+            {
+                // Schedule needs to be non-delayed here for the weird logic in refetchScores to work.
+                // If it is removed, the placeholder will be incorrectly updated to "no scores" rather than "retrieving".
+                // This whole flow should be refactored in the future.
+                Scheduler.Add(applyNewScores, false);
+            }
 
             void applyNewScores()
             {
diff --git a/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs b/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs
index ffb13dc458..50204db2bf 100644
--- a/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs
+++ b/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs
@@ -89,6 +89,8 @@ namespace osu.Game.Screens.Select.Leaderboards
 
         private IDisposable scoreSubscription;
 
+        private GetScoresRequest scoreRetrievalRequest;
+
         [BackgroundDependencyLoader]
         private void load()
         {
@@ -151,15 +153,14 @@ namespace osu.Game.Screens.Select.Leaderboards
             else if (filterMods)
                 requestMods = mods.Value;
 
-            var req = new GetScoresRequest(fetchBeatmapInfo, fetchRuleset, Scope, requestMods);
+            scoreRetrievalRequest = new GetScoresRequest(fetchBeatmapInfo, fetchRuleset, Scope, requestMods);
 
-            // Schedule is required to avoid potential object disposed exception when LoadComponentAsync is eventually called.
-            req.Success += r => Schedule(() => SetScores(
-                scoreManager.OrderByTotalScore(r.Scores.Select(s => s.ToScoreInfo(rulesets, fetchBeatmapInfo))),
-                r.UserScore?.CreateScoreInfo(rulesets, fetchBeatmapInfo)
-            ));
+            scoreRetrievalRequest.Success += response => SetScores(
+                scoreManager.OrderByTotalScore(response.Scores.Select(s => s.ToScoreInfo(rulesets, fetchBeatmapInfo))),
+                response.UserScore?.CreateScoreInfo(rulesets, fetchBeatmapInfo)
+            );
 
-            return req;
+            return scoreRetrievalRequest;
         }
 
         protected override LeaderboardScore CreateDrawableScore(ScoreInfo model, int index) => new LeaderboardScore(model, index, IsOnlineScope)
@@ -219,7 +220,9 @@ namespace osu.Game.Screens.Select.Leaderboards
         protected override void Dispose(bool isDisposing)
         {
             base.Dispose(isDisposing);
+
             scoreSubscription?.Dispose();
+            scoreRetrievalRequest?.Cancel();
         }
     }
 }