Fix Redis TryAcquireAsync handling for disconnected databases

src/DistributedLock.Redis/RedLock/RedLockAcquire.cs

@@ -133,8 +133,15 @@ private async Task<bool> WaitForAcquireAsync(IReadOnlyDictionary<IDatabase, Task
                 ++faultCount;
                 if (RedLockHelper.HasTooManyFailuresOrFaults(faultCount, this._databases.Count))
                 {
-                    var faultingTasks = tryAcquireTasks.Values.Where(t => t.IsCanceled || t.IsFaulted)
+                    var faultingTasks = tryAcquireTasks.Values
+                        .Where(t => t.IsCanceled || t.IsFaulted)
                         .ToArray();
+                    if (faultingTasks.Length == 0)
+                    {
+                        // The original Redis task is still pending, so propagate the
+                        // disconnected-database exception created separately above.
+                        await completed.ConfigureAwait(false);
+                    }
                     if (faultingTasks.Length == 1)
                     {
                         await faultingTasks[0].ConfigureAwait(false); // propagate the error
@@ -167,7 +174,7 @@ private async Task<bool> WaitForAcquireAsync(IReadOnlyDictionary<IDatabase, Task
         //
         // The fix I've implemented is to bypass the backlog via a connectivity check when it comes to the server casting the "deciding vote" in a multi-server
         // scenario. That way, the case above is resolved quickly because both proceses will see that C is disconnected and fail the current acquire without
-        // waiting. Note that this is always a no-op in the single-server scenario.
+        // waiting. In a single-server scenario, a disconnected server is immediately decisive because there are no other operations to complete first.
         //
         // The current approach DOES NOT handle the case where the deciding vote is down to multiple servers all of which are down. We could implement this
         // at the cost of additional complexity, but currently I don't see that as worthwhile since the scenario should be much less common and more problematic
@@ -181,8 +188,10 @@ private async Task<bool> WaitForAcquireAsync(IReadOnlyDictionary<IDatabase, Task
         // https://github.com/StackExchange/StackExchange.Redis/issues/2645
         Task? TryResolveDisconnectedDatabaseAsFaulted(RedLockAcquire @this)
         {
-            // First, check to see if (a) we have at least 1 success/failure and (b) one more would be decisive. If not, bail.
-            if (!((successCount > 0 && RedLockHelper.HasSufficientSuccesses(successCount + 1, @this._databases.Count))
+            // For multiple databases, first check to see if (a) we have at least 1 success/failure and (b) one more would be decisive.
+            // A disconnected single database is always decisive.
+            if (@this._databases.Count != 1
+                && !((successCount > 0 && RedLockHelper.HasSufficientSuccesses(successCount + 1, @this._databases.Count))
                 || (failCount > 0 && RedLockHelper.HasTooManyFailuresOrFaults(failCount + 1, @this._databases.Count))))
             {
                 return null;

src/DistributedLock.Tests/Tests/Redis/RedisDistributedLockTest.cs

@@ -29,6 +29,109 @@ public void TestValidatesConstructorParameters()
         Assert.Throws<ArgumentException>(() => new RedisDistributedLock("key", Enumerable.Empty<IDatabase>()));
     }
 
+    [Test, Category("CI")]
+    public void TestDisconnectedSingleDatabaseCausesTryAcquireAsyncToThrow()
+    {
+        var pendingAcquire = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
+        var database = new Mock<IDatabase>(MockBehavior.Strict);
+        database
+            .Setup(d => d.StringSetAsync(
+                It.IsAny<RedisKey>(),
+                It.IsAny<RedisValue>(),
+                It.IsAny<TimeSpan?>(),
+                It.IsAny<When>(),
+                It.IsAny<CommandFlags>()))
+            .Returns(pendingAcquire.Task);
+        database
+            .Setup(d => d.IsConnected(It.IsAny<RedisKey>(), It.IsAny<CommandFlags>()))
+            .Returns(false);
+
+        var @lock = new RedisDistributedLock(
+            "key",
+            database.Object,
+            options => options
+                .Expiry(TimeSpan.FromMilliseconds(200))
+                .MinValidityTime(TimeSpan.FromMilliseconds(100))
+        );
+
+        Assert.ThrowsAsync<RedisException>(() => @lock.TryAcquireAsync().AsTask());
+    }
+
+    [Test, Category("CI")]
+    public void TestSyntheticDisconnectedFaultDoesNotMaskRealFault()
+    {
+        var expectedException = new TimeZoneNotFoundException();
+        var faultedDatabase = new Mock<IDatabase>(MockBehavior.Strict);
+        faultedDatabase
+            .Setup(d => d.StringSetAsync(
+                It.IsAny<RedisKey>(),
+                It.IsAny<RedisValue>(),
+                It.IsAny<TimeSpan?>(),
+                It.IsAny<When>(),
+                It.IsAny<CommandFlags>()))
+            .Returns(Task.FromException<bool>(expectedException));
+        faultedDatabase
+            .Setup(d => d.ScriptEvaluateAsync(
+                It.IsAny<string>(),
+                It.IsAny<RedisKey[]>(),
+                It.IsAny<RedisValue[]>(),
+                It.IsAny<CommandFlags>()))
+            .ReturnsAsync(RedisResult.Create(false));
+
+        var connectedPendingAcquire = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
+        var connectedDatabase = new Mock<IDatabase>(MockBehavior.Strict);
+        connectedDatabase
+            .Setup(d => d.StringSetAsync(
+                It.IsAny<RedisKey>(),
+                It.IsAny<RedisValue>(),
+                It.IsAny<TimeSpan?>(),
+                It.IsAny<When>(),
+                It.IsAny<CommandFlags>()))
+            .Returns(connectedPendingAcquire.Task);
+        connectedDatabase
+            .Setup(d => d.IsConnected(It.IsAny<RedisKey>(), It.IsAny<CommandFlags>()))
+            .Returns(true);
+
+        var disconnectedPendingAcquire = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
+        var disconnectedDatabase = new Mock<IDatabase>(MockBehavior.Strict);
+        disconnectedDatabase
+            .Setup(d => d.StringSetAsync(
+                It.IsAny<RedisKey>(),
+                It.IsAny<RedisValue>(),
+                It.IsAny<TimeSpan?>(),
+                It.IsAny<When>(),
+                It.IsAny<CommandFlags>()))
+            .Returns(disconnectedPendingAcquire.Task);
+        disconnectedDatabase
+            .Setup(d => d.IsConnected(It.IsAny<RedisKey>(), It.IsAny<CommandFlags>()))
+            .Returns(false);
+
+        var @lock = new RedisDistributedLock(
+            "key",
+            new[] { faultedDatabase.Object, connectedDatabase.Object, disconnectedDatabase.Object }
+        );
+
+        Assert.ThrowsAsync<TimeZoneNotFoundException>(() => @lock.TryAcquireAsync().AsTask());
+    }
+
+    [Test, Category("CI")]
+    public async Task TestSingleDatabaseContentionCausesTryAcquireAsyncToReturnNull()
+    {
+        var database = new Mock<IDatabase>(MockBehavior.Strict);
+        database
+            .Setup(d => d.StringSetAsync(
+                It.IsAny<RedisKey>(),
+                It.IsAny<RedisValue>(),
+                It.IsAny<TimeSpan?>(),
+                It.IsAny<When>(),
+                It.IsAny<CommandFlags>()))
+            .ReturnsAsync(false);
+
+        var @lock = new RedisDistributedLock("key", database.Object);
+
+        Assert.That(await @lock.TryAcquireAsync(), Is.Null);
+    }
+
     /// <summary>
     /// Reproduces the bug in https://github.com/madelson/DistributedLock/issues/162
     /// where a Redis lock couldn't be acquired if the current CultureInfo was tr-TR,