Clean up a slow leak of ping packets when connections are dispose

diff --git a/Hazel/MessageReader.cs b/Hazel/MessageReader.cs
index f73fa4933c23d06616f93920b6b6301b6c40f6b2..1914437227bb6ee30f2204325a46966034633de7 100644 (file)
--- a/Hazel/MessageReader.cs
+++ b/Hazel/MessageReader.cs
@@ -1,10 +1,6 @@
 using System;
-using System.Collections.Generic;
-using System.Diagnostics;
-using System.Linq;
 using System.Runtime.CompilerServices;
 using System.Text;
-using System.Threading;
 
 namespace Hazel
 {

diff --git a/Hazel/ObjectPool.cs b/Hazel/ObjectPool.cs
index 0da7dbf39e3e36e272cff993844d0aff0bd37cf1..1b484159b7b8019b4aff299326eb3b469f028903 100644 (file)
--- a/Hazel/ObjectPool.cs
+++ b/Hazel/ObjectPool.cs
@@ -15,23 +15,22 @@ namespace Hazel
     {
         private int numberCreated;
         public int NumberCreated { get { return numberCreated; } }
+
         public int NumberInUse { get { return this.inuse.Count; } }
+        public int NumberNotInUse { get { return this.pool.Count; } }
 
-        public int Size { get { return this.pool.Count; } }
+        // Available objects
+        private readonly ConcurrentBag<T> pool = new ConcurrentBag<T>();
 
-        /// <summary>
-        ///     Our pool of objects
-        /// </summary>
-        ConcurrentBag<T> pool = new ConcurrentBag<T>();
-
-        private ConcurrentDictionary<T, bool> inuse = new ConcurrentDictionary<T, bool>();
+        // Unavailable objects
+        private readonly ConcurrentDictionary<T, bool> inuse = new ConcurrentDictionary<T, bool>();
 
         /// <summary>
         ///     The generator for creating new objects.
         /// </summary>
         /// <returns></returns>
-        Func<T> objectFactory;
-
+        private readonly Func<T> objectFactory;
+        
         /// <summary>
         ///     Internal constructor for our ObjectPool.
         /// </summary>

diff --git a/Hazel/Udp/UdpConnection.KeepAlive.cs b/Hazel/Udp/UdpConnection.KeepAlive.cs
index 121ae18aaa199337c841864f3400665a6ed63d80..20d1facebd06a09458bf0539b6cbd4eb98db786d 100644 (file)
--- a/Hazel/Udp/UdpConnection.KeepAlive.cs
+++ b/Hazel/Udp/UdpConnection.KeepAlive.cs
@@ -1,9 +1,6 @@
 using System;
 using System.Collections.Concurrent;
-using System.Collections.Generic;
 using System.Diagnostics;
-using System.Linq;
-using System.Text;
 using System.Threading;
 
 
@@ -105,8 +102,9 @@ namespace Hazel.Udp
 
         // Pings are special, quasi-reliable packets. 
         // We send them to trigger responses that validate our connection is alive
-        // They should never be the *cause* of a disconnect.
-        // Rather, the responses will reset our 
+        // An unacked ping should never be the sole cause of a disconnect.
+        // Rather, the responses will reset our pingsSinceAck, enough unacked 
+        // pings should cause a disconnect.
         void SendPing()
         {
             ushort id = (ushort)Interlocked.Increment(ref lastIDAllocated);
@@ -156,6 +154,14 @@ namespace Hazel.Udp
                 this.keepAliveTimer = null;
                 timer.Dispose();
             }
+
+            foreach (var kvp in activePingPackets)
+            {
+                if (this.activePingPackets.TryRemove(kvp.Key, out var pkt))
+                {
+                    pkt.Recycle();
+                }
+            }
         }
     }
 }
\ No newline at end of file

