Merge pull request #13618 from peppy/fix-realm-state-change-crashes

Fix realm threading issues to make it releaseable

osu.Game/Database/RealmContextFactory.cs

@@ -26,6 +26,11 @@ public class RealmContextFactory : Component, IRealmFactory
         /// </summary>
         private readonly object writeLock = new object();
 
+        /// <summary>
+        /// Lock object which is held during <see cref="BlockAllOperations"/> sections.
+        /// </summary>
+        private readonly SemaphoreSlim blockingLock = new SemaphoreSlim(1);
+
         private static readonly GlobalStatistic<int> reads = GlobalStatistics.Get<int>("Realm", "Get (Read)");
         private static readonly GlobalStatistic<int> writes = GlobalStatistics.Get<int>("Realm", "Get (Write)");
         private static readonly GlobalStatistic<int> refreshes = GlobalStatistics.Get<int>("Realm", "Dirty Refreshes");
@@ -33,17 +38,12 @@ public class RealmContextFactory : Component, IRealmFactory
         private static readonly GlobalStatistic<int> pending_writes = GlobalStatistics.Get<int>("Realm", "Pending writes");
         private static readonly GlobalStatistic<int> active_usages = GlobalStatistics.Get<int>("Realm", "Active usages");
 
-        private readonly ManualResetEventSlim blockingResetEvent = new ManualResetEventSlim(true);
-
         private Realm context;
 
         public Realm Context
         {
             get
             {
-                if (IsDisposed)
-                    throw new InvalidOperationException($"Attempted to access {nameof(Context)} on a disposed context factory");
-
                 if (context == null)
                 {
                     context = createContext();
@@ -64,7 +64,7 @@ public RealmContextFactory(Storage storage)
         public RealmUsage GetForRead()
         {
             reads.Value++;
-            return new RealmUsage(this);
+            return new RealmUsage(createContext());
         }
 
         public RealmWriteUsage GetForWrite()
@@ -73,8 +73,28 @@ public RealmWriteUsage GetForWrite()
             pending_writes.Value++;
 
             Monitor.Enter(writeLock);
+            return new RealmWriteUsage(createContext(), writeComplete);
+        }
 
-            return new RealmWriteUsage(this);
+        /// <summary>
+        /// Flush any active contexts and block any further writes.
+        /// </summary>
+        /// <remarks>
+        /// This should be used in places we need to ensure no ongoing reads/writes are occurring with realm.
+        /// ie. to move the realm backing file to a new location.
+        /// </remarks>
+        /// <returns>An <see cref="IDisposable"/> which should be disposed to end the blocking section.</returns>
+        public IDisposable BlockAllOperations()
+        {
+            if (IsDisposed)
+                throw new ObjectDisposedException(nameof(RealmContextFactory));
+
+            blockingLock.Wait();
+            flushContexts();
+
+            return new InvokeOnDisposal<RealmContextFactory>(this, endBlockingSection);
+
+            static void endBlockingSection(RealmContextFactory factory) => factory.blockingLock.Release();
         }
 
         protected override void Update()
@@ -87,15 +107,31 @@ protected override void Update()
 
         private Realm createContext()
         {
-            blockingResetEvent.Wait();
-
-            contexts_created.Value++;
-
-            return Realm.GetInstance(new RealmConfiguration(storage.GetFullPath($"{database_name}.realm", true))
+            try
             {
-                SchemaVersion = schema_version,
-                MigrationCallback = onMigration,
-            });
+                if (IsDisposed)
+                    throw new ObjectDisposedException(nameof(RealmContextFactory));
+
+                blockingLock.Wait();
+
+                contexts_created.Value++;
+
+                return Realm.GetInstance(new RealmConfiguration(storage.GetFullPath($"{database_name}.realm", true))
+                {
+                    SchemaVersion = schema_version,
+                    MigrationCallback = onMigration,
+                });
+            }
+            finally
+            {
+                blockingLock.Release();
+            }
+        }
+
+        private void writeComplete()
+        {
+            Monitor.Exit(writeLock);
+            pending_writes.Value--;
         }
 
         private void onMigration(Migration migration, ulong lastSchemaVersion)
@@ -109,26 +145,6 @@ private void onMigration(Migration migration, ulong lastSchemaVersion)
             }
         }
 
-        protected override void Dispose(bool isDisposing)
-        {
-            base.Dispose(isDisposing);
-
-            BlockAllOperations();
-        }
-
-        public IDisposable BlockAllOperations()
-        {
-            blockingResetEvent.Reset();
-            flushContexts();
-
-            return new InvokeOnDisposal<RealmContextFactory>(this, r => endBlockingSection());
-        }
-
-        private void endBlockingSection()
-        {
-            blockingResetEvent.Set();
-        }
-
         private void flushContexts()
         {
             var previousContext = context;
@@ -141,6 +157,18 @@ private void flushContexts()
             previousContext?.Dispose();
         }
 
