Merge pull request #13630 from smoogipoo/more-collection-safety

Make collection database even more safe

osu.Game/Collections/CollectionManager.cs

@@ -35,6 +35,7 @@ namespace osu.Game.Collections
         private const int database_version = 30000000;
 
         private const string database_name = "collection.db";
+        private const string database_backup_name = "collection.db.bak";
 
         public readonly BindableList<BeatmapCollection> Collections = new BindableList<BeatmapCollection>();
 
@@ -56,6 +57,17 @@ namespace osu.Game.Collections
         {
             Collections.CollectionChanged += collectionsChanged;
 
+            if (storage.Exists(database_backup_name))
+            {
+                // If a backup file exists, it means the previous write operation didn't run to completion.
+                // Always prefer the backup file in such a case as it's the most recent copy that is guaranteed to not be malformed.
+                //
+                // The database is saved 100ms after any change, and again when the game is closed, so there shouldn't be a large diff between the two files in the worst case.
+                if (storage.Exists(database_name))
+                    storage.Delete(database_name);
+                File.Copy(storage.GetFullPath(database_backup_name), storage.GetFullPath(database_name));
+            }
+
             if (storage.Exists(database_name))
             {
                 List<BeatmapCollection> beatmapCollections;
@@ -68,7 +80,7 @@ namespace osu.Game.Collections
             }
         }
 
-        private void collectionsChanged(object sender, NotifyCollectionChangedEventArgs e)
+        private void collectionsChanged(object sender, NotifyCollectionChangedEventArgs e) => Schedule(() =>
         {
             switch (e.Action)
             {
@@ -92,7 +104,7 @@ namespace osu.Game.Collections
             }
 
             backgroundSave();
-        }
+        });
 
         /// <summary>
         /// Set an endpoint for notifications to be posted to.
@@ -257,27 +269,50 @@ namespace osu.Game.Collections
             {
                 Interlocked.Increment(ref lastSave);
 
+                // This is NOT thread-safe!!
                 try
                 {
-                    // This is NOT thread-safe!!
+                    var tempPath = Path.GetTempFileName();
 
-                    using (var sw = new SerializationWriter(storage.GetStream(database_name, FileAccess.Write)))
+                    using (var ms = new MemoryStream())
                     {
-                        sw.Write(database_version);
+                        using (var sw = new SerializationWriter(ms, true))
-
-                        var collectionsCopy = Collections.ToArray();
-                        sw.Write(collectionsCopy.Length);
-
-                        foreach (var c in collectionsCopy)
                         {
-                            sw.Write(c.Name.Value);
+                            sw.Write(database_version);
 
-                            var beatmapsCopy = c.Beatmaps.ToArray();
+                            var collectionsCopy = Collections.ToArray();
-                            sw.Write(beatmapsCopy.Length);
+                            sw.Write(collectionsCopy.Length);
 
-                            foreach (var b in beatmapsCopy)
+                            foreach (var c in collectionsCopy)
-                                sw.Write(b.MD5Hash);
+                            {
+                                sw.Write(c.Name.Value);
+
+                                var beatmapsCopy = c.Beatmaps.ToArray();
+                                sw.Write(beatmapsCopy.Length);
+
+                                foreach (var b in beatmapsCopy)
+                                    sw.Write(b.MD5Hash);
+                            }
                         }
+
+                        using (var fs = File.OpenWrite(tempPath))
+                            ms.WriteTo(fs);
+
+                        var databasePath = storage.GetFullPath(database_name);
+                        var databaseBackupPath = storage.GetFullPath(database_backup_name);
+
+                        // Back up the existing database, clearing any existing backup.
+                        if (File.Exists(databaseBackupPath))
+                            File.Delete(databaseBackupPath);
+                        if (File.Exists(databasePath))
+                            File.Move(databasePath, databaseBackupPath);
+
+                        // Move the new database in-place of the existing one.
+                        File.Move(tempPath, databasePath);
+
+                        // If everything succeeded up to this point, remove the backup file.
+                        if (File.Exists(databaseBackupPath))
+                            File.Delete(databaseBackupPath);
                     }
 
                     if (saveFailures < 10)

osu.Game/IO/Legacy/SerializationWriter.cs

@@ -18,8 +18,8 @@ namespace osu.Game.IO.Legacy
     /// handle null strings and simplify use with ISerializable. </summary>
     public class SerializationWriter : BinaryWriter
     {
-        public SerializationWriter(Stream s)
+        public SerializationWriter(Stream s, bool leaveOpen = false)
-            : base(s, Encoding.UTF8)
+            : base(s, Encoding.UTF8, leaveOpen)
         {
         }