From 6f4c337a5696ef2e51c10511ceeed873cd0834b3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 24 Jan 2022 20:52:27 +0900 Subject: [PATCH 1/3] Fix a failed `BlockAllOperations` leaving update realm in unretrieved state If the operation timed out on.. ```csharp throw new TimeoutException(@"Took too long to acquire lock"); ``` ..from an update thread, it would not restore the update context. The next call would then fail on the assert that ensures a non-null context in such cases. Can add test coverage if required. --- osu.Game/Database/RealmAccess.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/osu.Game/Database/RealmAccess.cs b/osu.Game/Database/RealmAccess.cs index 66b4edbe84..64063664eb 100644 --- a/osu.Game/Database/RealmAccess.cs +++ b/osu.Game/Database/RealmAccess.cs @@ -595,8 +595,8 @@ namespace osu.Game.Database { if (updateRealm == null) { - // null realm means the update thread has not yet retrieved its instance. - // we don't need to worry about reviving the update instance in this case, so don't bother with the SynchronizationContext. + // null context means the update thread has not yet retrieved its context. + // we don't need to worry about reviving the update context in this case, so don't bother with the SynchronizationContext. Debug.Assert(!ThreadSafety.IsUpdateThread); } else @@ -639,18 +639,19 @@ namespace osu.Game.Database } catch { - realmCreationLock.Release(); + restoreOperation(); throw; } - return new InvokeOnDisposal(this, factory => - { - factory.realmCreationLock.Release(); - Logger.Log(@"Restoring realm operations.", LoggingTarget.Database); + return new InvokeOnDisposal(restoreOperation); + void restoreOperation() + { + Logger.Log(@"Restoring realm operations.", LoggingTarget.Database); + realmCreationLock.Release(); // Post back to the update thread to revive any subscriptions. syncContext?.Post(_ => ensureUpdateRealm(), null); - }); + } } // https://github.com/realm/realm-dotnet/blob/32f4ebcc88b3e80a3b254412665340cd9f3bd6b5/Realm/Realm/Extensions/ReflectionExtensions.cs#L46 From 8116806db3fa6b06e96d1f38eac77964a96435a2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 25 Jan 2022 14:00:58 +0900 Subject: [PATCH 2/3] Add test coverage of calling `BlockAllOperations` a second time after timeout --- osu.Game.Tests/Database/GeneralUsageTests.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game.Tests/Database/GeneralUsageTests.cs b/osu.Game.Tests/Database/GeneralUsageTests.cs index 2533c832e6..8262ef18d4 100644 --- a/osu.Game.Tests/Database/GeneralUsageTests.cs +++ b/osu.Game.Tests/Database/GeneralUsageTests.cs @@ -95,6 +95,11 @@ namespace osu.Game.Tests.Database }); stopThreadedUsage.Set(); + + // Ensure we can block a second time after the usage has ended. + using (realm.BlockAllOperations()) + { + } }); } } From 56d7d814658d1d202e405f237cf74d8a386d078b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 25 Jan 2022 14:47:21 +0900 Subject: [PATCH 3/3] Fix broken test due to `SynchronizationContext` never running expected work --- osu.Game.Tests/Database/GeneralUsageTests.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/osu.Game.Tests/Database/GeneralUsageTests.cs b/osu.Game.Tests/Database/GeneralUsageTests.cs index 8262ef18d4..dc0d42595b 100644 --- a/osu.Game.Tests/Database/GeneralUsageTests.cs +++ b/osu.Game.Tests/Database/GeneralUsageTests.cs @@ -87,6 +87,11 @@ namespace osu.Game.Tests.Database hasThreadedUsage.Wait(); + // Usually the host would run the synchronization context work per frame. + // For the sake of keeping this test simple (there's only one update invocation), + // let's replace it so we can ensure work is run immediately. + SynchronizationContext.SetSynchronizationContext(new ImmediateExecuteSynchronizationContext()); + Assert.Throws(() => { using (realm.BlockAllOperations()) @@ -102,5 +107,10 @@ namespace osu.Game.Tests.Database } }); } + + private class ImmediateExecuteSynchronizationContext : SynchronizationContext + { + public override void Post(SendOrPostCallback d, object? state) => d(state); + } } }