From 1a22377e19d45ee88dab9e9effba1260b7bacf47 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Sun, 26 Jun 2022 16:42:28 +0900
Subject: [PATCH] Fix test deadlock due to TPL threadpool saturation

As found in aggressive CI runs:

```csharp
00007F6F527F32B0 00007f74f937c72d (MethodDesc 00007f74f5545a78 + 0x22d System.Threading.ManualResetEventSlim.Wait(Int32, System.Threading.CancellationToken)), calling (MethodDesc 00007f74f51c0838 + 0 System.Threading.Monitor.Wait(System.Object, Int32))
00007F6F527F3330 00007f74f93861ca (MethodDesc 00007f74f5259100 + 0x10a System.Threading.Tasks.Task.SpinThenBlockingWait(Int32, System.Threading.CancellationToken)), calling (MethodDesc 00007f74f5545a78 + 0 System.Threading.ManualResetEventSlim.Wait(Int32, System.Threading.CancellationToken))
00007F6F527F3390 00007f74f9385d11 (MethodDesc 00007f74f52590e8 + 0x81 System.Threading.Tasks.Task.InternalWaitCore(Int32, System.Threading.CancellationToken)), calling (MethodDesc 00007f74f5259100 + 0 System.Threading.Tasks.Task.SpinThenBlockingWait(Int32, System.Threading.CancellationToken))
00007F6F527F33E0 00007f74fc3823af (MethodDesc 00007f74f5934c08 + 0x4f System.Threading.Tasks.Task`1[[System.Boolean, System.Private.CoreLib]].GetResultCore(Boolean)), calling (MethodDesc 00007f74f52590e8 + 0 System.Threading.Tasks.Task.InternalWaitCore(Int32, System.Threading.CancellationToken))
00007F6F527F3400 00007f74fb0aad92 (MethodDesc 00007f74fb28b000 + 0x122 osu.Game.Tests.Visual.OnlinePlay.OnlinePlayTestScene+<>c__DisplayClass21_0.<Setup>b__1(osu.Game.Online.API.APIRequest)), calling (MethodDesc 00007f74f5934be8 + 0 System.Threading.Tasks.Task`1[[System.Boolean, System.Private.CoreLib]].get_Result())
00007F6F527F3470 00007f74fa9c6dd7 (MethodDesc 00007f74f921f6c8 + 0x77 osu.Game.Online.API.DummyAPIAccess.PerformAsync(osu.Game.Online.API.APIRequest))
00007F6F527F34C0 00007f74fb13c718 (MethodDesc 00007f74fb4e0c50 + 0x538 osu.Game.Database.OnlineLookupCache`3+<performLookup>d__13[[System.Int32, System.Private.CoreLib],[System.__Canon, System.Private.CoreLib],[System.__Canon, System.Private.CoreLib]].MoveNext())
```

Going with the simplest solution, as what we had was pretty ugly to start with and I don't want to overthink this, just push a fix and hope for the best.
---
 .../Visual/OnlinePlay/OnlinePlayTestScene.cs  | 30 ++++++++-----------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs b/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs
index 282312c9c1..6577057c17 100644
--- a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs
+++ b/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs
@@ -4,14 +4,14 @@
 #nullable disable
 
 using System;
-using System.Threading.Tasks;
 using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
-using osu.Game.Database;
+using osu.Framework.Logging;
 using osu.Game.Beatmaps;
+using osu.Game.Database;
 using osu.Game.Online.API;
 using osu.Game.Online.Rooms;
 using osu.Game.Screens.OnlinePlay;
@@ -72,22 +72,18 @@ namespace osu.Game.Tests.Visual.OnlinePlay
 
             ((DummyAPIAccess)API).HandleRequest = request =>
             {
-                TaskCompletionSource<bool> tcs = new TaskCompletionSource<bool>();
-
-                // Because some of the handlers use realm, we need to ensure the game is still alive when firing.
-                // If we don't, a stray `PerformAsync` could hit an `ObjectDisposedException` if running too late.
-                Scheduler.Add(() =>
+                try
                 {
-                    bool result = handler.HandleRequest(request, API.LocalUser.Value, beatmapManager);
-                    tcs.SetResult(result);
-                }, false);
-
-#pragma warning disable RS0030
-                // We can't GetResultSafely() here (will fail with "Can't use GetResultSafely from inside an async operation."), but Wait is safe enough due to
-                // the task being a TaskCompletionSource.
-                // Importantly, this doesn't deadlock because of the scheduler call above running inline where feasible (see the `false` argument).
-                return tcs.Task.Result;
-#pragma warning restore RS0030
+                    return handler.HandleRequest(request, API.LocalUser.Value, beatmapManager);
+                }
+                catch (ObjectDisposedException)
+                {
+                    // These requests can be fired asynchronously, but potentially arrive after game components
+                    // have been disposed (ie. realm in BeatmapManager).
+                    // This only happens in tests and it's easiest to ignore them for now.
+                    Logger.Log($"Handled {nameof(ObjectDisposedException)} in test request handling");
+                    return true;
+                }
             };
         });