From 68243b987002926d908d08e48fca89e0af907d7f Mon Sep 17 00:00:00 2001 From: Thomas Buck Date: Wed, 24 Jun 2026 00:47:44 +0100 Subject: [PATCH] fix: run distributed-lock heartbeat on a dedicated connection (issue #79) SQLiteDistributedLock ran its heartbeat timer's UpdateExpiration on the consumer's HangfireDbContext while the consumer used that same NoMutex (non-thread-safe) connection on another thread, causing AccessViolationException and storage corruption (upstream raisedapp/Hangfire.Storage.SQLite#79). The heartbeat now obtains its own dedicated connection from the storage pool (storage.CreateAndOpenConnection()), so the timer thread never touches the caller's connection. Storage is plumbed through the two internal acquisition paths (HangfireSQLiteConnection.AcquireDistributedLock and ExpirationManager.RemoveExpireRows); the existing public Acquire overload is preserved and falls back to the legacy shared-connection behaviour when no storage is supplied. Bumps version to 0.5.1-beta. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_017xW2zhMvrJChpY58hfJBeZ --- .../ExpirationManager.cs | 2 +- .../Hangfire.Storage.SQLite.csproj | 7 +- .../HangfireSQLiteConnection.cs | 8 +- .../SQLiteDistributedLock.cs | 44 +++++++++- .../Hangfire.Storage.SQLite/SQLiteStorage.cs | 2 +- .../SQLiteDistributedLockHeartbeatFacts.cs | 83 +++++++++++++++++++ 6 files changed, 139 insertions(+), 7 deletions(-) create mode 100644 src/test/Hangfire.Storage.SQLite.Test/SQLiteDistributedLockHeartbeatFacts.cs diff --git a/src/main/Hangfire.Storage.SQLite/ExpirationManager.cs b/src/main/Hangfire.Storage.SQLite/ExpirationManager.cs index 1627a6e..7b0da98 100644 --- a/src/main/Hangfire.Storage.SQLite/ExpirationManager.cs +++ b/src/main/Hangfire.Storage.SQLite/ExpirationManager.cs @@ -106,7 +106,7 @@ private int RemoveExpireRows(HangfireDbContext db, string table) try { using (SQLiteDistributedLock.Acquire(DistributedLockKey, DefaultLockTimeout, - db, db.StorageOptions)) + db, db.StorageOptions, _storage)) { rowsAffected = db.Database.Execute(deleteScript); } diff --git a/src/main/Hangfire.Storage.SQLite/Hangfire.Storage.SQLite.csproj b/src/main/Hangfire.Storage.SQLite/Hangfire.Storage.SQLite.csproj index 86f57bc..b0f7f44 100644 --- a/src/main/Hangfire.Storage.SQLite/Hangfire.Storage.SQLite.csproj +++ b/src/main/Hangfire.Storage.SQLite/Hangfire.Storage.SQLite.csproj @@ -10,7 +10,7 @@ netstandard2.0 - 0.5.0-beta + 0.5.1-beta tbbuck.Hangfire.Storage.SQLite Thomas Buck Thomas Buck @@ -24,6 +24,11 @@ Hangfire Storage SQLite (tbbuck fork) An alternative SQLite storage for Hangfire. Maintained fork of RaisedApp/Hangfire.Storage.SQLite that modernises the SQLite dependency stack and remediates CVE-2025-6965. + 0.5.1-beta (tbbuck fork) + - Fix AccessViolationException / storage corruption in SQLiteDistributedLock: the lock + heartbeat now runs on its own dedicated connection instead of sharing the caller's + non-thread-safe (NoMutex) connection (upstream issue #79). + 0.5.0-beta (tbbuck fork) - Security: remediate CVE-2025-6965 (SQLite < 3.50.2, High/CVSS 7.2) by dropping the bundled SQLitePCLRaw.lib.e_sqlite3 2.1.11 native library. diff --git a/src/main/Hangfire.Storage.SQLite/HangfireSQLiteConnection.cs b/src/main/Hangfire.Storage.SQLite/HangfireSQLiteConnection.cs index 48668f3..09f2af2 100644 --- a/src/main/Hangfire.Storage.SQLite/HangfireSQLiteConnection.cs +++ b/src/main/Hangfire.Storage.SQLite/HangfireSQLiteConnection.cs @@ -19,6 +19,12 @@ public class HangfireSQLiteConnection : JobStorageConnection public HangfireDbContext DbContext { get; } + /// + /// Owning storage, set by . Lets the distributed + /// lock heartbeat run on a dedicated connection instead of this connection (issue #79). + /// + internal SQLiteStorage Storage { get; set; } + /// /// Ctor using default storage options /// @@ -47,7 +53,7 @@ public override void Dispose() public override IDisposable AcquireDistributedLock(string resource, TimeSpan timeout) { return Retry.Twice((_) => - SQLiteDistributedLock.Acquire($"HangFire:{resource}", timeout, DbContext, _storageOptions) + SQLiteDistributedLock.Acquire($"HangFire:{resource}", timeout, DbContext, _storageOptions, Storage) ); } diff --git a/src/main/Hangfire.Storage.SQLite/SQLiteDistributedLock.cs b/src/main/Hangfire.Storage.SQLite/SQLiteDistributedLock.cs index bd1b277..fab1a94 100644 --- a/src/main/Hangfire.Storage.SQLite/SQLiteDistributedLock.cs +++ b/src/main/Hangfire.Storage.SQLite/SQLiteDistributedLock.cs @@ -21,6 +21,10 @@ public class SQLiteDistributedLock : IDisposable private readonly SQLiteStorageOptions _storageOptions; + // When available, the heartbeat uses its own dedicated connection from the storage pool + // instead of the consumer's (NoMutex, not thread-safe) connection. See issue #79. + private readonly SQLiteStorage _storage; + private Timer _heartbeatTimer; private bool _completed; @@ -38,11 +42,13 @@ public class SQLiteDistributedLock : IDisposable /// Thrown if lock is not acuired within the timeout private SQLiteDistributedLock(string resource, HangfireDbContext database, - SQLiteStorageOptions storageOptions) + SQLiteStorageOptions storageOptions, + SQLiteStorage storage) { _resource = resource ?? throw new ArgumentNullException(nameof(resource)); _dbContext = database ?? throw new ArgumentNullException(nameof(database)); _storageOptions = storageOptions ?? throw new ArgumentNullException(nameof(storageOptions)); + _storage = storage; _resourceKey = Guid.NewGuid().ToString(); if (string.IsNullOrEmpty(resource)) @@ -56,13 +62,28 @@ public static SQLiteDistributedLock Acquire( TimeSpan timeout, HangfireDbContext database, SQLiteStorageOptions storageOptions) + { + return Acquire(resource, timeout, database, storageOptions, null); + } + + /// + /// Creates SQLite distributed lock, using a dedicated connection from + /// for the heartbeat so that the timer thread never shares the caller's (non-thread-safe) + /// connection. See issue #79. + /// + internal static SQLiteDistributedLock Acquire( + string resource, + TimeSpan timeout, + HangfireDbContext database, + SQLiteStorageOptions storageOptions, + SQLiteStorage storage) { if (timeout.TotalSeconds > int.MaxValue) { throw new ArgumentException($"The timeout specified is too large. Please supply a timeout equal to or less than {int.MaxValue} seconds", nameof(timeout)); } - var slock = new SQLiteDistributedLock(resource, database, storageOptions); + var slock = new SQLiteDistributedLock(resource, database, storageOptions, storage); slock.Acquire(timeout); slock.StartHeartBeat(); @@ -179,7 +200,24 @@ private void StartHeartBeat() // but since we use the resource key, we will not disturb other owners. try { - var didUpdate = UpdateExpiration(_dbContext.DistributedLockRepository, DateTime.UtcNow.Add(_storageOptions.DistributedLockLifetime)); + var newExpiry = DateTime.UtcNow.Add(_storageOptions.DistributedLockLifetime); + + bool didUpdate; + if (_storage != null) + { + // Run the heartbeat on a dedicated connection so the timer thread never + // touches the consumer's connection concurrently (issue #79). + using (var heartbeatContext = _storage.CreateAndOpenConnection()) + { + didUpdate = UpdateExpiration(heartbeatContext.DistributedLockRepository, newExpiry); + } + } + else + { + // Legacy path (no storage available): falls back to the shared connection. + didUpdate = UpdateExpiration(_dbContext.DistributedLockRepository, newExpiry); + } + Heartbeat?.Invoke(didUpdate); if (!didUpdate) { diff --git a/src/main/Hangfire.Storage.SQLite/SQLiteStorage.cs b/src/main/Hangfire.Storage.SQLite/SQLiteStorage.cs index 6b2e7ba..e1e2de7 100644 --- a/src/main/Hangfire.Storage.SQLite/SQLiteStorage.cs +++ b/src/main/Hangfire.Storage.SQLite/SQLiteStorage.cs @@ -107,7 +107,7 @@ public override IStorageConnection GetConnection() { CheckDisposed(); var dbContext = CreateAndOpenConnection(); - return new HangfireSQLiteConnection(dbContext, _storageOptions, QueueProviders); + return new HangfireSQLiteConnection(dbContext, _storageOptions, QueueProviders) { Storage = this }; } public override IMonitoringApi GetMonitoringApi() diff --git a/src/test/Hangfire.Storage.SQLite.Test/SQLiteDistributedLockHeartbeatFacts.cs b/src/test/Hangfire.Storage.SQLite.Test/SQLiteDistributedLockHeartbeatFacts.cs new file mode 100644 index 0000000..3cb14be --- /dev/null +++ b/src/test/Hangfire.Storage.SQLite.Test/SQLiteDistributedLockHeartbeatFacts.cs @@ -0,0 +1,83 @@ +using Hangfire.Storage.SQLite.Entities; +using Hangfire.Storage.SQLite.Test.Utils; +using System; +using System.IO; +using System.Linq; +using System.Threading; +using Xunit; + +namespace Hangfire.Storage.SQLite.Test +{ + public class SQLiteDistributedLockHeartbeatFacts + { + [Fact] + public void Heartbeat_RenewsExpiration_OnADedicatedConnection() + { + var options = new SQLiteStorageOptions { DistributedLockLifetime = TimeSpan.FromSeconds(1) }; // beat ~200ms + var storage = ConnectionUtils.CreateStorage(options); + using var consumer = storage.CreateAndOpenConnection(); + + DateTime ReadExpireAt() + { + using var reader = storage.CreateAndOpenConnection(); + return reader.DistributedLockRepository.First(x => x.Resource == "res-hb").ExpireAt; + } + + using (SQLiteDistributedLock.Acquire("res-hb", TimeSpan.FromSeconds(5), consumer, options, storage)) + { + var before = ReadExpireAt(); + Thread.Sleep(700); // allow a few heartbeats to fire + var after = ReadExpireAt(); + + Assert.True(after > before, $"Heartbeat should have renewed ExpireAt; before={before:O} after={after:O}"); + } + + // After dispose, the lock row is released. + using var check = storage.CreateAndOpenConnection(); + Assert.Empty(check.DistributedLockRepository.Where(x => x.Resource == "res-hb").ToList()); + } + + [Fact] + public void Lock_Heartbeat_DoesNotShareConsumerConnection_UnderConcurrentUse() + { + // Regression for issue #79: the heartbeat timer must run on its own connection so it never + // races the consumer's (NoMutex, non-thread-safe) connection. Uses a file-backed DB (WAL) + // so genuine concurrent writes from both connections are exercised. + var dbPath = Path.Combine(Path.GetTempPath(), $"hf_lock_{Guid.NewGuid():n}.db"); + try + { + var options = new SQLiteStorageOptions { DistributedLockLifetime = TimeSpan.FromSeconds(1) }; + var storage = new SQLiteStorage(dbPath, options); + + using (var consumer = storage.CreateAndOpenConnection()) + using (SQLiteDistributedLock.Acquire("res-stress", TimeSpan.FromSeconds(5), consumer, options, storage)) + { + // Hammer the consumer connection while the heartbeat fires concurrently. + var deadline = DateTime.UtcNow.AddSeconds(2); + var n = 0; + while (DateTime.UtcNow < deadline) + { + consumer.Database.Insert(new JobParameter + { + JobId = 1, + Name = $"p{n++}", + Value = "v", + ExpireAt = DateTime.UtcNow.AddMinutes(5), + }); + } + + Assert.True(n > 0); + } + + storage.Dispose(); + } + finally + { + foreach (var f in new[] { dbPath, dbPath + "-wal", dbPath + "-shm" }) + { + try { File.Delete(f); } catch { /* best effort */ } + } + } + } + } +}