Merge pull request #14909 from peppy/realm-context-factory-safer-blocking

Ensure realm blocks until all threaded usages are completed

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
+    {
+        /// <summary>
+        /// Just test the construction of a new database works.
+        /// </summary>
+        [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<TimeoutException>(() =>
+                {
+                    using (realmFactory.BlockAllOperations())
+                    {
+                    }
+                });
+
+                stopThreadedUsage.Set();
+            });
+        }
+    }
+}

osu.Game.Tests/Database/RealmTest.cs

@@ -0,0 +1,83 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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<RealmContextFactory, Storage> 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: {getFileSize(testStorage, realmFactory)}");
+                    realmFactory.Compact();
+                    Logger.Log($"Final database size after compact: {getFileSize(testStorage, realmFactory)}");
+                }
+            });
+        }
+
+        protected void RunTestWithRealmAsync(Func<RealmContextFactory, Storage, Task> 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: {getFileSize(testStorage, realmFactory)}");
+                    realmFactory.Compact();
+                    Logger.Log($"Final database size after compact: {getFileSize(testStorage, realmFactory)}");
+                }
+            });
+        }
+
+        private static long getFileSize(Storage testStorage, RealmContextFactory realmFactory)
+        {
+            try
+            {
+                using (var stream = testStorage.GetStream(realmFactory.Filename))
+                    return stream?.Length ?? 0;
+            }
+            catch
+            {
+                // windows runs may error due to file still being open.
+                return 0;
+            }
+        }
+    }
+}

osu.Game.Tests/osu.Game.Tests.csproj

@@ -4,6 +4,7 @@
     <PackageReference Include="Appveyor.TestLogger" Version="2.0.0" />
     <PackageReference Include="DeepEqual" Version="2.0.0" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.11.0" />
+    <PackageReference Include="Nito.AsyncEx" Version="5.1.2" />
     <PackageReference Include="NUnit" Version="3.13.2" />
     <PackageReference Include="NUnit3TestAdapter" Version="4.0.0" />
     <PackageReference Update="Microsoft.EntityFrameworkCore.Sqlite" Version="2.1.4" />

osu.Game/Database/RealmContextFactory.cs

@@ -135,23 +135,46 @@ 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();
-
-            lock (contextLock)
+            try
             {
-                context?.Dispose();
-                context = null;
+                contextCreationLock.Wait();
+
+                lock (contextLock)
+                {
+                    context?.Dispose();
+                    context = null;
+                }
+
+                const int sleep_length = 200;
+                int timeout = 5000;
+
+                // 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<RealmContextFactory>(this, endBlockingSection);
-
-            static void endBlockingSection(RealmContextFactory factory)
+            return new InvokeOnDisposal<RealmContextFactory>(this, factory =>
             {
                 factory.contextCreationLock.Release();
                 Logger.Log(@"Restoring realm operations.", LoggingTarget.Database);
-            }
+            });
         }
 
         protected override void Dispose(bool isDisposing)
@@ -163,8 +186,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();
             }