From 9c0abae2b0836dd7cb9e3584be85ccf34ad03615 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 30 Sep 2021 23:59:26 +0900 Subject: [PATCH 1/5] Add failing test coverage of realm blocking behaviour --- osu.Game.Tests/Database/GeneralUsageTests.cs | 64 ++++++++++++++++++ osu.Game.Tests/Database/RealmTest.cs | 70 ++++++++++++++++++++ osu.Game.Tests/osu.Game.Tests.csproj | 1 + 3 files changed, 135 insertions(+) create mode 100644 osu.Game.Tests/Database/GeneralUsageTests.cs create mode 100644 osu.Game.Tests/Database/RealmTest.cs diff --git a/osu.Game.Tests/Database/GeneralUsageTests.cs b/osu.Game.Tests/Database/GeneralUsageTests.cs new file mode 100644 index 0000000000..245981cd9b --- /dev/null +++ b/osu.Game.Tests/Database/GeneralUsageTests.cs @@ -0,0 +1,64 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; + +#nullable enable + +namespace osu.Game.Tests.Database +{ + [TestFixture] + public class GeneralUsageTests : RealmTest + { + /// + /// Just test the construction of a new database works. + /// + [Test] + public void TestConstructRealm() + { + RunTestWithRealm((realmFactory, _) => { realmFactory.CreateContext().Refresh(); }); + } + + [Test] + public void TestBlockOperations() + { + RunTestWithRealm((realmFactory, _) => + { + using (realmFactory.BlockAllOperations()) + { + } + }); + } + + [Test] + public void TestBlockOperationsWithContention() + { + RunTestWithRealm((realmFactory, _) => + { + ManualResetEventSlim stopThreadedUsage = new ManualResetEventSlim(); + ManualResetEventSlim hasThreadedUsage = new ManualResetEventSlim(); + + Task.Factory.StartNew(() => + { + using (realmFactory.CreateContext()) + { + hasThreadedUsage.Set(); + + stopThreadedUsage.Wait(); + } + }, TaskCreationOptions.LongRunning | TaskCreationOptions.HideScheduler); + + hasThreadedUsage.Wait(); + + Assert.Throws(() => + { + using (realmFactory.BlockAllOperations()) + { + } + }); + + stopThreadedUsage.Set(); + }); + } + } +} diff --git a/osu.Game.Tests/Database/RealmTest.cs b/osu.Game.Tests/Database/RealmTest.cs new file mode 100644 index 0000000000..2f4838cb67 --- /dev/null +++ b/osu.Game.Tests/Database/RealmTest.cs @@ -0,0 +1,70 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; +using Nito.AsyncEx; +using NUnit.Framework; +using osu.Framework.Logging; +using osu.Framework.Platform; +using osu.Framework.Testing; +using osu.Game.Database; + +#nullable enable + +namespace osu.Game.Tests.Database +{ + [TestFixture] + public abstract class RealmTest + { + private static readonly TemporaryNativeStorage storage; + + static RealmTest() + { + storage = new TemporaryNativeStorage("realm-test"); + storage.DeleteDirectory(string.Empty); + } + + protected void RunTestWithRealm(Action testAction, [CallerMemberName] string caller = "") + { + AsyncContext.Run(() => + { + var testStorage = storage.GetStorageForDirectory(caller); + + using (var realmFactory = new RealmContextFactory(testStorage, caller)) + { + Logger.Log($"Running test using realm file {testStorage.GetFullPath(realmFactory.Filename)}"); + testAction(realmFactory, testStorage); + + realmFactory.Dispose(); + Logger.Log($"Final database size: {testStorage.GetStream(realmFactory.Filename)?.Length ?? 0}"); + + realmFactory.Compact(); + Logger.Log($"Final database size after compact: {testStorage.GetStream(realmFactory.Filename)?.Length ?? 0}"); + } + }); + } + + protected void RunTestWithRealmAsync(Func testAction, [CallerMemberName] string caller = "") + { + AsyncContext.Run(async () => + { + var testStorage = storage.GetStorageForDirectory(caller); + + using (var realmFactory = new RealmContextFactory(testStorage, caller)) + { + Logger.Log($"Running test using realm file {testStorage.GetFullPath(realmFactory.Filename)}"); + + await testAction(realmFactory, testStorage); + + realmFactory.Dispose(); + Logger.Log($"Final database size: {testStorage.GetStream(realmFactory.Filename)?.Length ?? 0}"); + + realmFactory.Compact(); + Logger.Log($"Final database size after compact: {testStorage.GetStream(realmFactory.Filename)?.Length ?? 0}"); + } + }); + } + } +} diff --git a/osu.Game.Tests/osu.Game.Tests.csproj b/osu.Game.Tests/osu.Game.Tests.csproj index 696f930467..cd56cb51ae 100644 --- a/osu.Game.Tests/osu.Game.Tests.csproj +++ b/osu.Game.Tests/osu.Game.Tests.csproj @@ -4,6 +4,7 @@ + From cfd3bdf888fc24df4bc8eeb0f8def24471352bbb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 1 Oct 2021 01:32:28 +0900 Subject: [PATCH 2/5] Ensure realm blocks until all threaded usages are completed --- osu.Game/Database/RealmContextFactory.cs | 39 +++++++++++++++++++----- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index c51ac095bb..e3b0764721 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/osu.Game/Database/RealmContextFactory.cs @@ -133,20 +133,43 @@ namespace osu.Game.Database if (IsDisposed) throw new ObjectDisposedException(nameof(RealmContextFactory)); + // TODO: this can be added for safety once we figure how to bypass in test + // if (!ThreadSafety.IsUpdateThread) + // throw new InvalidOperationException($"{nameof(BlockAllOperations)} must be called from the update thread."); + Logger.Log(@"Blocking realm operations.", LoggingTarget.Database); - contextCreationLock.Wait(); + try + { + contextCreationLock.Wait(); - context?.Dispose(); - context = null; + const int sleep_length = 200; + int timeout = 5000; - return new InvokeOnDisposal(this, endBlockingSection); + context?.Dispose(); + context = null; - static void endBlockingSection(RealmContextFactory factory) + // see https://github.com/realm/realm-dotnet/discussions/2657 + while (!Compact()) + { + Thread.Sleep(sleep_length); + timeout -= sleep_length; + + if (timeout < 0) + throw new TimeoutException("Took too long to acquire lock"); + } + } + catch + { + contextCreationLock.Release(); + throw; + } + + return new InvokeOnDisposal(this, factory => { factory.contextCreationLock.Release(); Logger.Log(@"Restoring realm operations.", LoggingTarget.Database); - } + }); } protected override void Dispose(bool isDisposing) @@ -155,8 +178,8 @@ namespace osu.Game.Database if (!IsDisposed) { - // intentionally block all operations indefinitely. this ensures that nothing can start consuming a new context after disposal. - BlockAllOperations(); + // intentionally block context creation indefinitely. this ensures that nothing can start consuming a new context after disposal. + contextCreationLock.Wait(); contextCreationLock.Dispose(); } From b5345235cae7edb8430f31e97139f1bbe016ee95 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 1 Oct 2021 10:40:55 +0900 Subject: [PATCH 3/5] Handle window file access errors --- osu.Game.Tests/Database/RealmTest.cs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Database/RealmTest.cs b/osu.Game.Tests/Database/RealmTest.cs index 2f4838cb67..b7658d6408 100644 --- a/osu.Game.Tests/Database/RealmTest.cs +++ b/osu.Game.Tests/Database/RealmTest.cs @@ -38,10 +38,26 @@ namespace osu.Game.Tests.Database testAction(realmFactory, testStorage); realmFactory.Dispose(); - Logger.Log($"Final database size: {testStorage.GetStream(realmFactory.Filename)?.Length ?? 0}"); + + try + { + Logger.Log($"Final database size: {testStorage.GetStream(realmFactory.Filename)?.Length ?? 0}"); + } + catch + { + // windows runs may error due to file still being open. + } realmFactory.Compact(); - Logger.Log($"Final database size after compact: {testStorage.GetStream(realmFactory.Filename)?.Length ?? 0}"); + + try + { + Logger.Log($"Final database size after compact: {testStorage.GetStream(realmFactory.Filename)?.Length ?? 0}"); + } + catch + { + // windows runs may error due to file still being open. + } } }); } From 6ec2223b5c231fc747b31e3d9f87ba810d2e376b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 2 Oct 2021 23:01:44 +0900 Subject: [PATCH 4/5] Catch potential file access exceptions also in async flow --- osu.Game.Tests/Database/RealmTest.cs | 38 +++++++++++++--------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/osu.Game.Tests/Database/RealmTest.cs b/osu.Game.Tests/Database/RealmTest.cs index b7658d6408..219690db30 100644 --- a/osu.Game.Tests/Database/RealmTest.cs +++ b/osu.Game.Tests/Database/RealmTest.cs @@ -39,25 +39,9 @@ namespace osu.Game.Tests.Database realmFactory.Dispose(); - try - { - Logger.Log($"Final database size: {testStorage.GetStream(realmFactory.Filename)?.Length ?? 0}"); - } - catch - { - // windows runs may error due to file still being open. - } - + Logger.Log($"Final database size: {getFileSize(testStorage, realmFactory)}"); realmFactory.Compact(); - - try - { - Logger.Log($"Final database size after compact: {testStorage.GetStream(realmFactory.Filename)?.Length ?? 0}"); - } - catch - { - // windows runs may error due to file still being open. - } + Logger.Log($"Final database size after compact: {getFileSize(testStorage, realmFactory)}"); } }); } @@ -71,16 +55,28 @@ namespace osu.Game.Tests.Database using (var realmFactory = new RealmContextFactory(testStorage, caller)) { Logger.Log($"Running test using realm file {testStorage.GetFullPath(realmFactory.Filename)}"); - await testAction(realmFactory, testStorage); realmFactory.Dispose(); - Logger.Log($"Final database size: {testStorage.GetStream(realmFactory.Filename)?.Length ?? 0}"); + Logger.Log($"Final database size: {getFileSize(testStorage, realmFactory)}"); realmFactory.Compact(); - Logger.Log($"Final database size after compact: {testStorage.GetStream(realmFactory.Filename)?.Length ?? 0}"); + Logger.Log($"Final database size after compact: {getFileSize(testStorage, realmFactory)}"); } }); } + + private static long getFileSize(Storage testStorage, RealmContextFactory realmFactory) + { + try + { + return testStorage.GetStream(realmFactory.Filename)?.Length ?? 0; + } + catch + { + // windows runs may error due to file still being open. + return 0; + } + } } } From 537b29654e60e738128fa123345a01ab39074b22 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 4 Oct 2021 14:30:22 +0900 Subject: [PATCH 5/5] Fix stream being held open causing windows CI failures --- osu.Game.Tests/Database/RealmTest.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Database/RealmTest.cs b/osu.Game.Tests/Database/RealmTest.cs index 219690db30..576f901c1a 100644 --- a/osu.Game.Tests/Database/RealmTest.cs +++ b/osu.Game.Tests/Database/RealmTest.cs @@ -70,7 +70,8 @@ namespace osu.Game.Tests.Database { try { - return testStorage.GetStream(realmFactory.Filename)?.Length ?? 0; + using (var stream = testStorage.GetStream(realmFactory.Filename)) + return stream?.Length ?? 0; } catch {