Fix platform-specific TCP keep-alive issues

Directory.Build.props

@@ -38,7 +38,7 @@
     <SourceRoot Include="$(MSBuildThisFileDirectory)/"/>
   </ItemGroup>
 
-  <ItemGroup>
+  <ItemGroup Condition="'$(MSBuildProjectName)' != 'StackExchange.Redis.Build' and '$(MSBuildProjectName)' != 'docker' and '$(MSBuildProjectName)' != 'docs' and '$(MSBuildProjectName)' != '.github'">
     <PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" PrivateAssets="all" />
     <PackageReference Include="StyleCop.Analyzers" PrivateAssets="All" />
   </ItemGroup>

Directory.Packages.props

@@ -39,5 +39,6 @@
     <PackageVersion Include="xunit.v3" Version="3.0.0" />
     <PackageVersion Include="xunit.v3.runner.console" Version="3.0.0" />
     <PackageVersion Include="xunit.runner.visualstudio" Version="3.1.3" />
+    <PackageVersion Include="System.Private.Uri" Version="4.3.2" />
   </ItemGroup>
 </Project>

docs/Configuration.md

@@ -83,6 +83,7 @@ The `ConfigurationOptions` object has a wide range of properties, all of which a
 | configCheckSeconds={int} | `ConfigCheckSeconds` | `60`                         | Time (seconds) to check configuration. This serves as a keep-alive for interactive sockets, if it is supported.     |
 | defaultDatabase={int}  | `DefaultDatabase`      | `null`                       | Default database index, from `0` to `databases - 1`                                                       |
 | keepAlive={int}        | `KeepAlive`            | `-1`                         | Time (seconds) at which to send a message to help keep sockets alive (60 sec default)                     |
+| tcpKeepAlive={bool}    | `TcpKeepAlive`         | `true`                       | Enables TCP keep-alive when appropriate (endpoint- and platform-dependent)                                |
 | name={string}          | `ClientName`           | `null`                       | Identification for the connection within redis                                                            |
 | password={string}      | `Password`             | `null`                       | Password for the redis server                                                                             |
 | user={string}          | `User`                 | `null`                       | User for the redis server (for use with ACLs on redis 6 and above)                                        |

docs/ReleaseNotes.md

