make sure PhysicalConnection doesn't root the multiplexer; this, however,...

make sure PhysicalConnection doesn't root the multiplexer; this, however, means that every check to the bridge needs to be null-checked

StackExchange.Redis.Tests/GarbageCollectionTests.cs

@@ -42,8 +42,6 @@ public void MuxerIsCollected()
 
             int after = ConnectionMultiplexer.CollectedWithoutDispose;
 
-
-            Thread.Sleep(TimeSpan.FromSeconds(60));
             Assert.Null(wr.Target);
             Assert.Equal(before + 1, after);
         }

StackExchange.Redis/StackExchange/Redis/DebuggingAids.cs

@@ -77,13 +77,14 @@ internal partial class PhysicalConnection
 
         partial void OnDebugAbort()
         {
-            if (!Multiplexer.AllowConnect)
+            var bridge = BridgeCouldBeNull;
+            if (bridge == null || !bridge.Multiplexer.AllowConnect)
             {
                 throw new RedisConnectionException(ConnectionFailureType.InternalFailure, "debugging");
             }
         }
 
-        public bool IgnoreConnect => Multiplexer.IgnoreConnect;
+        public bool IgnoreConnect => BridgeCouldBeNull?.Multiplexer?.IgnoreConnect ?? false;
 
         private static volatile bool emulateStaleConnection;
         public static bool EmulateStaleConnection

StackExchange.Redis/StackExchange/Redis/Message.cs

@@ -35,7 +35,8 @@ protected override void WriteImpl(PhysicalConnection physical)
         {
             try
             {
-                physical.Multiplexer.LogLocked(log, "Writing to {0}: {1}", physical.Bridge, tail.CommandAndKey);
+                var bridge = physical.BridgeCouldBeNull;
+                bridge?.Multiplexer?.LogLocked(log, "Writing to {0}: {1}", bridge, tail.CommandAndKey);
             }
             catch { }
             tail.WriteTo(physical);

StackExchange.Redis/StackExchange/Redis/PhysicalBridge.cs

@@ -98,8 +98,19 @@ public void Dispose()
             {
                 physical = null;
             }
+            GC.SuppressFinalize(this);
+        }
+        ~PhysicalBridge()
+        {
+            // shouldn't *really* touch managed objects
+            // in a finalizer, but we need to kill that socket,
+            // and this is the first place that isn't going to
+            // be rooted by the socket async bits
+            try {
+                var tmp = physical;
+                tmp?.Shutdown();
+            } catch { }
         }
