Merge branch 'merge/packethandle-follow-up' into merge/tdsparserstateobject-lifetimes

Author: edwardneal

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs

@@ -86,8 +86,6 @@ internal abstract void CreatePhysicalSNIHandle(
 
         internal abstract void AssignPendingDNSInfo(string userProtocol, string DNSCacheKey, ref SQLDNSInfo pendingDNSInfo);
 
-        internal abstract bool IsFailedHandle();
-
         protected abstract void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async);
 
         protected abstract void FreeGcHandle(int remaining, bool release);
@@ -98,28 +96,8 @@ internal abstract void CreatePhysicalSNIHandle(
 
         internal abstract void Dispose();
 
-        internal abstract void DisposePacketCache();
-
-        internal abstract bool IsPacketEmpty(PacketHandle readPacket);
-
-        internal abstract PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint error);
-
-        internal abstract PacketHandle ReadAsync(SessionHandle handle, out uint error);
-
         internal abstract uint CheckConnection();
 
-        internal abstract void ReleasePacket(PacketHandle syncReadPacket);
-
-        protected abstract uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize);
-
-        internal abstract PacketHandle GetResetWritePacket(int dataSize);
-
-        internal abstract void ClearAllWritePackets();
-
-        internal abstract PacketHandle AddPacketToPendingList(PacketHandle packet);
-
-        protected abstract void RemovePacketFromPendingList(PacketHandle pointer);
-
         internal int DecrementPendingCallbacks(bool release)
         {
             int remaining = Interlocked.Decrement(ref _pendingCallbacks);
@@ -412,8 +390,6 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error)
             }
         }
 
-        protected abstract bool CheckPacket(PacketHandle packet, TaskCompletionSource<object> source);
-
         public void WriteAsyncCallback(PacketHandle packet, uint sniError) =>
             WriteAsyncCallback(IntPtr.Zero, packet, sniError);
 
@@ -719,10 +695,6 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu
             return task;
         }
 
-        internal abstract bool IsValidPacket(PacketHandle packetPointer);
-
-        internal abstract uint WritePacket(PacketHandle packet, bool sync);
-
         // Sends an attention signal - executing thread will consume attn.
         internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = false)
         {
@@ -871,7 +843,5 @@ private void SniWriteStatisticsAndTracing()
                 statistics.RequestNetworkServerTimer();
             }
         }
-
-        protected abstract PacketHandle EmptyReadPacket { get; }
     }
 }

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs

@@ -43,23 +43,34 @@ private enum NativeProtocols
         internal SNIPacket _sniAsyncAttnPacket = null;                // Packet to use to send Attn
         private readonly WritePacketCache _writePacketCache = new WritePacketCache(); // Store write packets that are ready to be re-used
 
-        public TdsParserStateObjectNative(TdsParser parser) : base(parser) { }
-
         private GCHandle _gcHandle;                                    // keeps this object alive until we're closed.
 
-        private Dictionary<IntPtr, SNIPacket> _pendingWritePackets = new Dictionary<IntPtr, SNIPacket>(); // Stores write packets that have been sent to SNI, but have not yet finished writing (i.e. we are waiting for SNI's callback)
+        private readonly Dictionary<IntPtr, SNIPacket> _pendingWritePackets = new Dictionary<IntPtr, SNIPacket>(); // Stores write packets that have been sent to SNI, but have not yet finished writing (i.e. we are waiting for SNI's callback)
 
-        internal TdsParserStateObjectNative(TdsParser parser, TdsParserStateObject physicalConnection, bool async) :
-            base(parser, physicalConnection, async)
+        internal TdsParserStateObjectNative(TdsParser parser, TdsParserStateObject physicalConnection, bool async)
+            : base(parser, physicalConnection, async)
         {
         }
 