@@ -10,6 +10,7 @@ Current package versions:
 
 - Fix logic inversion with `ARGREP NOCASE`, add `IsReversed` to simplify ordering, and support `ARINFO FULL`. ([#3087 by @mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/3087))
 - Avoid sentinel issues if `ROLE` unavailable; fix #3064 ([#3088 by @mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/3088))
+- Fix TCP platform-dependent TCP keep-alive problems, and make an explicit option. ([#3090 by @mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/3090))
 
 ## 2.13.10
 

src/StackExchange.Redis/Configuration/DefaultOptionsProvider.cs

@@ -264,6 +264,11 @@ protected virtual string GetDefaultClientName() =>
         /// </summary>
         public virtual RedisProtocol? Protocol => null;
 
+        /// <summary>
+        /// Gets whether to enable TCP keep-alive when appropriate (endpoint- and platform-dependent).
+        /// </summary>
+        public virtual bool TcpKeepAlive => true;
+
         /// <summary>
         /// Tries to get the RoleInstance Id if Microsoft.WindowsAzure.ServiceRuntime is loaded.
         /// In case of any failure, swallows the exception and returns null.

src/StackExchange.Redis/ConfigurationOptions.cs

@@ -111,7 +111,8 @@ internal const string
                 Tunnel = "tunnel",
                 SetClientLibrary = "setlib",
                 Protocol = "protocol",
-                HighIntegrity = "highIntegrity";
+                HighIntegrity = "highIntegrity",
+                TcpKeepAlive = "tcpKeepAlive";
 
             private static readonly Dictionary<string, string> normalizedOptions = new[]
             {
@@ -132,6 +133,7 @@ internal const string
                 PreserveAsyncOrder,
                 Proxy,
                 ResolveDns,
+                ResponseTimeout,
                 ServiceName,
                 Ssl,
                 SslHost,
@@ -141,8 +143,11 @@ internal const string
                 Version,
                 WriteBuffer,
                 CheckCertificateRevocation,
+                Tunnel,
+                SetClientLibrary,
                 Protocol,
                 HighIntegrity,
+                TcpKeepAlive,
             }.ToDictionary(x => x, StringComparer.OrdinalIgnoreCase);
 
             public static string TryNormalize(string value)
@@ -169,6 +174,7 @@ public static string TryNormalize(string value)
         private Version? defaultVersion;
 
         private int? keepAlive, asyncTimeout, syncTimeout, connectTimeout, responseTimeout, connectRetry, configCheckSeconds;
+        private bool? tcpKeepAlive;
 
         private Proxy? proxy;
 
@@ -596,6 +602,15 @@ public int KeepAlive
             set => keepAlive = value;
         }
 
+        /// <summary>
+        /// Gets or sets whether to enable TCP keep-alive when appropriate (endpoint- and platform-dependent).
+        /// </summary>
+        public bool TcpKeepAlive
+        {
+            get => tcpKeepAlive ?? Defaults.TcpKeepAlive;
+            set => tcpKeepAlive = value;
+        }
+
         /// <summary>
         /// The <see cref="ILoggerFactory"/> to get loggers for connection events.
         /// Note: changes here only affect <see cref="ConnectionMultiplexer"/>s created after.
@@ -847,6 +862,7 @@ public static ConfigurationOptions Parse(string configuration, bool ignoreUnknow
             heartbeatInterval = heartbeatInterval,
             heartbeatConsistencyChecks = heartbeatConsistencyChecks,
             highIntegrity = highIntegrity,
+            tcpKeepAlive = tcpKeepAlive,
         };
 
         /// <summary>
@@ -929,6 +945,7 @@ public string ToString(bool includePassword)
             Append(sb, OptionKeys.SetClientLibrary, setClientLibrary);
             Append(sb, OptionKeys.HighIntegrity, highIntegrity);
             Append(sb, OptionKeys.Protocol, FormatProtocol(_protocol));
+            Append(sb, OptionKeys.TcpKeepAlive, tcpKeepAlive);
             if (Tunnel is { IsInbuilt: true } tunnel)
             {
                 Append(sb, OptionKeys.Tunnel, tunnel.ToString());
@@ -1095,6 +1112,9 @@ private ConfigurationOptions DoParse(string configuration, bool ignoreUnknown)
                         case OptionKeys.HighIntegrity:
                             HighIntegrity = OptionKeys.ParseBoolean(key, value);
                             break;
+                        case OptionKeys.TcpKeepAlive:
+                            TcpKeepAlive = OptionKeys.ParseBoolean(key, value);
+                            break;
                         case OptionKeys.Tunnel:
                             if (value.IsNullOrWhiteSpace())
                             {

src/StackExchange.Redis/PhysicalConnection.cs

@@ -113,15 +113,16 @@ internal async Task BeginConnectAsync(ILogger? log)
             }
 
             Trace("Connecting...");
-            var tunnel = bridge.Multiplexer.RawConfig.Tunnel;
+            var rawConfig = bridge.Multiplexer.RawConfig;
+            var tunnel = rawConfig.Tunnel;
             var connectTo = endpoint;
             if (tunnel is not null)
             {
                 connectTo = await tunnel.GetSocketConnectEndpointAsync(endpoint, CancellationToken.None).ForAwait();
             }
             if (connectTo is not null)
             {
-                _socket = SocketManager.CreateSocket(connectTo);
+                _socket = SocketManager.CreateSocket(connectTo, rawConfig.TcpKeepAlive);
             }
 
             if (_socket is not null)

src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt

@@ -1,4 +1,7 @@
 #nullable enable
+StackExchange.Redis.ConfigurationOptions.TcpKeepAlive.get -> bool
+StackExchange.Redis.ConfigurationOptions.TcpKeepAlive.set -> void
+virtual StackExchange.Redis.Configuration.DefaultOptionsProvider.TcpKeepAlive.get -> bool
 [SER006]StackExchange.Redis.ArrayGrepRequest.IsCaseInsensitive.get -> bool
 [SER006]StackExchange.Redis.ArrayGrepRequest.IsCaseInsensitive.set -> void
 [SER006]StackExchange.Redis.ArrayGrepRequest.IsReversed.get -> bool

src/StackExchange.Redis/SocketManager.cs

@@ -3,6 +3,7 @@
 using System.IO.Pipelines;
 using System.Net;
 using System.Net.Sockets;
+using System.Runtime.InteropServices;
 using System.Threading;
 using Pipelines.Sockets.Unofficial;
 
@@ -215,7 +216,7 @@ private void DisposeRefs()
         /// </summary>
         ~SocketManager() => DisposeRefs();
 
-        internal static Socket CreateSocket(EndPoint endpoint)
+        internal static Socket CreateSocket(EndPoint endpoint, bool tcpKeepAlive)
         {
             var addressFamily = endpoint.AddressFamily;
             var protocolType = addressFamily == AddressFamily.Unix ? ProtocolType.Unspecified : ProtocolType.Tcp;
@@ -224,18 +225,7 @@ internal static Socket CreateSocket(EndPoint endpoint)
                 ? new Socket(SocketType.Stream, protocolType)
                 : new Socket(addressFamily, SocketType.Stream, protocolType);
             SocketConnection.SetRecommendedClientOptions(socket);
-            if (protocolType is ProtocolType.Tcp)
-            {
-                try
-                {
-                    // enable TCP keep-alive (best effort only)
-                    socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true);
-                }
-                catch (Exception ex)
-                {
-                    Debug.WriteLine(ex.Message);
-                }
-            }
+            if (tcpKeepAlive) TryEnableTcpKeepAlive(socket, endpoint);
             return socket;
         }
 
@@ -246,5 +236,39 @@ internal static Socket CreateSocket(EndPoint endpoint)
             var s = SchedulerPool;
             return s == null ? null : $"{s.AvailableCount} of {s.WorkerCount} available";
         }
+
+        internal static bool TryEnableTcpKeepAlive(Socket socket, EndPoint endPoint)
+        {
+            // TCP keep-alive; there's a clue in the name
+            if (socket.ProtocolType is not ProtocolType.Tcp) return false;
+
+            switch (endPoint)
+            {
+#if !NET10_0_OR_GREATER
+                // Prior to .NET 10, enabling TCP keep-alive on host-based endpoints fails outside of Windows.
+                // see https://github.com/StackExchange/StackExchange.Redis/issues/3086
+                case DnsEndPoint when !RuntimeInformation.IsOSPlatform(OSPlatform.Windows): return false;
+#endif
+                case DnsEndPoint:
+                case IPEndPoint:
+                    // fine
+                    break;
+                default:
+                    // don't enable on unexpected endpoint types (unix domain sockets, for example)
+                    return false;
+            }
+
+            try
+            {
+                // enable TCP keep-alive (best effort only)
+                socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true);
+                return true;
+            }
+            catch (Exception ex)
+            {
+                Debug.WriteLine(ex.Message);
+                return false;
+            }
+        }
     }
 }