+        protected override void Dispose(bool isDisposing)
+        {
+            if (!IsDisposed)
+            {
+                // intentionally block all operations indefinitely. this ensures that nothing can start consuming a new context after disposal.
+                BlockAllOperations();
+                blockingLock?.Dispose();
+            }
+
+            base.Dispose(isDisposing);
+        }
+
         /// <summary>
         /// A usage of realm from an arbitrary thread.
         /// </summary>
@@ -148,13 +176,10 @@ public class RealmUsage : IDisposable
         {
             public readonly Realm Realm;
 
-            protected readonly RealmContextFactory Factory;
-
-            internal RealmUsage(RealmContextFactory factory)
+            internal RealmUsage(Realm context)
             {
                 active_usages.Value++;
-                Factory = factory;
-                Realm = factory.createContext();
+                Realm = context;
             }
 
             /// <summary>
@@ -172,11 +197,13 @@ public virtual void Dispose()
         /// </summary>
         public class RealmWriteUsage : RealmUsage
         {
+            private readonly Action onWriteComplete;
             private readonly Transaction transaction;
 
-            internal RealmWriteUsage(RealmContextFactory factory)
-                : base(factory)
+            internal RealmWriteUsage(Realm context, Action onWriteComplete)
+                : base(context)
             {
+                this.onWriteComplete = onWriteComplete;
                 transaction = Realm.BeginWrite();
             }
 
@@ -200,8 +227,7 @@ public override void Dispose()
 
                 base.Dispose();
 
-                Monitor.Exit(Factory.writeLock);
-                pending_writes.Value--;
+                onWriteComplete();
             }
         }
     }

osu.Game/OsuGameBase.cs

@@ -24,6 +24,7 @@
 using osu.Framework.Graphics.Textures;
 using osu.Framework.Input;
 using osu.Framework.Logging;
+using osu.Framework.Threading;
 using osu.Game.Audio;
 using osu.Game.Database;
 using osu.Game.Input;
@@ -156,6 +157,8 @@ public virtual string Version
 
         private readonly BindableNumber<double> globalTrackVolumeAdjust = new BindableNumber<double>(GLOBAL_TRACK_VOLUME_ADJUST);
 
+        private IBindable<GameThreadState> updateThreadState;
+
         public OsuGameBase()
         {
             UseDevelopmentServer = DebugUtils.IsDebugBuild;
@@ -182,6 +185,10 @@ private void load()
             dependencies.Cache(contextFactory = new DatabaseContextFactory(Storage));
 
             dependencies.Cache(realmFactory = new RealmContextFactory(Storage));
+
+            updateThreadState = Host.UpdateThread.State.GetBoundCopy();
+            updateThreadState.BindValueChanged(updateThreadStateChanged);
+
             AddInternal(realmFactory);
 
             dependencies.CacheAs(Storage);
@@ -356,6 +363,23 @@ List<ScoreInfo> getBeatmapScores(BeatmapSetInfo set)
             Ruleset.BindValueChanged(onRulesetChanged);
         }
 
+        private IDisposable blocking;
+
+        private void updateThreadStateChanged(ValueChangedEvent<GameThreadState> state)
+        {
+            switch (state.NewValue)
+            {
+                case GameThreadState.Running:
+                    blocking?.Dispose();
+                    blocking = null;
+                    break;
+
+                case GameThreadState.Paused:
+                    blocking = realmFactory.BlockAllOperations();
+                    break;
+            }
+        }
+
         protected override void LoadComplete()
         {
             base.LoadComplete();

osu.Game/Overlays/Toolbar/ToolbarButton.cs

@@ -159,28 +159,6 @@ protected ToolbarButton()
             };
         }
 
-        private RealmKeyBinding realmKeyBinding;
-
-        protected override void LoadComplete()
-        {
-            base.LoadComplete();
-
-            if (Hotkey == null) return;
-
-            realmKeyBinding = realmFactory.Context.All<RealmKeyBinding>().FirstOrDefault(rkb => rkb.RulesetID == null && rkb.ActionInt == (int)Hotkey.Value);
-
-            if (realmKeyBinding != null)
-            {
-                realmKeyBinding.PropertyChanged += (sender, args) =>
-                {
-                    if (args.PropertyName == nameof(realmKeyBinding.KeyCombinationString))
-                        updateKeyBindingTooltip();
-                };
-            }
-
-            updateKeyBindingTooltip();
-        }
-
         protected override bool OnMouseDown(MouseDownEvent e) => true;
 
         protected override bool OnClick(ClickEvent e)
@@ -196,6 +174,7 @@ protected override bool OnHover(HoverEvent e)
 
             HoverBackground.FadeIn(200);
             tooltipContainer.FadeIn(100);
+
             return base.OnHover(e);
         }
 
@@ -222,6 +201,10 @@ public void OnReleased(GlobalAction action)
 
         private void updateKeyBindingTooltip()
         {
+            if (Hotkey == null) return;
+
+            var realmKeyBinding = realmFactory.Context.All<RealmKeyBinding>().FirstOrDefault(rkb => rkb.RulesetID == null && rkb.ActionInt == (int)Hotkey.Value);
+
             if (realmKeyBinding != null)
             {
                 var keyBindingString = realmKeyBinding.KeyCombination.ReadableString();