From d19e3034067437bbeddee23b3e58b9aec913f0a9 Mon Sep 17 00:00:00 2001 From: Terry Schneider Date: Tue, 26 May 2026 19:57:26 -0500 Subject: [PATCH] feat: implement lock lost handling and exceptions - Added LockLostException to handle scenarios where a distributed lock is lost. - Updated AcquireLockFailedException to include the key in the error message. - Enhanced LitRedisDistributedLockModel to track lock status and lost state. - Introduced MaxExtendRetries in RequestLockModel to configure lock renewal failure tolerance. - Updated README with examples for handling lost locks and lock renewal failures. - Added tests to verify lock lost behavior in LitRedisDistributedLockService. --- .../Exceptions/AcquireLockFailedException.cs | 4 +- LitRedis.Core/Exceptions/LockLostException.cs | 14 ++++ .../LitRedisDistributedLockService.cs | 70 ++++++++++++---- .../Models/LitRedisDistributedLockModel.cs | 82 +++++++++++++++++-- LitRedis.Core/Models/LitRedisLockStatus.cs | 8 ++ LitRedis.Core/Models/RequestLockModel.cs | 23 +++++- LitRedis.Sample/SampleHostedService.cs | 53 ++++++++++++ .../LitRedisDistributedLockServiceTests.cs | 42 ++++++++++ README.md | 56 ++++++++++++- 9 files changed, 321 insertions(+), 31 deletions(-) create mode 100644 LitRedis.Core/Exceptions/LockLostException.cs create mode 100644 LitRedis.Core/Models/LitRedisLockStatus.cs diff --git a/LitRedis.Core/Exceptions/AcquireLockFailedException.cs b/LitRedis.Core/Exceptions/AcquireLockFailedException.cs index 94641ad..f68a702 100644 --- a/LitRedis.Core/Exceptions/AcquireLockFailedException.cs +++ b/LitRedis.Core/Exceptions/AcquireLockFailedException.cs @@ -4,6 +4,6 @@ namespace LitRedis.Core.Exceptions; public class AcquireLockFailedException : Exception { - public AcquireLockFailedException(TimeSpan waitTime) - : base($"Failed to acquire lock after waiting {waitTime}") { } + public AcquireLockFailedException(string key, TimeSpan waitTime) + : base($"Failed to acquire lock for key '{key}' after waiting {waitTime}") { } } diff --git a/LitRedis.Core/Exceptions/LockLostException.cs b/LitRedis.Core/Exceptions/LockLostException.cs new file mode 100644 index 0000000..23a196b --- /dev/null +++ b/LitRedis.Core/Exceptions/LockLostException.cs @@ -0,0 +1,14 @@ +using System; + +namespace LitRedis.Core.Exceptions; + +public class LockLostException : Exception +{ + public LockLostException() + : base("The distributed lock was lost before the operation completed.") { } + + public LockLostException(string key) + : base($"The distributed lock for key '{key}' was lost before the operation completed.") { } + + public LockLostException(string message, Exception innerException) : base(message, innerException) { } +} diff --git a/LitRedis.Core/Implementations/LitRedisDistributedLockService.cs b/LitRedis.Core/Implementations/LitRedisDistributedLockService.cs index 5baf6db..f719b2e 100644 --- a/LitRedis.Core/Implementations/LitRedisDistributedLockService.cs +++ b/LitRedis.Core/Implementations/LitRedisDistributedLockService.cs @@ -35,44 +35,82 @@ public async Task AcquireLockAsync( if (await _litRedisDistributedLock.TakeLockAsync(lockKey, token, requestLockModel.LockIncrease, cancellationToken)) { - var stopped = false; + var lockLostCts = new CancellationTokenSource(); + var stopped = 0; - _ = Task.Run(async () => - { - while (!stopped && !cancellationToken.IsCancellationRequested) + LitRedisDistributedLockModel model = null; + + model = LitRedisDistributedLockModel.Success( + async () => { try { - await _litRedisDistributedLock.ExtendLockAsync(lockKey, token, requestLockModel.LockIncrease, cancellationToken); + // stop the keep alive thread and release the lock + Interlocked.Exchange(ref stopped, 1); + + if (!model.IsLost) + { + await _litRedisDistributedLock.ReleaseLockAsync(lockKey, token, CancellationToken.None); + } + } catch (Exception ex) { - _logger.LogError(ex, "Extended Distributed Lock Error"); + _logger.LogError(ex, "Error releasing lock"); } + }, + lockLostCts, + lockKey + ); + + _ = Task.Run(async () => + { + var consecutiveFailures = 0; + while (Volatile.Read(ref stopped) == 0 && !cancellationToken.IsCancellationRequested) + { try { await Task.Delay(requestLockModel.RenewLockInterval, cancellationToken); } catch (TaskCanceledException) { } - } - }, cancellationToken); - return LitRedisDistributedLockModel.Success( - async () => - { + if (Volatile.Read(ref stopped) != 0 || cancellationToken.IsCancellationRequested) + { + break; + } + try { - //stop the keep alive thread and release the lock - stopped = true; - await _litRedisDistributedLock.ReleaseLockAsync(lockKey, token, cancellationToken); + var extended = await _litRedisDistributedLock.ExtendLockAsync(lockKey, token, requestLockModel.LockIncrease, cancellationToken); + if (!extended) + { + _logger.LogWarning("Failed to extend distributed lock for key {LockKey}. Lock may have been lost.", lockKey); + model.MarkLost(); + break; + } + + consecutiveFailures = 0; + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + break; } catch (Exception ex) { - _logger.LogError(ex, "Error releasing lock"); + consecutiveFailures++; + _logger.LogError(ex, "Extended Distributed Lock Error ({ConsecutiveFailures}/{MaxRetries})", consecutiveFailures, requestLockModel.MaxExtendRetries); + + if (consecutiveFailures >= requestLockModel.MaxExtendRetries) + { + model.MarkLost(); + break; + } } } - ); + }, cancellationToken); + + return model; } try diff --git a/LitRedis.Core/Models/LitRedisDistributedLockModel.cs b/LitRedis.Core/Models/LitRedisDistributedLockModel.cs index 2d8488c..81c7397 100644 --- a/LitRedis.Core/Models/LitRedisDistributedLockModel.cs +++ b/LitRedis.Core/Models/LitRedisDistributedLockModel.cs @@ -1,4 +1,5 @@ using System; +using System.Threading; using System.Threading.Tasks; using LitRedis.Core.Exceptions; @@ -6,7 +7,9 @@ namespace LitRedis.Core.Models; public class LitRedisDistributedLockModel : IDisposable, IAsyncDisposable { - private bool _disposed; + private int _disposed; + private int _lostFlag; + private CancellationTokenSource _lockLostCts; private Func ReleaseAction { get; set; } @@ -16,19 +19,70 @@ public class LitRedisDistributedLockModel : IDisposable, IAsyncDisposable public TimeSpan? WaitTime { get; set; } - public LitRedisDistributedLockModel(bool succeeded, Func releaseAction, bool wasCancelled, TimeSpan? waitTime) + /// + /// Gets the current status of the distributed lock. + /// + public LitRedisLockStatus Status { get; internal set; } = LitRedisLockStatus.NotAcquired; + + /// + /// Returns true if the lock has been lost. Thread-safe. + /// + public bool IsLost => Volatile.Read(ref _lostFlag) == 1; + + /// + /// Cancelled if the background renewal loop fails to extend the lock, indicating the lock may have been lost. + /// + public CancellationToken LockLostToken => _lockLostCts?.Token ?? CancellationToken.None; + + /// + /// Gets the key that was locked. + /// + public string Key { get; } + + public LitRedisDistributedLockModel(bool succeeded, Func releaseAction, bool wasCancelled, TimeSpan? waitTime, CancellationTokenSource lockLostCts = null, string key = null) { Succeeded = succeeded; ReleaseAction = releaseAction; WasCancelled = wasCancelled; WaitTime = waitTime; + _lockLostCts = lockLostCts; + Key = key; } - public static LitRedisDistributedLockModel Success(Func releaseAction = null) - => new(true, releaseAction, false, null); + public static LitRedisDistributedLockModel Success(Func releaseAction = null, CancellationTokenSource lockLostCts = null, string key = null) + => new(true, releaseAction, false, null, lockLostCts, key) + { + Status = LitRedisLockStatus.Acquired, + }; public static LitRedisDistributedLockModel Failure(bool wasCancelled = false, TimeSpan? waitTime = null) - => new(false, null, wasCancelled, waitTime); + => new(false, null, wasCancelled, waitTime) { Status = LitRedisLockStatus.NotAcquired }; + + /// + /// Marks the lock as lost and signals . + /// + internal void MarkLost() + { + if (Interlocked.CompareExchange(ref _lostFlag, 1, 0) != 0) + { + return; + } + + Status = LitRedisLockStatus.Lost; + + // Signal the cancellation token to notify callers that the lock was lost. + // Do not null out the field here so callers can still observe the cancelled + // token via the LockLostToken property. Disposal will occur in DisposeAsync. + var cts = Volatile.Read(ref _lockLostCts); + try + { + cts?.Cancel(); + } + catch (ObjectDisposedException) + { + // CTS was already disposed; nothing to signal. + } + } private async Task ReleaseAsync() { @@ -42,21 +96,33 @@ private async Task ReleaseAsync() public async ValueTask DisposeAsync() { - if (_disposed) + if (Interlocked.CompareExchange(ref _disposed, 1, 0) != 0) { return; } await ReleaseAsync(); - _disposed = true; + var cts = Interlocked.Exchange(ref _lockLostCts, null); + cts?.Dispose(); } public void ThrowIfFailedToAcquire() { if (!Succeeded) { - throw new AcquireLockFailedException(WaitTime.GetValueOrDefault()); + throw new AcquireLockFailedException(Key, WaitTime.GetValueOrDefault()); + } + } + + /// + /// Throws a if the background renewal loop has reported that the lock was lost. + /// + public void ThrowOnLockLost() + { + if (IsLost) + { + throw new LockLostException(Key); } } diff --git a/LitRedis.Core/Models/LitRedisLockStatus.cs b/LitRedis.Core/Models/LitRedisLockStatus.cs new file mode 100644 index 0000000..83182c9 --- /dev/null +++ b/LitRedis.Core/Models/LitRedisLockStatus.cs @@ -0,0 +1,8 @@ +namespace LitRedis.Core.Models; + +public enum LitRedisLockStatus +{ + NotAcquired = 0, + Acquired = 1, + Lost = 2, +} diff --git a/LitRedis.Core/Models/RequestLockModel.cs b/LitRedis.Core/Models/RequestLockModel.cs index 00ed74d..416531e 100644 --- a/LitRedis.Core/Models/RequestLockModel.cs +++ b/LitRedis.Core/Models/RequestLockModel.cs @@ -22,13 +22,30 @@ public class RequestLockModel /// /// Gets or sets a value indicating how long to extend the lock each time it is renewed. /// - public TimeSpan LockIncrease { get; set; } = TimeSpan.FromSeconds(10); + public TimeSpan LockIncrease { get; set; } = TimeSpan.FromSeconds(30); /// - /// Sets a value indicating how long to extend the lock each time it is renewed. + /// Gets or sets the maximum number of consecutive exceptions from ExtendLockAsync + /// before the lock is considered lost. Defaults to 3. Set to 0 to mark lost on the first exception. + /// + public int MaxExtendRetries { get; set; } = 3; + + /// + /// Sets the maximum number of consecutive exceptions before the lock is considered lost. + /// + /// Maximum consecutive exceptions allowed before marking the lock lost. + /// The request lock model + public RequestLockModel WithMaxExtendRetries(int maxExtendRetries) + { + MaxExtendRetries = maxExtendRetries; + return this; + } + + /// + /// Sets a value indicating how long to extend the lock each time it is renewed. /// /// How long to increase the lock for. - /// + /// The request lock model public RequestLockModel WithLockIncrease(TimeSpan lockIncrease) { LockIncrease = lockIncrease; diff --git a/LitRedis.Sample/SampleHostedService.cs b/LitRedis.Sample/SampleHostedService.cs index b1b85d3..aa94e42 100644 --- a/LitRedis.Sample/SampleHostedService.cs +++ b/LitRedis.Sample/SampleHostedService.cs @@ -1,6 +1,7 @@ using System; using System.Threading; using System.Threading.Tasks; +using LitRedis.Core.Exceptions; using LitRedis.Core.Interfaces; using LitRedis.Core.Models; using Microsoft.Extensions.Caching.Distributed; @@ -29,6 +30,8 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) await DoLockNoWaitSample(stoppingToken); + await DoLockLostSample(stoppingToken); + while (!stoppingToken.IsCancellationRequested) { var tasks = new[] @@ -60,6 +63,7 @@ private async Task DoLockNoWaitSample(CancellationToken stoppingToken) } }, stoppingToken); + await Task.Delay(TimeSpan.FromSeconds(1), stoppingToken); var dontGetIt = Task.Run(async () => { @@ -84,6 +88,55 @@ private async Task DoLockNoWaitSample(CancellationToken stoppingToken) await Task.WhenAll(getIt, dontGetIt); } + private async Task DoLockLostSample(CancellationToken stoppingToken) + { + try + { + var myId = Guid.NewGuid(); + + // We will always lose the lock if the renewal interval is longer than the lock increase + var model = RequestLockModel + .WithKey("lit-sample-lock-lost") + .WithLockIncrease(TimeSpan.FromSeconds(2)) + .WithRenewLockInterval(TimeSpan.FromSeconds(5)); + + await using var locker = await redisDistributedLockService.AcquireLockAsync(model, stoppingToken); + + if (!locker.Succeeded) + { + logger.LogInformation("Never got the lock {@Id}", myId); + return; + } + + logger.LogInformation("Got the lock, status is {@Status} {@Id}", locker.Status, myId); + + // Option 1: link the LockLostToken into your work so it cancels if the lock is lost + using var workCts = CancellationTokenSource.CreateLinkedTokenSource(stoppingToken, locker.LockLostToken); + + try + { + await Task.Delay(TimeSpan.FromSeconds(10), workCts.Token); + } + catch (OperationCanceledException) when (locker.Status == LitRedisLockStatus.Lost) + { + logger.LogWarning("Work was cancelled because the lock was lost {@Id}", myId); + } + + // Option 2: poll explicitly when reaching a critical section + locker.ThrowOnLockLost(); + + logger.LogInformation("Finished work while still holding the lock {@Id}", myId); + } + catch (LockLostException ex) + { + logger.LogCritical(ex, "Lock was lost mid-operation"); + } + catch (Exception ex) + { + logger.LogCritical(ex, "Error running lock lost sample"); + } + } + private async Task DoLockSample(CancellationToken stoppingToken) { try diff --git a/LitRedis.Tests/Core/Implementations/LitRedisDistributedLockServiceTests.cs b/LitRedis.Tests/Core/Implementations/LitRedisDistributedLockServiceTests.cs index 6aaebe7..a810211 100644 --- a/LitRedis.Tests/Core/Implementations/LitRedisDistributedLockServiceTests.cs +++ b/LitRedis.Tests/Core/Implementations/LitRedisDistributedLockServiceTests.cs @@ -4,6 +4,7 @@ using AutoFixture; using AutoFixture.AutoMoq; using FluentAssertions; +using LitRedis.Core.Exceptions; using LitRedis.Core.Implementations; using LitRedis.Core.Interfaces; using LitRedis.Core.Models; @@ -61,4 +62,45 @@ public async Task Lit_Redis_Distributed_Lock_Service_Should_Not_Acquire_Lock() result.Succeeded.Should().BeFalse(); wrapper.Verify(x => x.TakeLockAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.AtLeast(2)); } + + [TestMethod] + public async Task Lit_Redis_Distributed_Lock_Service_Should_Mark_Lock_Lost_When_Lock_Fails_To_Extend() + { + //arrange + var fixture = new Fixture().Customize(new AutoMoqCustomization()); + var wrapper = fixture.Freeze>(); + wrapper.Setup(x => x.TakeLockAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(true); + wrapper.Setup(x => x.ExtendLockAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(false); + + var redisDistributedLock = fixture.Create(); + + var model = RequestLockModel + .WithKey("fake") + .WithLockWaitTimeout(TimeSpan.FromSeconds(1)) + .WithRenewLockInterval(TimeSpan.FromMilliseconds(100)); + + //act + await using var locker = await redisDistributedLock.AcquireLockAsync(model, default); + + locker.Succeeded.Should().BeTrue(); + + // Wait for the renewal loop to run: delay (100ms) + extend call + mark lost + await Task.Delay(TimeSpan.FromMilliseconds(200)); + + //assert + locker.Status.Should().Be(LitRedisLockStatus.Lost); + locker.LockLostToken.IsCancellationRequested.Should().BeTrue(); + + Action throwOnLost = () => locker.ThrowOnLockLost(); + throwOnLost.Should().Throw(); + + // Verify ExtendLockAsync was called (diagnostic) + wrapper.Verify(x => x.ExtendLockAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.AtLeastOnce); + + // Disposing a lost lock must not attempt to release the key on the server. + await locker.DisposeAsync(); + wrapper.Verify(x => x.ReleaseLockAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } } diff --git a/README.md b/README.md index 6c72099..5df454e 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,7 @@ - [Wait forever](#wait-forever) - [Set the wait increase, timeout, and interval](#set-the-wait-increase-timeout-and-interval) - [No wait](#no-wait) + - [Handling lost locks](#handling-lost-locks) # lit-redis A C# managed Redis Library for doing caching, locking, and concurrency @@ -107,7 +108,7 @@ Attempt to acquire a lock on a particular key, waiting until the lock is able to #### Default -By default, `LockIncrease` and `RenewLockInterval` are both set to 10 seconds. +By default, `LockIncrease` is set to 30 seconds and `RenewLockInterval` is set to 10 seconds. The renewal interval should always be shorter than the lock increase so the lock is extended before it expires. ```csharp try { @@ -198,4 +199,55 @@ try { catch (Exception ex) { _logger.LogCritical(ex, "Error"); } -``` \ No newline at end of file +``` + +## Acquire failure exception + +If acquiring a lock fails, calling `ThrowIfFailedToAcquire()` will throw an `AcquireLockFailedException` + +### Handling lost locks + +Once a lock is acquired, a background task keeps it alive by extending it on every `RenewLockInterval`. If a renewal ever fails (for example because Redis returned an error or another client took over the key after expiration), the lock is considered _lost_. When this happens the model exposes two ways to react so you can pick the one that fits your code style. + +- `Status` reflects the current state of the lock: `NotAcquired`, `Acquired`, or `Lost`. +- `LockLostToken` is a `CancellationToken` that is cancelled when the lock is lost. It's safe to link it into your own work via `CancellationTokenSource.CreateLinkedTokenSource`. +- `ThrowOnLockLost()` throws a `LockLostException` if the lock has already been lost. Useful right before a critical section. + +```csharp +try { + var model = RequestLockModel + .WithKey("lit-sample") + .WithLockIncrease(TimeSpan.FromSeconds(5)) + .WithRenewLockInterval(TimeSpan.FromSeconds(2)); + + await using var locker = await _redisDistributedLockService.AcquireLockAsync(model, stoppingToken); + + if (!locker.Succeeded) + { + _logger.LogInformation("No lock acquired"); + return; + } + + // 1. Link the cancellation token into your work + using var workCts = CancellationTokenSource.CreateLinkedTokenSource(stoppingToken, locker.LockLostToken); + await DoWorkAsync(workCts.Token); + + // 2. Throw at a critical section + locker.ThrowOnLockLost(); + await CommitAsync(stoppingToken); +} +catch (LockLostException ex) { + _logger.LogCritical(ex, "Lock was lost mid-operation"); +} +catch (Exception ex) { + _logger.LogCritical(ex, "Error"); +} +``` + +When the lock is lost, the release callback will _not_ attempt to release the key on dispose, since another holder may already own it. + +## Lock renewal failures + +If the background renewal loop repeatedly fails to extend the lock the library will mark the lock as lost. The number of consecutive extension failures tolerated before marking the lock lost is configurable via `RequestLockModel.MaxExtendRetries` (default: 3). You can set it fluently with `WithMaxExtendRetries(int)` on the request model. + +When a lock is marked lost the `Status` becomes `Lost` and `LockLostToken` will be cancelled so callers can react promptly.