-
         public void ReportNextFailure()
         {
             reportNextFailure = true;
@@ -399,7 +410,7 @@ internal void OnHeartbeat(bool ifConnectedOnly)
                             if (state == (int)State.ConnectedEstablished)
                             {
                                 Interlocked.Exchange(ref connectTimeoutRetryCount, 0);
-                                tmp.Bridge.ServerEndPoint.ClearUnselectable(UnselectableFlags.DidNotRespond);
+                                tmp.BridgeCouldBeNull?.ServerEndPoint?.ClearUnselectable(UnselectableFlags.DidNotRespond);
                             }
                             tmp.OnBridgeHeartbeat();
                             int writeEverySeconds = ServerEndPoint.WriteEverySeconds,

StackExchange.Redis/StackExchange/Redis/RedisDatabase.cs

@@ -3303,11 +3303,13 @@ public override int GetHashSlot(ServerSelectionStrategy serverSelectionStrategy)
 
             public IEnumerable<Message> GetMessages(PhysicalConnection connection)
             {
-                if (script != null && connection.Multiplexer.CommandMap.IsAvailable(RedisCommand.SCRIPT)
+                PhysicalBridge bridge;
+                if (script != null && (bridge = connection.BridgeCouldBeNull) != null
+                    && bridge.Multiplexer.CommandMap.IsAvailable(RedisCommand.SCRIPT)
                     && (Flags & CommandFlags.NoScriptCache) == 0)
                 {
                     // a script was provided (rather than a hash); check it is known and supported
-                    asciiHash = connection.Bridge.ServerEndPoint.GetScriptHash(script, command);
+                    asciiHash = bridge.ServerEndPoint.GetScriptHash(script, command);
 
                     if (asciiHash == null)
                     {

StackExchange.Redis/StackExchange/Redis/RedisTransaction.cs

@@ -223,8 +223,11 @@ public IEnumerable<Message> GetMessages(PhysicalConnection connection)
                     // up-version servers, pre-condition failures exit with UNWATCH; and on down-version servers pre-condition
                     // failures exit with DISCARD - but that's ok : both work fine
 
-                    bool explicitCheckForQueued = !connection.Bridge.ServerEndPoint.GetFeatures().ExecAbort;
-                    var multiplexer = connection.Multiplexer;
+                    var bridge = connection.BridgeCouldBeNull;
+                    if (bridge == null) throw new ObjectDisposedException(connection.ToString());
+
+                    bool explicitCheckForQueued = !bridge.ServerEndPoint.GetFeatures().ExecAbort;
+                    var multiplexer = bridge.Multiplexer;
 
                     // PART 1: issue the pre-conditions
                     if (!IsAborted && conditions.Length != 0)
@@ -332,15 +335,15 @@ public IEnumerable<Message> GetMessages(PhysicalConnection connection)
                 }
                 if (IsAborted)
                 {
-                    connection.Multiplexer.Trace("Aborting: canceling wrapped messages");
-                    var bridge = connection.Bridge;
+                    connection.Trace("Aborting: canceling wrapped messages");
+                    var bridge = connection.BridgeCouldBeNull;
                     foreach (var op in InnerOperations)
                     {
                         op.Wrapped.Cancel();
-                        bridge.CompleteSyncOrAsync(op.Wrapped);
+                        bridge?.CompleteSyncOrAsync(op.Wrapped);
                     }
                 }
-                connection.Multiplexer.Trace("End ot transaction: " + Command);
+                connection.Trace("End of transaction: " + Command);
                 yield return this; // acts as either an EXEC or an UNWATCH, depending on "aborted"
             }
 
@@ -378,11 +381,11 @@ public override bool SetResult(PhysicalConnection connection, Message message, R
                 if (result.IsError && message is TransactionMessage tran)
                 {
                     string error = result.GetString();
-                    var bridge = connection.Bridge;
+                    var bridge = connection.BridgeCouldBeNull;
                     foreach (var op in tran.InnerOperations)
                     {
                         ServerFail(op.Wrapped, error);
-                        bridge.CompleteSyncOrAsync(op.Wrapped);
+                        bridge?.CompleteSyncOrAsync(op.Wrapped);
                     }
                 }
                 return base.SetResult(connection, message, result);
@@ -392,26 +395,26 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes
             {
                 if (message is TransactionMessage tran)
                 {
-                    var bridge = connection.Bridge;
+                    var bridge = connection.BridgeCouldBeNull;
                     var wrapped = tran.InnerOperations;
                     switch (result.Type)
                     {
                         case ResultType.SimpleString:
                             if (tran.IsAborted && result.IsEqual(RedisLiterals.BytesOK))
                             {
-                                connection.Multiplexer.Trace("Acknowledging UNWATCH (aborted electively)");
+                                connection.Trace("Acknowledging UNWATCH (aborted electively)");
                                 SetResult(message, false);
                                 return true;
                             }
                             //EXEC returned with a NULL
                             if (!tran.IsAborted && result.IsNull)
                             {
-                                connection.Multiplexer.Trace("Server aborted due to failed EXEC");
+                                connection.Trace("Server aborted due to failed EXEC");
                                 //cancel the commands in the transaction and mark them as complete with the completion manager
                                 foreach (var op in wrapped)
                                 {
                                     op.Wrapped.Cancel();
-                                    bridge.CompleteSyncOrAsync(op.Wrapped);
+                                    bridge?.CompleteSyncOrAsync(op.Wrapped);
                                 }
                                 SetResult(message, false);
                                 return true;
@@ -423,23 +426,23 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes
                                 var arr = result.GetItems();
                                 if (result.IsNull)
                                 {
-                                    connection.Multiplexer.Trace("Server aborted due to failed WATCH");
+                                    connection.Trace("Server aborted due to failed WATCH");
                                     foreach (var op in wrapped)
                                     {
                                         op.Wrapped.Cancel();
-                                        bridge.CompleteSyncOrAsync(op.Wrapped);
+                                        bridge?.CompleteSyncOrAsync(op.Wrapped);
                                     }
                                     SetResult(message, false);
                                     return true;
                                 }
                                 else if (wrapped.Length == arr.Length)
                                 {
-                                    connection.Multiplexer.Trace("Server committed; processing nested replies");
+                                    connection.Trace("Server committed; processing nested replies");
                                     for (int i = 0; i < arr.Length; i++)
                                     {
                                         if (wrapped[i].Wrapped.ComputeResult(connection, arr[i]))
                                         {
-                                            bridge.CompleteSyncOrAsync(wrapped[i].Wrapped);
+                                            bridge?.CompleteSyncOrAsync(wrapped[i].Wrapped);
                                         }
                                     }
                                     SetResult(message, true);

StackExchange.Redis/StackExchange/Redis/ResultProcessor.cs

@@ -166,18 +166,19 @@ public void SetException(Message message, Exception ex)
         // true if ready to be completed (i.e. false if re-issued to another server)
         public virtual bool SetResult(PhysicalConnection connection, Message message, RawResult result)
         {
+            var bridge = connection.BridgeCouldBeNull;
             if (message is LoggingMessage logging)
             {
                 try
                 {
-                    connection.Multiplexer.LogLocked(logging.Log, "Response from {0} / {1}: {2}", connection.Bridge, message.CommandAndKey, result);
+                    bridge?.Multiplexer?.LogLocked(logging.Log, "Response from {0} / {1}: {2}", bridge, message.CommandAndKey, result);
                 }
                 catch { }
             }
             if (result.IsError)
             {
-                if (result.AssertStarts(NOAUTH)) connection?.Multiplexer?.SetAuthSuspect();
-                var bridge = connection.Bridge;
+                if (result.AssertStarts(NOAUTH)) bridge?.Multiplexer?.SetAuthSuspect();
+                
                 var server = bridge.ServerEndPoint;
                 bool log = !message.IsInternalCall;
                 bool isMoved = result.AssertStarts(MOVED);
@@ -196,9 +197,11 @@ public virtual bool SetResult(PhysicalConnection connection, Message message, Ra
                         // no point sending back to same server, and no point sending to a dead server
                         if (!Equals(server.EndPoint, endpoint))
                         {
-                            if (bridge.Multiplexer.TryResend(hashSlot, message, endpoint, isMoved))
+                            if (bridge == null)
+                            { } // already toast
+                            else if (bridge.Multiplexer.TryResend(hashSlot, message, endpoint, isMoved))
                             {
-                                connection.Multiplexer.Trace(message.Command + " re-issued to " + endpoint, isMoved ? "MOVED" : "ASK");
+                                bridge.Multiplexer.Trace(message.Command + " re-issued to " + endpoint, isMoved ? "MOVED" : "ASK");
                                 return false;
                             }
                             else
@@ -227,7 +230,7 @@ public virtual bool SetResult(PhysicalConnection connection, Message message, Ra
                 {
                     bridge.Multiplexer.OnErrorMessage(server.EndPoint, err);
                 }
-                connection.Multiplexer.Trace("Completed with error: " + err + " (" + GetType().Name + ")", ToString());
+                bridge?.Multiplexer?.Trace("Completed with error: " + err + " (" + GetType().Name + ")", ToString());
                 if (unableToConnectError)
                 {
                     ConnectionFail(message, ConnectionFailureType.UnableToConnect, err);
@@ -242,7 +245,7 @@ public virtual bool SetResult(PhysicalConnection connection, Message message, Ra
                 bool coreResult = SetResultCore(connection, message, result);
                 if (coreResult)
                 {
-                    connection.Multiplexer.Trace("Completed with success: " + result.ToString() + " (" + GetType().Name + ")", ToString());
+                    bridge?.Multiplexer?.Trace("Completed with success: " + result.ToString() + " (" + GetType().Name + ")", ToString());
                 }
                 else
                 {
@@ -481,7 +484,7 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes
                         var sl = message as RedisDatabase.ScriptLoadMessage;
                         if (sl != null)
                         {
-                            connection.Bridge.ServerEndPoint.AddScript(sl.Script, asciiHash);
+                            connection.BridgeCouldBeNull?.ServerEndPoint?.AddScript(sl.Script, asciiHash);
                         }
                         SetResult(message, hash);
                         return true;
@@ -561,16 +564,21 @@ public override bool SetResult(PhysicalConnection connection, Message message, R
             {
                 if (result.IsError && result.AssertStarts(READONLY))
                 {
-                    var server = connection.Bridge.ServerEndPoint;
-                    server.Multiplexer.Trace("Auto-configured role: slave");
-                    server.IsSlave = true;
+                    var bridge = connection.BridgeCouldBeNull;
+                    if(bridge != null)
+                    {
+                        var server = bridge.ServerEndPoint;
+                        server.Multiplexer.Trace("Auto-configured role: slave");
+                        server.IsSlave = true;
+                    }                    
                 }
                 return base.SetResult(connection, message, result);
             }
 
             protected override bool SetResultCore(PhysicalConnection connection, Message message, RawResult result)
             {
-                var server = connection.Bridge.ServerEndPoint;
+                var server = connection.BridgeCouldBeNull?.ServerEndPoint;
+                if (server == null) return false;
                 switch (result.Type)
                 {
                     case ResultType.BulkString:
@@ -777,8 +785,10 @@ private sealed class ClusterNodesProcessor : ResultProcessor<ClusterConfiguratio
         {
             internal static ClusterConfiguration Parse(PhysicalConnection connection, string nodes)
             {
-                var server = connection.Bridge.ServerEndPoint;
-                var config = new ClusterConfiguration(connection.Multiplexer.ServerSelectionStrategy, nodes, server.EndPoint);
+                var bridge = connection.BridgeCouldBeNull;
+                if (bridge == null) throw new ObjectDisposedException(connection.ToString());
+                var server = bridge.ServerEndPoint;
+                var config = new ClusterConfiguration(bridge.Multiplexer.ServerSelectionStrategy, nodes, server.EndPoint);
                 server.SetClusterConfiguration(config);
                 return config;
             }
@@ -789,7 +799,8 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes
                 {
                     case ResultType.BulkString:
                         string nodes = result.GetString();
-                        connection.Bridge.ServerEndPoint.ServerType = ServerType.Cluster;
+                        var bridge = connection.BridgeCouldBeNull;
+                        if (bridge != null) bridge.ServerEndPoint.ServerType = ServerType.Cluster;
                         var config = Parse(connection, nodes);
                         SetResult(message, config);
                         return true;
@@ -823,7 +834,7 @@ private sealed class ConnectionIdentityProcessor : ResultProcessor<EndPoint>
         {
             protected override bool SetResultCore(PhysicalConnection connection, Message message, RawResult result)
             {
-                SetResult(message, connection.Bridge.ServerEndPoint.EndPoint);
+                SetResult(message, connection.BridgeCouldBeNull?.ServerEndPoint?.EndPoint);
                 return true;
             }
         }
@@ -918,7 +929,7 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes
                     SetResult(message, true);
                     return true;
                 }
-                if(message.Command == RedisCommand.AUTH) connection?.Multiplexer?.SetAuthSuspect();
+                if(message.Command == RedisCommand.AUTH) connection?.BridgeCouldBeNull?.Multiplexer?.SetAuthSuspect();
                 return false;
             }
         }
@@ -1312,7 +1323,7 @@ public override bool SetResult(PhysicalConnection connection, Message message, R
             {
                 if (result.Type == ResultType.Error && result.AssertStarts(NOSCRIPT))
                 { // scripts are not flushed individually, so assume the entire script cache is toast ("SCRIPT FLUSH")
-                    connection.Bridge.ServerEndPoint.FlushScriptCache();
+                    connection.BridgeCouldBeNull?.ServerEndPoint?.FlushScriptCache();
                     message.SetScriptUnavailable();
                 }
                 // and apply usual processing for the rest
@@ -1837,7 +1848,7 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes
                 switch (message.Command)
                 {
                     case RedisCommand.ECHO:
-                        happy = result.Type == ResultType.BulkString && (!establishConnection || result.IsEqual(connection.Multiplexer.UniqueId));
+                        happy = result.Type == ResultType.BulkString && (!establishConnection || result.IsEqual(connection.BridgeCouldBeNull?.Multiplexer?.UniqueId));
                         break;
                     case RedisCommand.PING:
                         happy = result.Type == ResultType.SimpleString && result.IsEqual(RedisLiterals.BytesPONG);
@@ -1854,7 +1865,7 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes
                 }
                 if (happy)
                 {
-                    if (establishConnection) connection.Bridge.OnFullyEstablished(connection);
+                    if (establishConnection) connection.BridgeCouldBeNull?.OnFullyEstablished(connection);
                     SetResult(message, happy);
                     return true;
                 }

StackExchange.Redis/StackExchange/Redis/ServerEndPoint.cs

@@ -480,12 +480,15 @@ internal void OnFullyEstablished(PhysicalConnection connection)
             try
             {
                 if (connection == null) return;
-                var bridge = connection.Bridge;
-                if (bridge == subscription)
+                var bridge = connection.BridgeCouldBeNull;
+                if (bridge != null)
                 {
-                    Multiplexer.ResendSubscriptions(this);
+                    if (bridge == subscription)
+                    {
+                        Multiplexer.ResendSubscriptions(this);
+                    }
+                    Multiplexer.OnConnectionRestored(EndPoint, bridge.ConnectionType);
                 }
-                Multiplexer.OnConnectionRestored(EndPoint, bridge.ConnectionType);
             }
             catch (Exception ex)
             {
@@ -630,7 +633,16 @@ internal void WriteDirectOrQueueFireAndForget<T>(PhysicalConnection connection,
                 else
                 {
                     Multiplexer.Trace("Writing direct: " + message);
-                    connection.Bridge.WriteMessageTakingWriteLock(connection, message);
+                    var bridge = connection.BridgeCouldBeNull;
+                    if (bridge == null)
+                    {
+                        throw new ObjectDisposedException(connection.ToString());
+                    }
+                    else
+                    {
+                        bridge.WriteMessageTakingWriteLock(connection, message);
+                    }
+                    
                 }
             }
         }
@@ -676,14 +688,19 @@ private Task HandshakeAsync(PhysicalConnection connection, TextWriter log)
                 }
             }
 
-            var connType = connection.Bridge.ConnectionType;
+            var bridge = connection.BridgeCouldBeNull;
+            if (bridge == null)
+            {
+                return Task.CompletedTask;
+            }
+            var connType = bridge.ConnectionType;
 
             if (connType == ConnectionType.Interactive)
             {
                 Multiplexer.LogLocked(log, "Auto-configure...");
                 AutoConfigure(connection);
             }
-            Multiplexer.LogLocked(log, "Sending critical tracer: {0}", connection.Bridge);
+            Multiplexer.LogLocked(log, "Sending critical tracer: {0}", bridge);
             var tracer = GetTracerMessage(true);
             tracer = LoggingMessage.Create(log, tracer);
             WriteDirectOrQueueFireAndForget(connection, tracer, ResultProcessor.EstablishConnection);