+        public TdsParserStateObjectNative(TdsParser parser)
+            : base(parser)
+        {
+        }
+
+        ////////////////
+        // Properties //
+        ////////////////
+
         internal SNIHandle Handle => _sessionHandle;
 
         internal override uint Status => _sessionHandle != null ? _sessionHandle.Status : TdsEnums.SNI_UNINITIALIZED;
 
         internal override SessionHandle SessionHandle => SessionHandle.FromNativeHandle(_sessionHandle);
 
+        protected override PacketHandle EmptyReadPacket => PacketHandle.FromNativePointer(default);
+
+        internal override Guid? SessionId => default;
+
         protected override void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async)
         {
             Debug.Assert(physicalConnection is TdsParserStateObjectNative, "Expected a stateObject of type " + this.GetType());
@@ -256,22 +267,6 @@ protected override void FreeGcHandle(int remaining, bool release)
 
         internal override bool IsFailedHandle() => _sessionHandle.Status != TdsEnums.SNI_SUCCESS;
 
-        internal override PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint error)
-        {
-            SNIHandle handle = Handle;
-            if (handle == null)
-            {
-                throw ADP.ClosedConnectionError();
-            }
-            IntPtr readPacketPtr = IntPtr.Zero;
-            error = SniNativeWrapper.SniReadSyncOverAsync(handle, ref readPacketPtr, GetTimeoutRemaining());
-            return PacketHandle.FromNativePointer(readPacketPtr);
-        }
-
-        protected override PacketHandle EmptyReadPacket => PacketHandle.FromNativePointer(default);
-
-        internal override Guid? SessionId => default;
-
         internal override bool IsPacketEmpty(PacketHandle readPacket)
         {
             Debug.Assert(readPacket.Type == PacketHandle.NativePointerType || readPacket.Type == 0, "unexpected packet type when requiring NativePointer");
@@ -298,6 +293,14 @@ internal override PacketHandle ReadAsync(SessionHandle handle, out uint error)
             return PacketHandle.FromNativePointer(readPacketPtr);
         }
 
+        internal override PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint error)
+        {
+            SNIHandle handle = Handle ?? throw ADP.ClosedConnectionError();
+            IntPtr readPacketPtr = IntPtr.Zero;
+            error = SniNativeWrapper.SniReadSyncOverAsync(handle, ref readPacketPtr, GetTimeoutRemaining());
+            return PacketHandle.FromNativePointer(readPacketPtr);
+        }
+
         internal override PacketHandle CreateAndSetAttentionPacket()
         {
             SNIHandle handle = Handle;
@@ -332,11 +335,9 @@ internal override PacketHandle AddPacketToPendingList(PacketHandle packetToAdd)
         internal override bool IsValidPacket(PacketHandle packetPointer)
         {
             Debug.Assert(packetPointer.Type == PacketHandle.NativePointerType || packetPointer.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePointer");
-            return (
-                (packetPointer.Type == PacketHandle.NativePointerType && packetPointer.NativePointer != IntPtr.Zero)
-                ||
-                (packetPointer.Type == PacketHandle.NativePacketType && packetPointer.NativePacket != null)
-            );
+
+            return (packetPointer.Type == PacketHandle.NativePointerType && packetPointer.NativePointer != IntPtr.Zero)
+                || (packetPointer.Type == PacketHandle.NativePacketType && packetPointer.NativePacket != null);
         }
 
         internal override PacketHandle GetResetWritePacket(int dataSize)
@@ -444,8 +445,16 @@ internal override void DisposePacketCache()
         {
             lock (_writePacketLockObject)
             {
-                _writePacketCache.Dispose();
-                // Do not set _writePacketCache to null, just in case a WriteAsyncCallback completes after this point
+#if NETFRAMEWORK
+                RuntimeHelpers.PrepareConstrainedRegions();
+#endif
+                try
+                { }
+                finally
+                {
+                    _writePacketCache.Dispose();
+                    // Do not set _writePacketCache to null, just in case a WriteAsyncCallback completes after this point
+                }
             }
         }