From 44c69620a62cfd89532c7d6b7b9c0947848009cc Mon Sep 17 00:00:00 2001
From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com>
Date: Tue, 7 Feb 2023 21:22:04 -0800
Subject: [PATCH] fix: Tidy up socket dispose. (#1334)

---
 Projects/Server/Network/NetState/NetState.cs  | 13 ++--
 .../Server/Network/Packets/OutgoingPackets.cs |  6 +-
 Projects/Server/Utilities/Utility.cs          | 75 +++++++++----------
 3 files changed, 46 insertions(+), 48 deletions(-)

diff --git a/Projects/Server/Network/NetState/NetState.cs b/Projects/Server/Network/NetState/NetState.cs
index 4a7602995..8f34dc9bf 100755
--- a/Projects/Server/Network/NetState/NetState.cs
+++ b/Projects/Server/Network/NetState/NetState.cs
@@ -1091,6 +1091,12 @@ public partial class NetState : IComparable<NetState>
             }
 #endif
 
+        var m = Mobile;
+        if (m?.NetState == this)
+        {
+            m.NetState = null;
+        }
+
         TcpServer.Instances.Remove(this);
         try
         {
@@ -1104,14 +1110,8 @@ public partial class NetState : IComparable<NetState>
         Connection.Close();
         _handle.Free();
 
-        var m = Mobile;
         Mobile = null;
 
-        if (m?.NetState == this)
-        {
-            m.NetState = null;
-        }
-
         var a = Account;
 
         Gumps.Clear();
@@ -1120,6 +1120,7 @@ public partial class NetState : IComparable<NetState>
         Account = null;
         ServerInfo = null;
         CityInfo = null;
+        Connection = null;
 
         var count = TcpServer.Instances.Count;
 
diff --git a/Projects/Server/Network/Packets/OutgoingPackets.cs b/Projects/Server/Network/Packets/OutgoingPackets.cs
index a20356fe9..18e95f796 100644
--- a/Projects/Server/Network/Packets/OutgoingPackets.cs
+++ b/Projects/Server/Network/Packets/OutgoingPackets.cs
@@ -5,5 +5,9 @@ namespace Server.Network;
 public static class OutgoingPackets
 {
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    public static bool CannotSendPackets(this NetState ns) => ns?.Connection == null || ns.BlockAllPackets;
+    public static bool CannotSendPackets(this NetState ns) =>
+        // Do not check for NetState.Running. Packets are sent to a "disconnected" socket as part of the OnDisconnect events
+        // up until the Connection is nulled. Closing the connection is done synchronously, therefore packets will not be sent
+        // once the Mobile.NetState is null.
+        ns?.Connection == null || ns.BlockAllPackets;
 }
diff --git a/Projects/Server/Utilities/Utility.cs b/Projects/Server/Utilities/Utility.cs
index a7d5210b9..2a533e850 100644
--- a/Projects/Server/Utilities/Utility.cs
+++ b/Projects/Server/Utilities/Utility.cs
@@ -6,6 +6,7 @@ using System.IO;
 using System.Net;
 using System.Net.Sockets;
 using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
 using System.Text;
 using System.Xml;
 using Microsoft.Toolkit.HighPerformance;
@@ -1455,74 +1456,66 @@ public static class Utility
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static bool Remove<T>(ref List<T> list, T value)
     {
-        if (list != null)
+        if (list?.Remove(value) != true)
         {
-            var removed = list.Remove(value);
-
-            if (list.Count == 0)
-            {
-                list = null;
-            }
-
-            return removed;
+            return false;
         }
 
-        return false;
+        if (list.Count == 0)
+        {
+            list = null;
+        }
+
+        return true;
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static bool Remove<T>(ref HashSet<T> set, T value)
     {
-        if (set != null)
+        if (set?.Remove(value) != true)
         {
-            var removed = set.Remove(value);
-
-            if (set.Count == 0)
-            {
-                set = null;
-            }
-
-            return removed;
+            return false;
         }
 
-        return false;
+        if (set.Count == 0)
+        {
+            set = null;
+        }
+
+        return true;
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static bool Remove<K, V>(ref Dictionary<K, V> dict, K key)
     {
-        if (dict != null)
+        if (dict?.Remove(key) != true)
         {
-            var removed = dict.Remove(key);
-
-            if (dict.Count == 0)
-            {
-                dict = null;
-            }
-
-            return removed;
+            return false;
         }
 
-        return false;
+        if (dict.Count == 0)
+        {
+            dict = null;
+        }
+
+        return true;
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static bool Remove<K, V>(ref Dictionary<K, V> dict, K key, out V value)
     {
-        if (dict != null)
+        if (dict?.Remove(key, out value) != true)
         {
-            var removed = dict.Remove(key, out value);
-
-            if (dict.Count == 0)
-            {
-                dict = null;
-            }
-
-            return removed;
+            value = default;
+            return false;
         }
 
-        value = default;
-        return false;
+        if (dict.Count == 0)
+        {
+            dict = null;
+        }
+
+        return true;
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]