Managed SNI prevent orphaned active packets being GC'ed without clear (…

…#888)
dotnet · Feb 1, 2021 · 951eb6d · 951eb6d
1 parent 9e3f0d6
commit 951eb6d
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 9 deletions.
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.cs
@@ -2,7 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-// #define TRACE_HISTORY // this is used for advanced debugging when you need to trace the entire lifetime of a single packet, be very careful with it
+ // #define TRACE_HISTORY // this is used for advanced debugging when you need to trace the entire lifetime of a single packet, be very careful with it
 
 using System;
 using System.Buffers;
@@ -41,15 +41,15 @@ internal struct History
         {
             public enum Direction
             {
-                Rent=0,
-                Return=1,
+                Rent = 0,
+                Return = 1,
             }
 
             public Direction Action;
             public int RefCount;
             public string Stack;
         }
-
+      
         internal List<History> _history = null;
 
         /// <summary>
@@ -62,7 +62,7 @@ public SNIPacket(SNIHandle owner,int id)
             : this()
         {
 #if TRACE_HISTORY
-            _history = new List<Activity>();
+            _history = new List<History>();
 #endif
             _id = id;
             _owner = owner;

diff --git a/...rosoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs b/...rosoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs
@@ -71,14 +71,34 @@ internal override void AssignPendingDNSInfo(string userProtocol, string DNSCache
 
         internal void ReadAsyncCallback(SNIPacket packet, uint error)
         {
-            ReadAsyncCallback(IntPtr.Zero, PacketHandle.FromManagedPacket(packet), error);
-            _sessionHandle?.ReturnPacket(packet);
+            SNIHandle sessionHandle = _sessionHandle;
+            if (sessionHandle != null)
+            {
+                ReadAsyncCallback(IntPtr.Zero, PacketHandle.FromManagedPacket(packet), error);
+                sessionHandle?.ReturnPacket(packet);
+            }
+            else
+            {
+                // clear the packet and drop it to GC because we no longer know how to return it to the correct owner
+                // this can only happen if a packet is in-flight when the _sessionHandle is cleared
+                packet.Release();
+            }
         }
 
         internal void WriteAsyncCallback(SNIPacket packet, uint sniError)
         {
-            WriteAsyncCallback(IntPtr.Zero, PacketHandle.FromManagedPacket(packet), sniError);
-            _sessionHandle?.ReturnPacket(packet);
+            SNIHandle sessionHandle = _sessionHandle;
+            if (sessionHandle != null)
+            {
+                WriteAsyncCallback(IntPtr.Zero, PacketHandle.FromManagedPacket(packet), sniError);
+                sessionHandle?.ReturnPacket(packet);
+            }
+            else
+            {
+                // clear the packet and drop it to GC because we no longer know how to return it to the correct owner
+                // this can only happen if a packet is in-flight when the _sessionHandle is cleared
+                packet.Release();
+            }
         }
 
         protected override void RemovePacketFromPendingList(PacketHandle packet)