From ede71b117b79797de6ca6f8db600d977caa12289 Mon Sep 17 00:00:00 2001 From: Ivan Murashka Date: Tue, 11 Aug 2026 13:42:52 +0200 Subject: [PATCH] refactor: drop the storage finalizer BinaryStorage holds no unmanaged resources, so the finalizer had nothing to release. It skipped the save, disposed collections that were already unreachable, and released the editor path lock by mutating a static HashSet from the finalizer thread while the main thread could be reading it from Construct. With the finalizer gone, disposing is the only path, so Dispose no longer needs the disposing flag or the two branches it guarded. --- CHANGELOG.md | 1 + src/Runtime/BinaryStorage.cs | 43 +++++++++--------------------------- 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cc0aa5..62ebecd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed - `IsDirty`. With auto-save on it was almost always `false`, and with the background writer it meant "no change is waiting to be handed over", not "everything is on disk". Use `Save()` when you need the data on disk. +- The `BinaryStorage` finalizer. It held no unmanaged resources to release, it never saved anything, and the only thing it did was release the editor-side path lock from the finalizer thread while the main thread could be reading the same set. A storage that is never disposed now keeps that lock until the domain reloads, so dispose your storages, as before. ### Fixed - Two storages opened through paths that differ only in form (`a/save.dat` and `./a/save.dat`) no longer publish the same file without serializing against each other. diff --git a/src/Runtime/BinaryStorage.cs b/src/Runtime/BinaryStorage.cs index de8a416..a25690d 100644 --- a/src/Runtime/BinaryStorage.cs +++ b/src/Runtime/BinaryStorage.cs @@ -653,43 +653,25 @@ private static void ThrowIfCollection([CallerMemberName] string action = null #endregion - #region Dispose Pattern + #region Dispose - /// Finalizer - ~BinaryStorage() - { - Dispose(false); - } - - /// Disposes the resources used by the storage. + /// Writes any unsaved data to disk, disposes the stored collections and releases the storage. public virtual void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } - - /// Disposes the resources used by the storage. - /// Whether managed resources should be disposed. - private void Dispose(bool disposing) { if (IsDisposed) { return; } - if (disposing) + if (AutoSave && _hasUnsavedChanges) { - if (AutoSave && _hasUnsavedChanges) - { - SaveDataOnDisk(true); - } - else - { - _persistence.Flush(); - } + SaveDataOnDisk(true); + } + else + { + _persistence.Flush(); } - // Always dispose IReactiveCollection instances foreach (var record in _data.Values) { UntrackCollectionOf(record); @@ -699,13 +681,10 @@ private void Dispose(bool disposing) OnKeyChanged = null; OnKeyRemoved = null; - if (disposing) + _data.Clear(); + for (var i = 0; i < _supportedTypes.Count; i++) { - _data.Clear(); - for (var i = 0; i < _supportedTypes.Count; i++) - { - _supportedTypes[i].Count = 0; - } + _supportedTypes[i].Count = 0; } IsDisposed = true;