Merge pull request #31798 from peppy/carousel-v2-async-fix

Fix `Carousel.FilterAsync` not working when called from a non-update thread
ppy · Feb 5, 2025 · ceb424f · ceb424f
2 parents ea725e2 + e5943e4
commit ceb424f
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 16 deletions.
diff --git a/osu.Game.Tests/Visual/SongSelect/BeatmapCarouselV2TestScene.cs b/osu.Game.Tests/Visual/SongSelect/BeatmapCarouselV2TestScene.cs
@@ -130,7 +130,7 @@ protected void CreateCarousel()
             });
         }
 
-        protected void SortBy(FilterCriteria criteria) => AddStep($"sort {criteria.Sort} group {criteria.Group}", () => Carousel.Filter(criteria));
+        protected void SortBy(FilterCriteria criteria) => AddStep($"sort:{criteria.Sort} group:{criteria.Group}", () => Carousel.Filter(criteria));
 
         protected void WaitForDrawablePanels() => AddUntilStep("drawable panels loaded", () => Carousel.ChildrenOfType<ICarouselPanel>().Count(), () => Is.GreaterThan(0));
         protected void WaitForSorting() => AddUntilStep("sorting finished", () => Carousel.IsFiltering, () => Is.False);

diff --git a/osu.Game/Screens/SelectV2/Carousel.cs b/osu.Game/Screens/SelectV2/Carousel.cs
@@ -228,21 +228,16 @@ protected Carousel()
 
         private async Task performFilter()
         {
-            Debug.Assert(SynchronizationContext.Current != null);
-
             Stopwatch stopwatch = Stopwatch.StartNew();
             var cts = new CancellationTokenSource();
 
-            lock (this)
-            {
-                cancellationSource.Cancel();
-                cancellationSource = cts;
-            }
+            var previousCancellationSource = Interlocked.Exchange(ref cancellationSource, cts);
+            await previousCancellationSource.CancelAsync().ConfigureAwait(false);
 
             if (DebounceDelay > 0)
             {
                 log($"Filter operation queued, waiting for {DebounceDelay} ms debounce");
-                await Task.Delay(DebounceDelay, cts.Token).ConfigureAwait(true);
+                await Task.Delay(DebounceDelay, cts.Token).ConfigureAwait(false);
             }
 
             // Copy must be performed on update thread for now (see ConfigureAwait above).
@@ -266,19 +261,22 @@ await Task.Run(async () =>
                 {
                     log("Cancelled due to newer request arriving");
                 }
-            }, cts.Token).ConfigureAwait(true);
+            }, cts.Token).ConfigureAwait(false);
 
             if (cts.Token.IsCancellationRequested)
                 return;
 
-            log("Items ready for display");
-            carouselItems = items.ToList();
-            displayedRange = null;
+            Schedule(() =>
+            {
+                log("Items ready for display");
+                carouselItems = items.ToList();
+                displayedRange = null;
 
-            // Need to call this to ensure correct post-selection logic is handled on the new items list.
-            HandleItemSelected(currentSelection.Model);
+                // Need to call this to ensure correct post-selection logic is handled on the new items list.
+                HandleItemSelected(currentSelection.Model);
 
-            refreshAfterSelection();
+                refreshAfterSelection();
+            });
 
             void log(string text) => Logger.Log($"Carousel[op {cts.GetHashCode().ToString()}] {stopwatch.ElapsedMilliseconds} ms: {text}");
         }