diff --git a/src/BLite.Core/DocumentDbContext.cs b/src/BLite.Core/DocumentDbContext.cs index 04fd579..8d5a3d0 100644 --- a/src/BLite.Core/DocumentDbContext.cs +++ b/src/BLite.Core/DocumentDbContext.cs @@ -86,17 +86,28 @@ protected DocumentDbContext(string databasePath, PageFileConfig config, BLiteKvO _databasePath = databasePath; _storage = new StorageEngine(databasePath, config); _cdc = new CDC.ChangeStreamDispatcher(); - _storage.RegisterCdc(_cdc); _freeSpaceIndexes = new FreeSpaceIndexProvider(_storage); _kvStore = new BLiteKvStore(_storage, kvOptions); - // Initialize model before collections - var modelBuilder = new ModelBuilder(); - OnModelCreating(modelBuilder); - _model = modelBuilder.GetEntityBuilders(); - InitializeCollections(); - DropOrphanCollections(); - RunGdprStrictValidation(kvOptions); + try + { + // Initialize model before collections + var modelBuilder = new ModelBuilder(); + OnModelCreating(modelBuilder); + _model = modelBuilder.GetEntityBuilders(); + InitializeCollections(); + DropOrphanCollections(); + RunGdprStrictValidation(kvOptions); + } + catch + { + try { _storage.Dispose(); } catch { /* best-effort; preserve original exception */ } + try { _cdc.Dispose(); } catch { /* best-effort */ } + _disposed = true; + throw; + } + + _storage.RegisterCdc(_cdc); } /// @@ -148,16 +159,28 @@ protected DocumentDbContext(string databasePath, CryptoOptions crypto, BLiteKvOp _storage = new StorageEngine(databasePath, config); _cdc = new CDC.ChangeStreamDispatcher(); - _storage.RegisterCdc(_cdc); _freeSpaceIndexes = new FreeSpaceIndexProvider(_storage); _kvStore = new BLiteKvStore(_storage, kvOptions); - var modelBuilder = new ModelBuilder(); - OnModelCreating(modelBuilder); - _model = modelBuilder.GetEntityBuilders(); - InitializeCollections(); - DropOrphanCollections(); - RunGdprStrictValidation(kvOptions); + try + { + var modelBuilder = new ModelBuilder(); + OnModelCreating(modelBuilder); + _model = modelBuilder.GetEntityBuilders(); + InitializeCollections(); + DropOrphanCollections(); + RunGdprStrictValidation(kvOptions); + } + catch + { + try { _storage.Dispose(); } catch { /* best-effort; preserve original exception */ } + try { _cdc.Dispose(); } catch { /* best-effort */ } + try { _ownedCoordinator?.Dispose(); } catch { /* best-effort */ } + _disposed = true; + throw; + } + + _storage.RegisterCdc(_cdc); } /// @@ -196,16 +219,27 @@ protected DocumentDbContext(StorageEngine storage, BLiteKvOptions? kvOptions = n { _storage = storage ?? throw new ArgumentNullException(nameof(storage)); _cdc = new CDC.ChangeStreamDispatcher(); - _storage.RegisterCdc(_cdc); _freeSpaceIndexes = new FreeSpaceIndexProvider(_storage); _kvStore = new BLiteKvStore(_storage, kvOptions); - var modelBuilder = new ModelBuilder(); - OnModelCreating(modelBuilder); - _model = modelBuilder.GetEntityBuilders(); - InitializeCollections(); - DropOrphanCollections(); - RunGdprStrictValidation(kvOptions); + try + { + var modelBuilder = new ModelBuilder(); + OnModelCreating(modelBuilder); + _model = modelBuilder.GetEntityBuilders(); + InitializeCollections(); + DropOrphanCollections(); + RunGdprStrictValidation(kvOptions); + } + catch + { + // _storage is caller-owned; do not dispose it here. + _cdc.Dispose(); + _disposed = true; + throw; + } + + _storage.RegisterCdc(_cdc); } /// diff --git a/src/BLite.Core/Storage/StorageEngine.cs b/src/BLite.Core/Storage/StorageEngine.cs index fa5ba5d..5c5eb99 100644 --- a/src/BLite.Core/Storage/StorageEngine.cs +++ b/src/BLite.Core/Storage/StorageEngine.cs @@ -1,5 +1,6 @@ using System.Collections.Concurrent; using System.Threading.Channels; +using BLite.Core.Encryption; using BLite.Core.Transactions; namespace BLite.Core.Storage; @@ -117,105 +118,123 @@ public sealed partial class StorageEngine : IDisposable public StorageEngine(string databasePath, PageFileConfig config) { _config = config; + ICryptoProvider? walCryptoProvider = null; + try + { + // Use WalPath if specified, otherwise derive from databasePath (default behavior unchanged) + var walPath = config.WalPath ?? Path.ChangeExtension(databasePath, ".wal"); - // Use WalPath if specified, otherwise derive from databasePath (default behavior unchanged) - var walPath = config.WalPath ?? Path.ChangeExtension(databasePath, ".wal"); - - // Ensure WAL parent directory exists - var walDirectory = Path.GetDirectoryName(walPath); - if (!string.IsNullOrWhiteSpace(walDirectory)) - Directory.CreateDirectory(walDirectory); + // Ensure WAL parent directory exists + var walDirectory = Path.GetDirectoryName(walPath); + if (!string.IsNullOrWhiteSpace(walDirectory)) + Directory.CreateDirectory(walDirectory); - // Initialize storage infrastructure - _pageFile = new PageFile(databasePath, config); + // Initialize storage infrastructure + _pageFile = new PageFile(databasePath, config); - _pageFile.Open(); - // NOTE: If CryptoProvider is a coordinator-managed provider, opening the main file - // primes the coordinator's database salt, making CreateSiblingProvider available - // for index, WAL, and per-collection files. + _pageFile.Open(); + // NOTE: If CryptoProvider is a coordinator-managed provider, opening the main file + // primes the coordinator's database salt, making CreateSiblingProvider available + // for index, WAL, and per-collection files. - // Phase 3: open separate index file if configured - if (config.IndexFilePath != null) - { - var idxDirectory = Path.GetDirectoryName(config.IndexFilePath); - if (!string.IsNullOrWhiteSpace(idxDirectory)) - Directory.CreateDirectory(idxDirectory); - - // Derive a dedicated index-file provider so that index and data pages - // encrypted with the same pageId never share a key. - var idxConfig = config.CryptoProvider != null - ? AsStandaloneConfig(config) with { CryptoProvider = config.CryptoProvider.CreateSiblingProvider(2, 0) } - : AsStandaloneConfig(config); - - _indexFile = new PageFile(config.IndexFilePath, idxConfig); - _indexFile.Open(); - } + // Phase 3: open separate index file if configured + if (config.IndexFilePath != null) + { + var idxDirectory = Path.GetDirectoryName(config.IndexFilePath); + if (!string.IsNullOrWhiteSpace(idxDirectory)) + Directory.CreateDirectory(idxDirectory); + + // Derive a dedicated index-file provider so that index and data pages + // encrypted with the same pageId never share a key. + var idxConfig = config.CryptoProvider != null + ? AsStandaloneConfig(config) with { CryptoProvider = config.CryptoProvider.CreateSiblingProvider(2, 0) } + : AsStandaloneConfig(config); + + _indexFile = new PageFile(config.IndexFilePath, idxConfig); + _indexFile.Open(); + } - // Phase 4: initialize collection-per-file structures if configured - if (config.CollectionDataDirectory != null) - { - Directory.CreateDirectory(config.CollectionDataDirectory); - _collectionFiles = new ConcurrentDictionary>(StringComparer.OrdinalIgnoreCase); - _collectionNameToSlot = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); - _collectionSlotToName = new Dictionary(); - _slotsFilePath = Path.Combine(config.CollectionDataDirectory, ".slots"); - LoadCollectionSlots(); - } + // Phase 4: initialize collection-per-file structures if configured + if (config.CollectionDataDirectory != null) + { + Directory.CreateDirectory(config.CollectionDataDirectory); + _collectionFiles = new ConcurrentDictionary>(StringComparer.OrdinalIgnoreCase); + _collectionNameToSlot = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); + _collectionSlotToName = new Dictionary(); + _slotsFilePath = Path.Combine(config.CollectionDataDirectory, ".slots"); + LoadCollectionSlots(); + } - // When encryption is configured, derive a dedicated WAL provider from the main-file - // provider so that WAL records are always encrypted alongside the rest of the database. - var walCryptoProvider = config.CryptoProvider?.CreateSiblingProvider(3, 0); + // When encryption is configured, derive a dedicated WAL provider from the main-file + // provider so that WAL records are always encrypted alongside the rest of the database. + walCryptoProvider = config.CryptoProvider?.CreateSiblingProvider(3, 0); + + _wal = new WriteAheadLog(walPath, walCryptoProvider, config.LockTimeout.WriteTimeoutMs, config.AllowMultiProcessAccess); + walCryptoProvider = null; + _walCache = new ConcurrentDictionary>(); + _walIndex = new ConcurrentDictionary(); + _activeTransactions = new ConcurrentDictionary(); + _nextTransactionId = 0; // Interlocked.Increment pre-increments, so first txnId == 1. + + // ── Multi-process WAL coordination (opt-in) ───────────────────────── + // When AllowMultiProcessAccess is true, open the .wal-shm sidecar that + // coordinates cross-process writers / readers (see roadmap/v5/MULTI_PROCESS_WAL.md). + // The SHM file lives next to the WAL so both have the same lifetime. + if (config.AllowMultiProcessAccess) + { + var shmPath = walPath + "-shm"; + _shm = Transactions.WalSharedMemory.Open(shmPath, config.PageSize); + _walOffsets = new ConcurrentDictionary(); + + // If a previous writer crashed without releasing the writer lock, the + // recorded PID will not be alive — clear it so this process (or another) + // can re-acquire the lock. The OS-level mutex / OFD lock has already been + // auto-released by the kernel; this only clears our PID stamp. + _shm.ForceClearStaleWriter(); + } - _wal = new WriteAheadLog(walPath, walCryptoProvider, config.LockTimeout.WriteTimeoutMs, config.AllowMultiProcessAccess); - _walCache = new ConcurrentDictionary>(); - _walIndex = new ConcurrentDictionary(); - _activeTransactions = new ConcurrentDictionary(); - _nextTransactionId = 0; // Interlocked.Increment pre-increments, so first txnId == 1. + // Admission gate: limits concurrent commit pressure on WAL/commit locks. + _writerGate = config.LockTimeout.MaxConcurrentWriters > 0 + ? new SemaphoreSlim(config.LockTimeout.MaxConcurrentWriters, config.LockTimeout.MaxConcurrentWriters) + : null; - // ── Multi-process WAL coordination (opt-in) ───────────────────────── - // When AllowMultiProcessAccess is true, open the .wal-shm sidecar that - // coordinates cross-process writers / readers (see roadmap/v5/MULTI_PROCESS_WAL.md). - // The SHM file lives next to the WAL so both have the same lifetime. - if (config.AllowMultiProcessAccess) - { - var shmPath = walPath + "-shm"; - _shm = Transactions.WalSharedMemory.Open(shmPath, config.PageSize); - _walOffsets = new ConcurrentDictionary(); - - // If a previous writer crashed without releasing the writer lock, the - // recorded PID will not be alive — clear it so this process (or another) - // can re-acquire the lock. The OS-level mutex / OFD lock has already been - // auto-released by the kernel; this only clears our PID stamp. - _shm.ForceClearStaleWriter(); + // Start the group commit writer. + _commitChannel = Channel.CreateBounded(new BoundedChannelOptions(4096) + { + FullMode = BoundedChannelFullMode.Wait, + SingleReader = true, + SingleWriter = false + }); + _writerTask = Task.Run(() => GroupCommitWriterAsync(_writerCts.Token)); + + // RecoverAsync from WAL if exists (crash recovery or resume after close) + // This replays any committed transactions not yet checkpointed + if (_wal.GetCurrentSize() > 0) + { + RecoverAsync().GetAwaiter().GetResult(); + } + + InitializeDictionary(); + InitializeKv(); + + // Create and start checkpoint manager + // _checkpointManager = new Transactions.CheckpointManager(this); + // _checkpointManager.StartAutoCheckpoint(); } - - // Admission gate: limits concurrent commit pressure on WAL/commit locks. - _writerGate = config.LockTimeout.MaxConcurrentWriters > 0 - ? new SemaphoreSlim(config.LockTimeout.MaxConcurrentWriters, config.LockTimeout.MaxConcurrentWriters) - : null; - - // Start the group commit writer. - _commitChannel = Channel.CreateBounded(new BoundedChannelOptions(4096) + catch { - FullMode = BoundedChannelFullMode.Wait, - SingleReader = true, - SingleWriter = false - }); - _writerTask = Task.Run(() => GroupCommitWriterAsync(_writerCts.Token)); - - // RecoverAsync from WAL if exists (crash recovery or resume after close) - // This replays any committed transactions not yet checkpointed - if (_wal.GetCurrentSize() > 0) - { - RecoverAsync().GetAwaiter().GetResult(); + try { _writerCts.Cancel(); } catch { /* best-effort */ } + try { _writerTask?.Wait(TimeSpan.FromSeconds(5)); } catch { /* best-effort */ } + try { _wal?.Dispose(); } catch { /* best-effort */ } + try { _pageFile?.Dispose(); } catch { /* best-effort */ } + try { _indexFile?.Dispose(); } catch { /* best-effort */ } + try { _writerGate?.Dispose(); } catch { /* best-effort */ } + try { _shm?.Dispose(); } catch { /* best-effort */ } + if (walCryptoProvider is IDisposable disposableWalCrypto) + try { disposableWalCrypto.Dispose(); } catch { /* best-effort */ } + try { _writerCts.Dispose(); } catch { /* best-effort */ } + throw; } - - InitializeDictionary(); - InitializeKv(); - - // Create and start checkpoint manager - // _checkpointManager = new Transactions.CheckpointManager(this); - // _checkpointManager.StartAutoCheckpoint(); } /// @@ -315,9 +334,9 @@ public void Dispose() } // 3. Close WAL and PageFile. - _wal?.Dispose(); - _pageFile?.Dispose(); - _indexFile?.Dispose(); + try { _wal?.Dispose(); } catch { /* best-effort */ } + try { _pageFile?.Dispose(); } catch { /* best-effort */ } + try { _indexFile?.Dispose(); } catch { /* best-effort */ } // 4. Close per-collection PageFiles (Phase 4) if (_collectionFiles != null) @@ -329,11 +348,11 @@ public void Dispose() _collectionFiles.Clear(); } - _commitLock?.Dispose(); - _metadataLock?.Dispose(); - _writerGate?.Dispose(); - _metrics?.Dispose(); - _shm?.Dispose(); + try { _commitLock?.Dispose(); } catch { /* best-effort */ } + try { _metadataLock?.Dispose(); } catch { /* best-effort */ } + try { _writerGate?.Dispose(); } catch { /* best-effort */ } + try { _metrics?.Dispose(); } catch { /* best-effort */ } + try { _shm?.Dispose(); } catch { /* best-effort */ } } internal void RegisterCdc(CDC.ChangeStreamDispatcher cdc) diff --git a/tests/BLite.Tests/EncryptionTests.cs b/tests/BLite.Tests/EncryptionTests.cs index 0f651b1..9a5f66f 100644 --- a/tests/BLite.Tests/EncryptionTests.cs +++ b/tests/BLite.Tests/EncryptionTests.cs @@ -1577,4 +1577,43 @@ public async Task CryptoOptions_StringAndBytes_AreInteroperable() Assert.Equal("interop", e!.Payload); } } + + // ── DocumentDbContext constructor failure / file-lock regression (issue #129) ─ + + /// + /// Regression test for issue #129: + /// If the constructor fails (e.g. wrong encryption + /// password causing a decryption error), all acquired file handles must be released + /// before the exception propagates to the caller. + /// After the constructor throws, the database file must be deletable in the same + /// process without GC.Collect() or process restart. + /// + [Fact] + public async Task DocumentDbContext_ConstructorFailure_WrongPassword_FileNotLocked() + { + var path = TempDb(); + var correct = new CryptoOptions("correct-password", iterations: 1); + var wrong = new CryptoOptions("wrong-password", iterations: 1); + + // 1. Create the encrypted database and persist at least one document so that + // collection-metadata pages are written to the main file. + await using (var ctx = new MultiFileTestDbContext(path, correct)) + { + await ctx.Entries.InsertAsync(new MultiFileEntry { Id = 1, Payload = "hello", Tag = "t" }); + await ctx.CheckpointAsync(); // flush WAL into main file + } + + // 2. Attempt to open with the wrong password — the constructor must throw. + Assert.ThrowsAny(() => + { + using var ctx = new MultiFileTestDbContext(path, wrong); + }); + +// 3. The main database file must not be locked after the failed constructor. +// Before the fix this would throw IOException (exclusive open fails on Windows; advisory-lock fails on Unix). +Assert.True(File.Exists(path), "Database file should exist before attempting deletion."); +using (var _ = new FileStream(path, FileMode.Open, FileAccess.ReadWrite, FileShare.None)) { } +File.Delete(path); +Assert.False(File.Exists(path), "Database file must be deletable immediately after failed constructor."); + } }