diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index d1cd9e7cb..dd5d92f83 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -8,6 +8,7 @@ Current package versions: ## Unreleased +- Fix [#2392](https://github.com/StackExchange/StackExchange.Redis/issues/2392): Dequeue *all* timed out messages from the backlog when not connected (including Fire+Forget) ([#2397 by kornelpal](https://github.com/StackExchange/StackExchange.Redis/pull/2397)) - Fix [#2400](https://github.com/StackExchange/StackExchange.Redis/issues/2400): Expose `ChannelMessageQueue` as `IAsyncEnumerable` ([#2402 by mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/2402)) - Add: support for `CLIENT SETINFO` (lib name/version) during handshake; opt-out is via `ConfigurationOptions`; also support read of `resp`, `lib-ver` and `lib-name` via `CLIENT LIST` ([#2414 by mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/2414)) diff --git a/src/StackExchange.Redis/PhysicalBridge.cs b/src/StackExchange.Redis/PhysicalBridge.cs index efcd6518f..7fa8af27f 100644 --- a/src/StackExchange.Redis/PhysicalBridge.cs +++ b/src/StackExchange.Redis/PhysicalBridge.cs @@ -869,14 +869,14 @@ private void CheckBacklogForTimeouts() while (_backlog.TryPeek(out Message? message)) { // See if the message has pass our async timeout threshold - // or has otherwise been completed (e.g. a sync wait timed out) which would have cleared the ResultBox - if (!message.HasTimedOut(now, timeout, out var _) || message.ResultBox == null) break; // not a timeout - we can stop looking + // Note: All timed out messages must be dequeued, even when no completion is needed, to be able to dequeue and complete other timed out messages. + if (!message.HasTimedOut(now, timeout, out var _)) break; // not a timeout - we can stop looking lock (_backlog) { // Peek again since we didn't have lock before... // and rerun the exact same checks as above, note that it may be a different message now if (!_backlog.TryPeek(out message)) break; - if (!message.HasTimedOut(now, timeout, out var _) && message.ResultBox != null) break; + if (!message.HasTimedOut(now, timeout, out var _)) break; if (!BacklogTryDequeue(out var message2) || (message != message2)) // consume it for real { @@ -884,10 +884,15 @@ private void CheckBacklogForTimeouts() } } - // Tell the message it has failed - // Note: Attempting to *avoid* reentrancy/deadlock issues by not holding the lock while completing messages. - var ex = Multiplexer.GetException(WriteResult.TimeoutBeforeWrite, message, ServerEndPoint); - message.SetExceptionAndComplete(ex, this); + // We only handle async timeouts here, synchronous timeouts are handled upstream. + // Those sync timeouts happen in ConnectionMultiplexer.ExecuteSyncImpl() via Monitor.Wait. + if (message.ResultBoxIsAsync) + { + // Tell the message it has failed + // Note: Attempting to *avoid* reentrancy/deadlock issues by not holding the lock while completing messages. + var ex = Multiplexer.GetException(WriteResult.TimeoutBeforeWrite, message, ServerEndPoint); + message.SetExceptionAndComplete(ex, this); + } } } diff --git a/tests/StackExchange.Redis.Tests/Issues/Issue2392Tests.cs b/tests/StackExchange.Redis.Tests/Issues/Issue2392Tests.cs new file mode 100644 index 000000000..fe3e9673d --- /dev/null +++ b/tests/StackExchange.Redis.Tests/Issues/Issue2392Tests.cs @@ -0,0 +1,46 @@ +using System; +using System.Threading.Tasks; +using Xunit; +using Xunit.Abstractions; + +namespace StackExchange.Redis.Tests.Issues +{ + public class Issue2392Tests : TestBase + { + public Issue2392Tests(ITestOutputHelper output) : base(output) { } + + [Fact] + public async Task Execute() + { + var options = new ConfigurationOptions() + { + BacklogPolicy = new() + { + QueueWhileDisconnected = true, + AbortPendingOnConnectionFailure = false, + }, + AbortOnConnectFail = false, + ConnectTimeout = 1, + ConnectRetry = 0, + AsyncTimeout = 1, + SyncTimeout = 1, + AllowAdmin = true, + }; + options.EndPoints.Add("127.0.0.1:1234"); + + using var conn = await ConnectionMultiplexer.ConnectAsync(options, Writer); + var key = Me(); + var db = conn.GetDatabase(); + var server = conn.GetServerSnapshot()[0]; + + // Fail the connection + conn.AllowConnect = false; + server.SimulateConnectionFailure(SimulatedFailureType.All); + Assert.False(conn.IsConnected); + + await db.StringGetAsync(key, flags: CommandFlags.FireAndForget); + var ex = await Assert.ThrowsAnyAsync(() => db.StringGetAsync(key).WithTimeout(5000)); + Assert.True(ex is RedisTimeoutException or RedisConnectionException); + } + } +}