From 45d537ef727e787afeaf044b9cfc12af50c29bf9 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 10 Mar 2022 18:50:53 +0900 Subject: [PATCH 1/3] Fix potential multiplayer crash with async disposal --- osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs index d8fb448437..b49f373c7b 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs @@ -175,7 +175,10 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer base.StartGameplay(); }); - private void onResultsReady() => resultsReady.SetResult(true); + private void onResultsReady() => Scheduler.Add(() => + { + resultsReady.SetResult(true); + }); protected override async Task PrepareScoreForResultsAsync(Score score) { From 2d135e5be61e2615fe045747a34c834d31d3c7fb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 10 Mar 2022 19:52:08 +0900 Subject: [PATCH 2/3] Explain purpose of `Schedule` call in `MultiplayerPlayer.onResultsReady` --- .../Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs index b49f373c7b..78e7f3d987 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs @@ -175,10 +175,12 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer base.StartGameplay(); }); - private void onResultsReady() => Scheduler.Add(() => + private void onResultsReady() { - resultsReady.SetResult(true); - }); + // Schedule is required to ensure that `TaskCompletionSource.SetResult` is not called more than once. + // A scenario where this can occur is if this instance is not immediately disposed (ie. async disposal queue). + Schedule(() => resultsReady.SetResult(true)); + } protected override async Task PrepareScoreForResultsAsync(Score score) { From 885cb3ce5bc0768a3e434436a5e540bfaff4a5b4 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 10 Mar 2022 20:02:58 +0900 Subject: [PATCH 3/3] Add current screen checks for added safety --- .../OnlinePlay/Multiplayer/MultiplayerPlayer.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs index 78e7f3d987..043315c790 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs @@ -10,6 +10,7 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Logging; +using osu.Framework.Screens; using osu.Game.Graphics.UserInterface; using osu.Game.Online.Multiplayer; using osu.Game.Online.Rooms; @@ -171,6 +172,9 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer private void onMatchStarted() => Scheduler.Add(() => { + if (!this.IsCurrentScreen()) + return; + loadingDisplay.Hide(); base.StartGameplay(); }); @@ -179,7 +183,13 @@ namespace osu.Game.Screens.OnlinePlay.Multiplayer { // Schedule is required to ensure that `TaskCompletionSource.SetResult` is not called more than once. // A scenario where this can occur is if this instance is not immediately disposed (ie. async disposal queue). - Schedule(() => resultsReady.SetResult(true)); + Schedule(() => + { + if (!this.IsCurrentScreen()) + return; + + resultsReady.SetResult(true); + }); } protected override async Task PrepareScoreForResultsAsync(Score score)