From c1ab12a71219a79f7759b1167c44b4e13dbbc21b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Wed, 28 Oct 2026 06:07:57 +0100 Subject: [PATCH] fix(storage): a partial PageFileConfig gets LockTimeout.Default, not Immediate (#322) A PageFileConfig built with an object initializer that does not set LockTimeout opened with default(LockTimeout), whose values equal LockTimeout.Immediate, so every contended lock failed at once. LockTimeout now records whether any of its properties was set (not part of equality), and Normalize() replaces an unset value with LockTimeout.Default, as it already does for PageSize and GrowthBlockSize. An explicit Immediate or From(0, 0, 0) is kept. Co-Authored-By: Claude Opus 5.5 --- src/BLite.Core/Storage/LockTimeout.cs | 22 ++++++++++-- src/BLite.Core/Storage/PageFile.cs | 9 +++-- .../PageFileConfigLockTimeoutTests.cs | 34 +++++++++++++++++++ 3 files changed, 59 insertions(+), 6 deletions(-) create mode 100644 tests/BLite.Tests/PageFileConfigLockTimeoutTests.cs diff --git a/src/BLite.Core/Storage/LockTimeout.cs b/src/BLite.Core/Storage/LockTimeout.cs index 10693990..4503f858 100644 --- a/src/BLite.Core/Storage/LockTimeout.cs +++ b/src/BLite.Core/Storage/LockTimeout.cs @@ -14,6 +14,11 @@ namespace BLite.Core.Storage; /// busy_timeout is 0) or specify custom values to give concurrent callers /// time to release contended locks. /// +/// +/// default(LockTimeout), or new LockTimeout() with no property set, means +/// "not specified": replaces it with +/// . To fail fast, set explicitly. +/// /// public readonly struct LockTimeout : IEquatable { @@ -25,7 +30,7 @@ namespace BLite.Core.Storage; /// >0 — wait up to this many milliseconds. /// /// - public int ReadTimeoutMs { get; init; } + public int ReadTimeoutMs { get => _readTimeoutMs; init { _readTimeoutMs = value; _isSpecified = true; } } /// /// Timeout for acquiring write locks (milliseconds). @@ -35,7 +40,7 @@ namespace BLite.Core.Storage; /// >0 — wait up to this many milliseconds. /// /// - public int WriteTimeoutMs { get; init; } + public int WriteTimeoutMs { get => _writeTimeoutMs; init { _writeTimeoutMs = value; _isSpecified = true; } } /// /// Maximum number of threads that can simultaneously enter the commit path. @@ -50,7 +55,18 @@ namespace BLite.Core.Storage; /// >0 — at most this many concurrent commit operations. /// /// - public int MaxConcurrentWriters { get; init; } + public int MaxConcurrentWriters { get => _maxConcurrentWriters; init { _maxConcurrentWriters = value; _isSpecified = true; } } + + private readonly int _readTimeoutMs; + private readonly int _writeTimeoutMs; + private readonly int _maxConcurrentWriters; + private readonly bool _isSpecified; + + /// + /// false for default(LockTimeout): no property was set, so the value is + /// indistinguishable from but was not chosen. Not part of equality. + /// + internal bool IsSpecified => _isSpecified; /// /// Immediate failure on contention — equivalent to SQLite's default diff --git a/src/BLite.Core/Storage/PageFile.cs b/src/BLite.Core/Storage/PageFile.cs index 2936ed28..9f29fef4 100644 --- a/src/BLite.Core/Storage/PageFile.cs +++ b/src/BLite.Core/Storage/PageFile.cs @@ -46,7 +46,8 @@ public readonly struct PageFileConfig /// Lock acquisition timeouts for read and write operations. /// Controls how long the engine waits for a contended lock before throwing /// (analogous to SQLite's SQLITE_BUSY). - /// Defaults to (500 ms read / 500 ms write). + /// Defaults to (500 ms read / 500 ms write), also when + /// the config is built with an object initializer that does not set it. /// Use for fail-fast behaviour. /// public LockTimeout LockTimeout { get; init; } @@ -133,8 +134,9 @@ public readonly struct PageFileConfig public bool AllowMultiProcessAccess { get; init; } /// - /// Returns a copy of this config where any zero-valued numeric fields are replaced - /// with the corresponding values from . + /// Returns a copy of this config where any zero-valued numeric fields, and a + /// that was never set, are replaced with the corresponding + /// values from . /// This ensures that a partially-constructed /// (e.g. new PageFileConfig { AllowMultiProcessAccess = true }) is always /// valid before being handed to the storage engine. @@ -146,6 +148,7 @@ public PageFileConfig Normalize() { PageSize = PageSize > 0 ? PageSize : defaults.PageSize, GrowthBlockSize = GrowthBlockSize > 0 ? GrowthBlockSize : defaults.GrowthBlockSize, + LockTimeout = LockTimeout.IsSpecified ? LockTimeout : defaults.LockTimeout, }; } diff --git a/tests/BLite.Tests/PageFileConfigLockTimeoutTests.cs b/tests/BLite.Tests/PageFileConfigLockTimeoutTests.cs new file mode 100644 index 00000000..c25b1ff8 --- /dev/null +++ b/tests/BLite.Tests/PageFileConfigLockTimeoutTests.cs @@ -0,0 +1,34 @@ +using BLite.Core.Storage; + +namespace BLite.Tests; + +/// +/// Regression test for #322: a built with an object initializer +/// that does not set used to open with +/// default(LockTimeout), which has the same values as , +/// so every contended lock failed at once. +/// +public class PageFileConfigLockTimeoutTests +{ + [Fact] + public void StorageEngine_PartialConfigWithoutLockTimeout_GetsDefault_ExplicitImmediateIsKept() + { + var dir = Path.Combine(Path.GetTempPath(), $"blite_locktimeout_{Guid.NewGuid():N}"); + Directory.CreateDirectory(dir); + try + { + using (var engine = new StorageEngine(Path.Combine(dir, "partial.db"), new PageFileConfig { AllowMultiProcessAccess = false })) + Assert.Equal(LockTimeout.Default, engine.LockTimeout); + + using (var engine = new StorageEngine(Path.Combine(dir, "immediate.db"), new PageFileConfig { LockTimeout = LockTimeout.Immediate })) + Assert.Equal(LockTimeout.Immediate, engine.LockTimeout); + + using (var engine = new StorageEngine(Path.Combine(dir, "from.db"), new PageFileConfig { LockTimeout = LockTimeout.From(0, 0, 0) })) + Assert.Equal(LockTimeout.Immediate, engine.LockTimeout); + } + finally + { + try { Directory.Delete(dir, recursive: true); } catch { /* best-effort */ } + } + } +}