mirror of
https://github.com/ppy/osu.git
synced 2026-05-21 01:39:54 +08:00
Bartłomiej Dach
ce8406613a
Exposed by CI failures ([example](https://github.com/ppy/osu/actions/runs/23888446400#user-content-r0s0)). The race occurs when a consumer calls `GetBindableDifficulty()` for the first time and then a cache invalidation is triggered. The sequence of events triggering the failure is as follows: 1. Consumer calls `GetBindableDifficulty()` to get a difficulty bindable for a given beatmap tracking the game-global ruleset / mods. This triggers difficulty calculation A. 2. In the meantime, another process requests a cache invalidation for the same beatmap as the one supplied by the consumer in step (1). This incurs a cache purge and triggers difficulty calculation B, but never cancels difficulty calculation A. 3. Difficulty calculation B concludes and writes the correct, latest difficulty value to the bindable. 4. Difficulty calculation A concludes and writes an incorrect, stale difficulty value to the bindable. See below for patch that reproduces this behaviour on my machine 100% reliably: ```diff diff --git a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs index https://github.com/ppy/osu/commit/d6b40639161e26af223f03761b3826b0cd7f4a87..c9604e0e58 100644 --- a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs +++ b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs @@ -252,17 +252,17 @@ private void updateBindable(BindableStarDifficulty bindable, IRulesetInfo? rules GetDifficultyAsync(bindable.BeatmapInfo, rulesetInfo, mods, cancellationToken, computationDelay) .ContinueWith(task => { + StarDifficulty? starDifficulty = task.GetResultSafely(); + // We're on a threadpool thread, but we should exit back to the update thread so consumers can safely handle value-changed events. - Schedule(() => + Scheduler.AddDelayed(() => { if (cancellationToken.IsCancellationRequested) return; - StarDifficulty? starDifficulty = task.GetResultSafely(); - if (starDifficulty != null) bindable.Value = starDifficulty.Value; - }); + }, starDifficulty?.Stars > 0 ? 400 : 0); }, cancellationToken); } ``` The goal of the patch is to reorder the write to the bindable in order to trigger the scenario described above. Due to the invasiveness of the patch it is not suitable to add as a test, and chances are that the schedule delay may need to be tweaked if anyone else wants to check that patch. Anyway, the solution here is to use the same pattern of creating a linked cancellation token even on the first retrieval of a bindable difficulty, and registering it in the list of cancellation tokens that already existed to service the ruleset- / mod-tracking flow. Some extra rearranging in https://github.com/ppy/osu/commit/9184299239a8b5e82957d46db33f1d26bab238fd is needed to ensure the linked tokens created to do this don't stay behind after they are no longer needed for anything.
ce8406613a
·
2026-04-03 21:57:07 +09:00
History