Skip to content

Commit e0424b2

Browse files
committed
fix RESP3 handshakes
1 parent 1286481 commit e0424b2

4 files changed

Lines changed: 19 additions & 8 deletions

File tree

src/StackExchange.Redis/ResultProcessor.cs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2892,12 +2892,20 @@ public override bool SetResult(PhysicalConnection connection, Message message, i
28922892
}
28932893
}
28942894

2895+
// We need to SetResult before OnFullyEstablished so that the
2896+
// protocol is reliably known *before* we do next-steps.
28952897
if (connection.Protocol is null)
28962898
{
28972899
// if we didn't get a valid response from HELLO, then we have to assume RESP2 at some point
28982900
connection.SetProtocol(RedisProtocol.Resp2);
28992901
}
29002902

2903+
if (final & establishConnection)
2904+
{
2905+
// This is what ultimately brings us to complete a connection, by advancing the state forward from a successful tracer after connection.
2906+
connection.BridgeCouldBeNull?.OnFullyEstablished(connection, $"From command: {message.Command}");
2907+
}
2908+
29012909
return final;
29022910
}
29032911

@@ -2939,11 +2947,6 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes
29392947
}
29402948
if (happy)
29412949
{
2942-
if (establishConnection)
2943-
{
2944-
// This is what ultimately brings us to complete a connection, by advancing the state forward from a successful tracer after connection.
2945-
connection.BridgeCouldBeNull?.OnFullyEstablished(connection, $"From command: {message.Command}");
2946-
}
29472950
SetResult(message, happy);
29482951
return true;
29492952
}

src/StackExchange.Redis/ServerEndPoint.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -695,14 +695,19 @@ internal void OnFullyEstablished(PhysicalConnection connection, string source)
695695
// Clear the unselectable flag ASAP since we are open for business
696696
ClearUnselectable(UnselectableFlags.DidNotRespond);
697697

698-
bool isResp3 = KnowOrAssumeResp3();
698+
bool isResp3 = connection?.Protocol is >= RedisProtocol.Resp3;
699699
if (bridge == subscription || isResp3)
700700
{
701701
// Note: this MUST be fire and forget, because we might be in the middle of a Sync processing
702702
// TracerProcessor which is executing this line inside a SetResultCore().
703703
// Since we're issuing commands inside a SetResult path in a message, we'd create a deadlock by waiting.
704704
Multiplexer.EnsureSubscriptions(CommandFlags.FireAndForget);
705705
}
706+
else if (SupportsSubscriptions && Multiplexer.RawConfig.Protocol > RedisProtocol.Resp2)
707+
{
708+
// interactive, and we wanted RESP3+, but we didn't get it; spin up pub/sub
709+
Activate(ConnectionType.Subscription, null);
710+
}
706711
if (IsConnected && (IsSubscriberConnected || !SupportsSubscriptions || isResp3))
707712
{
708713
// Only connect on the second leg - we can accomplish this by checking both

tests/StackExchange.Redis.Tests/InProcessTestServer.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ namespace StackExchange.Redis.Tests;
1717
public class InProcessTestServer : MemoryCacheRedisServer
1818
{
1919
private readonly ITestOutputHelper? _log;
20-
public InProcessTestServer(ITestOutputHelper? log = null)
20+
public InProcessTestServer(ITestOutputHelper? log = null, EndPoint? endpoint = null)
21+
: base(endpoint)
2122
{
2223
RedisVersion = RedisFeatures.v6_0_0; // for client to expect RESP3
2324
_log = log;

tests/StackExchange.Redis.Tests/Resp3HandshakeTests.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Concurrent;
33
using System.Collections.Generic;
4+
using System.Net;
45
using System.Threading.Tasks;
56
using StackExchange.Redis.Server;
67
using Xunit;
@@ -99,8 +100,9 @@ public async Task Handshake(RedisProtocol client, ServerResponse server, Handsha
99100
}
100101
}
101102

103+
private static readonly EndPoint EP = new DnsEndPoint("home", 8000);
102104
private sealed class HandshakeServer(ServerResponse response, ITestOutputHelper log)
103-
: InProcessTestServer(log)
105+
: InProcessTestServer(log, EP)
104106
{
105107
protected override RedisProtocol MaxProtocol => response switch
106108
{

0 commit comments

Comments
 (0)