diff --git a/Hazel/Udp/UdpConnection.Reliable.cs b/Hazel/Udp/UdpConnection.Reliable.cs
index a7e915e249aab3c064c655745e95ac6ff1b14206..b18c8c81e318c196353d7cfbd37f8b881888f75e 100644 (file)
--- a/Hazel/Udp/UdpConnection.Reliable.cs
+++ b/Hazel/Udp/UdpConnection.Reliable.cs
@@ -227,13 +227,11 @@ namespace Hazel.Udp
         /// <param name="ackCallback">The callback to make once the packet has been acknowledged.</param>
         void AttachReliableID(byte[] buffer, int offset, int sendLength, Action ackCallback = null)
         {
-            //Find an ID not used yet.
             ushort id = (ushort)Interlocked.Increment(ref lastIDAllocated);
 
             buffer[offset] = (byte)(id >> 8);
             buffer[offset + 1] = (byte)id;
 
-            //Create packet object
             Packet packet = Packet.GetObject();
             packet.Set(
                 id,
@@ -409,9 +407,8 @@ namespace Hazel.Udp
             //Get ID
             ushort id = (ushort)((bytes[1] << 8) + bytes[2]);
 
-            //Dispose of timer and remove from dictionary
-            Packet packet;
-            if (reliableDataPacketsSent.TryRemove(id, out packet))
+            // Dispose of timer and remove from dictionary
+            if (reliableDataPacketsSent.TryRemove(id, out Packet packet))
             {
                 float rt = packet.Stopwatch.ElapsedMilliseconds;
 
@@ -426,12 +423,13 @@ namespace Hazel.Udp
             else if (this.activePingPackets.TryRemove(id, out PingPacket pingPkt))
             {
                 float rt = pingPkt.Stopwatch.ElapsedMilliseconds;
+
+                pingPkt.Recycle();
+
                 lock (PingLock)
                 {
                     this.AveragePingMs = Math.Max(50, this.AveragePingMs * .7f + rt * .3f);
                 }
-
-                pingPkt.Recycle();
             }
 
             Statistics.LogReliableReceive(0, bytes.Length);
@@ -464,8 +462,7 @@ namespace Hazel.Udp
         {
             foreach (var kvp in reliableDataPacketsSent)
             {
-                Packet pkt;
-                if (this.reliableDataPacketsSent.TryRemove(kvp.Key, out pkt))
+                if (this.reliableDataPacketsSent.TryRemove(kvp.Key, out var pkt))
                 {
                     pkt.Recycle();
                 }

diff --git a/Hazel/Udp/UdpConnection.cs b/Hazel/Udp/UdpConnection.cs
index 503b1c6b6b4e4f740ed22234ea3efc5b9ab6e4d8..051214e2348173aecee050f2006d51e2f2635478 100644 (file)
--- a/Hazel/Udp/UdpConnection.cs
+++ b/Hazel/Udp/UdpConnection.cs
@@ -34,7 +34,6 @@ namespace Hazel.Udp
         /// <inheritdoc/>
         public override void Send(MessageWriter msg)
         {
-            //Early check
             if (this._state != ConnectionState.Connected)
                 throw new InvalidOperationException("Could not send data as this Connection is not connected. Did you disconnect?");
 

diff --git a/Hazel/Udp/UdpConnectionListener.cs b/Hazel/Udp/UdpConnectionListener.cs
index 9a8a3d93bc80fa8975c5ebe8d8e637cf3c53b7b6..e1d6f46927e89ec2323446910d0adcb072320645 100644 (file)
--- a/Hazel/Udp/UdpConnectionListener.cs
+++ b/Hazel/Udp/UdpConnectionListener.cs
@@ -115,8 +115,6 @@ namespace Hazel.Udp
             }
             catch (SocketException)
             {
-                //Client no longer reachable, pretend it didn't happen
-                //TODO possibly able to disconnect client, see other TODO
                 message?.Recycle();
                 StartListeningForData();
                 return;
@@ -124,6 +122,7 @@ namespace Hazel.Udp
             catch (Exception ex)
             {
                 //If the socket's been disposed then we can just end there.
+                message.Recycle();
                 this.Logger?.Invoke("Stopped due to: " + ex.Message);
                 return;
             }
@@ -169,6 +168,7 @@ namespace Hazel.Udp
             catch (Exception ex)
             {
                 //If the socket's been disposed then we can just end there.
+                message.Recycle();
                 this.Logger?.Invoke("Stopped due to: " + ex.Message);
                 return;
             }

diff --git a/Hazel/Udp/UdpServerConnection.cs b/Hazel/Udp/UdpServerConnection.cs
index 4c18a6e41741ac70071e6c1fb1aa5897672a4afa..fa15edbcba8c7243e009cac4b2ef36c07b8950bd 100644 (file)
--- a/Hazel/Udp/UdpServerConnection.cs
+++ b/Hazel/Udp/UdpServerConnection.cs
@@ -85,8 +85,8 @@ namespace Hazel.Udp
                 if (this._state == ConnectionState.Connected
                     || this._state == ConnectionState.Disconnecting)
                 {
-                    SendDisconnect();
                     this._state = ConnectionState.NotConnected;
+                    SendDisconnect();
                 }
             }