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 */ } + } + } + } + } +}