Merge pull request #18816 from peppy/avoid-waiting-forever

Ban usage of `ManualResetEventSlim.Wait()` without a timeout value
ppy · Jun 23, 2022 · 9ed3762 · 9ed3762
2 parents ac91c9d + 51268d0
commit 9ed3762
Show file tree

Hide file tree

Showing 7 changed files with 16 additions and 8 deletions.
diff --git a/CodeAnalysis/BannedSymbols.txt b/CodeAnalysis/BannedSymbols.txt
@@ -16,3 +16,4 @@ M:Realms.CollectionExtensions.SubscribeForNotifications`1(System.Linq.IQueryable
 M:Realms.CollectionExtensions.SubscribeForNotifications`1(System.Collections.Generic.IList{``0},Realms.NotificationCallbackDelegate{``0});Use osu.Game.Database.RealmObjectExtensions.QueryAsyncWithNotifications(IList<T>,NotificationCallbackDelegate<T>) instead.
 M:System.Threading.Tasks.Task.Wait();Don't use Task.Wait. Use Task.WaitSafely() to ensure we avoid deadlocks.
 P:System.Threading.Tasks.Task`1.Result;Don't use Task.Result. Use Task.GetResultSafely() to ensure we avoid deadlocks.
+M:System.Threading.ManualResetEventSlim.Wait();Specify a timeout to avoid waiting forever.
diff --git a/osu.Game.Benchmarks/BenchmarkRealmReads.cs b/osu.Game.Benchmarks/BenchmarkRealmReads.cs
@@ -76,7 +76,7 @@ public void BenchmarkDirectPropertyReadUpdateThread()
                 }
             });
 
-            done.Wait();
+            done.Wait(60000);
         }
 
         [Benchmark]
@@ -115,7 +115,7 @@ public void BenchmarkRealmLivePropertyReadUpdateThread()
                 }
             });
 
-            done.Wait();
+            done.Wait(60000);
         }
 
         [Benchmark]

diff --git a/osu.Game.Tests/Database/GeneralUsageTests.cs b/osu.Game.Tests/Database/GeneralUsageTests.cs
@@ -79,11 +79,11 @@ public void TestBlockOperationsWithContention()
                     {
                         hasThreadedUsage.Set();
 
-                        stopThreadedUsage.Wait();
+                        stopThreadedUsage.Wait(60000);
                     });
                 }, TaskCreationOptions.LongRunning | TaskCreationOptions.HideScheduler);
 
-                hasThreadedUsage.Wait();
+                hasThreadedUsage.Wait(60000);
 
                 Assert.Throws<TimeoutException>(() =>
                 {

diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableLoungeRoom.cs
@@ -46,7 +46,7 @@ private void load()
                 {
                     Task.Run(() =>
                     {
-                        allowResponseCallback.Wait();
+                        allowResponseCallback.Wait(10000);
                         allowResponseCallback.Reset();
                         Schedule(() => d?.Invoke("Incorrect password"));
                     });

diff --git a/osu.Game/Database/DatabaseContextFactory.cs b/osu.Game/Database/DatabaseContextFactory.cs
@@ -209,6 +209,10 @@ public void FlushConnections()
 
         public void SetMigrationCompletion() => migrationComplete.Set();
 
-        public void WaitForMigrationCompletion() => migrationComplete.Wait();
+        public void WaitForMigrationCompletion()
+        {
+            if (!migrationComplete.Wait(300000))
+                throw new TimeoutException("Migration took too long (likely stuck).");
+        }
     }
 }
diff --git a/osu.Game/Graphics/ScreenshotManager.cs b/osu.Game/Graphics/ScreenshotManager.cs
@@ -103,7 +103,9 @@ public Task TakeScreenshotAsync() => Task.Run(async () =>
                             framesWaitedEvent.Set();
                     }, 10, true);
 
-                    framesWaitedEvent.Wait();
+                    if (!framesWaitedEvent.Wait(1000))
+                        throw new TimeoutException("Screenshot data did not arrive in a timely fashion");
+
                     waitDelegate.Cancel();
                 }
             }

diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs
@@ -451,7 +451,8 @@ public bool Migrate(string path)
                     readyToRun.Set();
                 }, false);
 
-                readyToRun.Wait();
+                if (!readyToRun.Wait(30000))
+                    throw new TimeoutException("Attempting to block for migration took too long.");
 
                 bool? cleanupSucceded = (Storage as OsuStorage)?.Migrate(Host.GetStorage(path));