Fix realm refetch operations potentially being unsafe

As seen in test failure https://github.com/ppy/osu/runs/6357384721?check_suite_focus=true.
Lawtrohux · May 10, 2022 · 33f0242 · 33f0242
1 parent 87f6f74
commit 33f0242
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 9 deletions.
diff --git a/osu.Game/Database/RealmAccess.cs b/osu.Game/Database/RealmAccess.cs
@@ -344,6 +344,26 @@ public void Run(Action<Realm> action)
             }
         }
 
+        /// <summary>
+        /// Write changes to realm.
+        /// </summary>
+        /// <param name="action">The work to run.</param>
+        public T Write<T>(Func<Realm, T> action)
+        {
+            if (ThreadSafety.IsUpdateThread)
+            {
+                total_writes_update.Value++;
+                return Realm.Write(action);
+            }
+            else
+            {
+                total_writes_async.Value++;
+
+                using (var realm = getRealmInstance())
+                    return realm.Write(action);
+            }
+        }
+
         /// <summary>
         /// Write changes to realm.
         /// </summary>

diff --git a/osu.Game/Stores/RealmArchiveModelManager.cs b/osu.Game/Stores/RealmArchiveModelManager.cs
@@ -45,11 +45,16 @@ private void performFileOperation(TModel item, Action<TModel> operation)
             // This method should be removed as soon as all the surrounding pieces support non-detached operations.
             if (!item.IsManaged)
             {
-                var managed = Realm.Realm.Find<TModel>(item.ID);
-                managed.Realm.Write(() => operation(managed));
-
-                item.Files.Clear();
-                item.Files.AddRange(managed.Files.Detach());
+                // Importantly, begin the realm write *before* re-fetching, else the update realm may not be in a consistent state
+                // (ie. if an async import finished very recently).
+                Realm.Realm.Write(realm =>
+                {
+                    var managed = Realm.Realm.Find<TModel>(item.ID);
+                    operation(managed);
+
+                    item.Files.Clear();
+                    item.Files.AddRange(managed.Files.Detach());
+                });
             }
             else
                 operation(item);
@@ -165,30 +170,34 @@ public void Undelete(List<TModel> items, bool silent = false)
 
         public bool Delete(TModel item)
         {
-            return Realm.Run(realm =>
+            // Importantly, begin the realm write *before* re-fetching, else the update realm may not be in a consistent state
+            // (ie. if an async import finished very recently).
+            return Realm.Write(realm =>
             {
                 if (!item.IsManaged)
                     item = realm.Find<TModel>(item.ID);
 
                 if (item?.DeletePending != false)
                     return false;
 
-                realm.Write(r => item.DeletePending = true);
+                item.DeletePending = true;
                 return true;
             });
         }
 
         public void Undelete(TModel item)
         {
-            Realm.Run(realm =>
+            // Importantly, begin the realm write *before* re-fetching, else the update realm may not be in a consistent state
+            // (ie. if an async import finished very recently).
+            Realm.Write(realm =>
             {
                 if (!item.IsManaged)
                     item = realm.Find<TModel>(item.ID);
 
                 if (item?.DeletePending != true)
                     return;
 
-                realm.Write(r => item.DeletePending = false);
+                item.DeletePending = false;
             });
         }