mirror of
https://github.com/ppy/osu.git
synced 2024-11-13 18:47:27 +08:00
3c5e9ac9a9
Closes https://github.com/ppy/osu/issues/26035. `submitOnFailOrQuit()`, as the name suggests, can be called both when the player has failed, or when the player screen is being exited from. Notably, when perfect mod with auto-retry is active, the two happen almost simultaneously. This double call exposes a data race in `submitScore()` concerning the handling of `scoreSubmissionSource`. The race could be experimentally confirmed by applying the following patch: diff --git a/osu.Game/Screens/Play/SubmittingPlayer.cs b/osu.Game/Screens/Play/SubmittingPlayer.cs index 83adf1f960..76dd29bbdb 100644 --- a/osu.Game/Screens/Play/SubmittingPlayer.cs +++ b/osu.Game/Screens/Play/SubmittingPlayer.cs @@ -228,6 +228,7 @@ private Task submitScore(Score score) return Task.CompletedTask; } + Logger.Log($"{nameof(scoreSubmissionSource)} is {(scoreSubmissionSource == null ? "null" : "not null")}"); if (scoreSubmissionSource != null) return scoreSubmissionSource.Task; @@ -237,6 +238,7 @@ private Task submitScore(Score score) Logger.Log($"Beginning score submission (token:{token.Value})..."); + Logger.Log($"creating new {nameof(scoreSubmissionSource)}"); scoreSubmissionSource = new TaskCompletionSource<bool>(); var request = CreateSubmissionRequest(score, token.Value); which would result in the following log output: [runtime] 2024-01-02 09:54:13 [verbose]: scoreSubmissionSource is null [runtime] 2024-01-02 09:54:13 [verbose]: scoreSubmissionSource is null [runtime] 2024-01-02 09:54:13 [verbose]: Beginning score submission (token:36780)... [runtime] 2024-01-02 09:54:13 [verbose]: creating new scoreSubmissionSource [runtime] 2024-01-02 09:54:13 [verbose]: Beginning score submission (token:36780)... [runtime] 2024-01-02 09:54:13 [verbose]: creating new scoreSubmissionSource [network] 2024-01-02 09:54:13 [verbose]: Performing request osu.Game.Online.Solo.SubmitSoloScoreRequest [network] 2024-01-02 09:54:14 [verbose]: Request to https://dev.ppy.sh/api/v2/beatmaps/869310/solo/scores/36780 successfully completed! [network] 2024-01-02 09:54:14 [verbose]: SubmitSoloScoreRequest finished with response size of 639 bytes [network] 2024-01-02 09:54:14 [verbose]: Performing request osu.Game.Online.Solo.SubmitSoloScoreRequest [runtime] 2024-01-02 09:54:14 [verbose]: Score submission completed! (token:36780 id:20247) [network] 2024-01-02 09:54:14 [verbose]: Request to https://dev.ppy.sh/api/v2/beatmaps/869310/solo/scores/36780 successfully completed! [network] 2024-01-02 09:54:14 [verbose]: SubmitSoloScoreRequest finished with response size of 639 bytes [runtime] 2024-01-02 09:54:14 [error]: An unhandled error has occurred. [runtime] 2024-01-02 09:54:14 [error]: System.InvalidOperationException: An attempt was made to transition a task to a final state when it had already completed. [runtime] 2024-01-02 09:54:14 [error]: at osu.Game.Screens.Play.SubmittingPlayer.<>c__DisplayClass30_0.<submitScore>b__0(MultiplayerScore s) in /home/dachb/Documents/opensource/osu/osu.Game/Screens/Play/SubmittingPlayer.cs:line 250 The intention of the submission logic was to only ever create one `scoreSubmissionSource`, and then reuse this one if a redundant submission request was made. However, because of the temporal proximity of fail and quit in this particular case, combined with the fact that the calls to `submitScore()` are taking place on TPL threads, means that there is a read-write data race on `scoreSubmissionSource`, wherein the source can be actually created twice. This leads to two concurrent score submission requests, which, upon completion, attempt to transition only _the second_ `scoreSubmissionSource` to a final state (this is because the API success/failure request callbacks capture `this`, i.e. the entire `SubmittingPlayer` instance, rather than the `scoreSubmissionSource` reference specifically). To fix, ensure correct synchronisation on the read-write critical section, which should prevent the `scoreSubmissionSource` from being created multiple times. |
||
|---|---|---|
| .. | ||
| Break | ||
| HUD | ||
| PlayerSettings | ||
| ArgonKeyCounter.cs | ||
| ArgonKeyCounterDisplay.cs | ||
| BeatmapMetadataDisplay.cs | ||
| BreakOverlay.cs | ||
| BreakTracker.cs | ||
| ComboEffects.cs | ||
| DimmableStoryboard.cs | ||
| EpilepsyWarning.cs | ||
| FailAnimationContainer.cs | ||
| FailOverlay.cs | ||
| GameplayClockContainer.cs | ||
| GameplayClockExtensions.cs | ||
| GameplayMenuOverlay.cs | ||
| GameplayOffsetControl.cs | ||
| GameplayState.cs | ||
| HotkeyExitOverlay.cs | ||
| HotkeyRetryOverlay.cs | ||
| HUDOverlay.cs | ||
| IGameplayClock.cs | ||
| ILocalUserPlayInfo.cs | ||
| MasterGameplayClockContainer.cs | ||
| OffsetCorrectionClock.cs | ||
| PauseOverlay.cs | ||
| Player.cs | ||
| PlayerConfiguration.cs | ||
| PlayerLoader.cs | ||
| PlayerTouchInputDetector.cs | ||
| ReplayPlayer.cs | ||
| ReplayPlayerLoader.cs | ||
| ResumeOverlay.cs | ||
| RoomSubmittingPlayer.cs | ||
| SaveFailedScoreButton.cs | ||
| ScreenSuspensionHandler.cs | ||
| ScreenWithBeatmapBackground.cs | ||
| SkipOverlay.cs | ||
| SoloPlayer.cs | ||
| SoloSpectatorPlayer.cs | ||
| SoloSpectatorScreen.cs | ||
| SpectatorPlayer.cs | ||
| SpectatorPlayerLoader.cs | ||
| SpectatorResultsScreen.cs | ||
| SquareGraph.cs | ||
| SubmittingPlayer.cs | ||