tests/RedisConfigs/.docker/Redis/Dockerfile

@@ -1,4 +1,4 @@
-ARG CLIENT_LIBS_TEST_IMAGE=redislabs/client-libs-test:8.8-rc1
+ARG CLIENT_LIBS_TEST_IMAGE=redislabs/client-libs-test:8.8.0
 FROM ${CLIENT_LIBS_TEST_IMAGE}
 
 COPY --from=configs ./Basic /data/Basic/

tests/RedisConfigs/docker-compose.yml

@@ -5,7 +5,7 @@ services:
     build:
       context: .docker/Redis
       args:
-        CLIENT_LIBS_TEST_IMAGE: ${CLIENT_LIBS_TEST_IMAGE:-redislabs/client-libs-test:8.8-rc1}
+        CLIENT_LIBS_TEST_IMAGE: ${CLIENT_LIBS_TEST_IMAGE:-redislabs/client-libs-test:8.8.0}
       additional_contexts:
          configs: .
     platform: linux

tests/StackExchange.Redis.Tests/ArrayTests.cs

@@ -126,6 +126,9 @@ public async Task DeleteAndDeleteRange()
     [Fact(Timeout = 10000)]
     public async Task DeleteLastElementPublishesArrayDeleteBeforeKeyDeleteNotifications()
     {
+        #if !DEBUG
+        Assert.Skip("Debug only due to parallelism overhead");
+        #endif
         await using var conn = Create(allowAdmin: true, require: RedisFeatures.v8_8_0);
         var db = conn.GetDatabase();
         await AssertArrayKeyspaceNotificationsEnabledAsync(conn);
@@ -136,6 +139,8 @@ public async Task DeleteLastElementPublishesArrayDeleteBeforeKeyDeleteNotificati
         var sub = conn.GetSubscriber();
         var channel = RedisChannel.Pattern($"__key*@{db.Database}__:*");
         var queue = await sub.SubscribeAsync(channel);
+        await Task.Delay(100);
+        await sub.PingAsync();
         try
         {
             Assert.True(await db.ArraySetAsync(key, 0, "a"));
@@ -629,9 +634,12 @@ private static void AssertValues(RedisValue[] actual, params RedisValue[] expect
 
     private async Task<(KeyNotificationKind Kind, KeyNotificationType Type)> ReadNotificationAsync(ChannelMessageQueue queue, RedisKey key)
     {
-        for (int i = 0; i < 64; i++)
+        // there might be a lot of parallel notifications happening from parallel tests; as such, we might need to skip a lot of unrelated
+        // stuff - allow for the timeout
+        var ct = TestContext.Current.CancellationToken;
+        while (!ct.IsCancellationRequested)
         {
-            var message = await queue.ReadAsync(TestContext.Current.CancellationToken);
+            var message = await queue.ReadAsync(ct);
             if (message.TryParseKeyNotification(out var notification))
             {
                 Log($"{notification.Kind}, {notification.Type} {message}");

tests/StackExchange.Redis.Tests/ClusterTests.cs

@@ -521,6 +521,8 @@ public async Task TransactionWithSameSlotKeys()
     [InlineData("abc", 100)]
     public async Task Keys(string? pattern, int pageSize)
     {
+        NoConcurrentRuntime();
+
         await using var conn = Create(allowAdmin: true);
 
         var dbId = TestConfig.GetDedicatedDB(conn);
@@ -621,6 +623,8 @@ public async Task GetConfig()
     [Fact(Skip = "FlushAllDatabases")]
     public async Task AccessRandomKeys()
     {
+        NoConcurrentRuntime();
+
         await using var conn = Create(allowAdmin: true);
 
         var cluster = conn.GetDatabase();

tests/StackExchange.Redis.Tests/ConfigTests.cs

@@ -79,13 +79,33 @@ orderby name
                 "sslHost",
                 "SslProtocols",
                 "syncTimeout",
+                "tcpKeepAlive",
                 "tieBreaker",
                 "Tunnel",
                 "user",
             },
             fields);
     }
 
+    [Fact]
+    public void OptionKeysAreAllNormalized()
+    {
+        var optionKeys = typeof(ConfigurationOptions).GetNestedType("OptionKeys", BindingFlags.NonPublic)!;
+        var constants = (
+            from field in optionKeys.GetFields(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static)
+            where field.IsLiteral && !field.IsInitOnly && field.FieldType == typeof(string)
+            orderby field.Name
+            select (string)field.GetRawConstantValue()!).ToArray();
+
+        var normalizedOptions = (System.Collections.Generic.IReadOnlyDictionary<string, string>)optionKeys
+            .GetField("normalizedOptions", BindingFlags.NonPublic | BindingFlags.Static)!
+            .GetValue(null)!;
+
+        Assert.Equal(
+            constants.OrderBy(x => x, StringComparer.Ordinal),
+            normalizedOptions.Keys.OrderBy(x => x, StringComparer.Ordinal));
+    }
+
     [Fact]
     public void SslProtocols_SingleValue()
     {

tests/StackExchange.Redis.Tests/DatabaseTests.cs

@@ -67,6 +67,8 @@ public async Task CommandList()
     [Fact]
     public async Task CountKeys()
     {
+        NoConcurrentRuntime();
+
         var db1Id = TestConfig.GetDedicatedDB();
         var db2Id = TestConfig.GetDedicatedDB();
         await using (var conn = Create(allowAdmin: true))

tests/StackExchange.Redis.Tests/HotKeysTests.cs

@@ -15,6 +15,8 @@ public class HotKeysClusterTests(ITestOutputHelper output, SharedConnectionFixtu
     [InlineData(false)]
     public void CanUseClusterFilter(bool sample)
     {
+        NoConcurrentRuntime();
+
         var key = Me();
         using var muxer = GetServer(key, out var server);
         Log($"server: {Format.ToString(server.EndPoint)}, key: '{key}'");
@@ -129,6 +131,8 @@ public async Task StopWhenNotRunningIsFalseAsync()
     [Fact]
     public void CanStartStopReset()
     {
+        NoConcurrentRuntime();
+
         RedisKey key = Me();
         using var muxer = GetServer(key, out var server);
         server.HotKeysStart(duration: Duration);
@@ -215,6 +219,8 @@ private void CheckSimpleWithKey(RedisKey key, HotKeysResult hotKeys, IServer ser
     [Fact]
     public async Task CanStartStopResetAsync()
     {
+        NoConcurrentRuntime();
+
         RedisKey key = Me();
         await using var muxer = GetServer(key, out var server);
         await server.HotKeysStartAsync(duration: Duration);
@@ -244,6 +250,8 @@ public async Task CanStartStopResetAsync()
     [Fact]
     public async Task DurationFilterAsync()
     {
+        NoConcurrentRuntime();
+
         Skip.UnlessLongRunning(); // time-based tests are horrible
 
         RedisKey key = Me();
@@ -276,6 +284,8 @@ public async Task DurationFilterAsync()
     [InlineData(HotKeysMetrics.Network | HotKeysMetrics.Cpu)]
     public async Task MetricsChoiceAsync(HotKeysMetrics metrics)
     {
+        NoConcurrentRuntime();
+
         RedisKey key = Me();
         await using var muxer = GetServer(key, out var server);
         await server.HotKeysStartAsync(metrics, duration: Duration);
@@ -305,6 +315,8 @@ public async Task MetricsChoiceAsync(HotKeysMetrics metrics)
     [Fact]
     public async Task SampleRatioUsageAsync()
     {
+        NoConcurrentRuntime();
+
         RedisKey key = Me();
         await using var muxer = GetServer(key, out var server);
         await server.HotKeysStartAsync(sampleRatio: 3, duration: Duration);