From c79425e69f9437aae077a5f5c31385c4766bdfd1 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 16 May 2025 19:36:51 +0100 Subject: [PATCH 01/28] Port PacketHandle to netfx --- .../src/Microsoft.Data.SqlClient.csproj | 4 ++-- .../netfx/src/Microsoft.Data.SqlClient.csproj | 3 +++ ...ore.Windows.cs => PacketHandle.Windows.cs} | 23 +++++++++++++++---- 3 files changed, 23 insertions(+), 7 deletions(-) rename src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/{PacketHandle.netcore.Windows.cs => PacketHandle.Windows.cs} (78%) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj index eb556a79ec..eb0504f047 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj @@ -878,8 +878,8 @@ Microsoft\Data\SqlClient\LocalDb\LocalDbApi.Windows.cs - - Microsoft\Data\SqlClient\PacketHandle.netcore.Windows.cs + + Microsoft\Data\SqlClient\PacketHandle.Windows.cs Microsoft\Data\SqlClient\SessionHandle.Windows.cs diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj index e1c3277fb7..23380b07a6 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj @@ -447,6 +447,9 @@ Microsoft\Data\SqlClient\Packet.cs + + Microsoft\Data\SqlClient\PacketHandle.Windows.cs + Microsoft\Data\SqlClient\ParameterPeekAheadValue.cs diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/PacketHandle.netcore.Windows.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/PacketHandle.Windows.cs similarity index 78% rename from src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/PacketHandle.netcore.Windows.cs rename to src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/PacketHandle.Windows.cs index aca8292100..8811729e94 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/PacketHandle.netcore.Windows.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/PacketHandle.Windows.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#if NET - using System; namespace Microsoft.Data.SqlClient @@ -22,13 +20,16 @@ internal readonly ref struct PacketHandle { public const int NativePointerType = 1; public const int NativePacketType = 2; +#if NET public const int ManagedPacketType = 3; public readonly SNI.SNIPacket ManagedPacket; +#endif public readonly SNIPacket NativePacket; public readonly IntPtr NativePointer; public readonly int Type; +#if NET private PacketHandle(IntPtr nativePointer, SNIPacket nativePacket, SNI.SNIPacket managedPacket, int type) { Type = type; @@ -36,7 +37,16 @@ private PacketHandle(IntPtr nativePointer, SNIPacket nativePacket, SNI.SNIPacket NativePointer = nativePointer; NativePacket = nativePacket; } +#else + private PacketHandle(IntPtr nativePointer, SNIPacket nativePacket, int type) + { + Type = type; + NativePointer = nativePointer; + NativePacket = nativePacket; + } +#endif +#if NET public static PacketHandle FromManagedPacket(SNI.SNIPacket managedPacket) => new PacketHandle(default, default, managedPacket, ManagedPacketType); @@ -45,9 +55,12 @@ public static PacketHandle FromNativePointer(IntPtr nativePointer) => public static PacketHandle FromNativePacket(SNIPacket nativePacket) => new PacketHandle(default, nativePacket, default, NativePacketType); +#else + public static PacketHandle FromNativePointer(IntPtr nativePointer) => + new PacketHandle(nativePointer, default, NativePointerType); - + public static PacketHandle FromNativePacket(SNIPacket nativePacket) => + new PacketHandle(default, nativePacket, NativePacketType); +#endif } } - -#endif From e41cf0e710238cb381bacf143a2a38bdab57b512 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 16 May 2025 19:46:05 +0100 Subject: [PATCH 02/28] Remove PacketHandle alias shim --- .../src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs | 2 -- .../Data/SqlClient/TdsParserStateObject.Multiplexer.cs | 3 --- .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 1 - 3 files changed, 6 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 49f350f904..9de21620b9 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -18,8 +18,6 @@ namespace Microsoft.Data.SqlClient { - using PacketHandle = IntPtr; - internal partial class TdsParserStateObject { protected SNIHandle _sessionHandle = null; // the SNI handle we're to work on diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.Multiplexer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.Multiplexer.cs index 19d3d5add2..f3847c436e 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.Multiplexer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.Multiplexer.cs @@ -7,9 +7,6 @@ namespace Microsoft.Data.SqlClient { -#if NETFRAMEWORK - using PacketHandle = IntPtr; -#endif partial class TdsParserStateObject { private Packet _partialPacket; diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 8cbc275043..61cc18b536 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -19,7 +19,6 @@ namespace Microsoft.Data.SqlClient { #if NETFRAMEWORK - using PacketHandle = IntPtr; using RuntimeHelpers = System.Runtime.CompilerServices.RuntimeHelpers; #endif From b1a44134c4cea97dd5cc348330f8331557c44387 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 16 May 2025 23:18:38 +0100 Subject: [PATCH 03/28] Align netfx use of PacketHandle --- .../SqlClient/TdsParserStateObject.netfx.cs | 95 +++++++++++++------ .../SqlClient/TdsParserSafeHandles.Windows.cs | 8 -- .../TdsParserStateObject.Multiplexer.cs | 5 - .../TdsParserStateObject.TestHarness.cs | 5 +- 4 files changed, 68 insertions(+), 45 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 9de21620b9..d061e759c9 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -136,27 +136,35 @@ internal void CreatePhysicalSNIHandle( ipPreference, cachedDNSInfo, hostNameInCertificate); } - internal bool IsPacketEmpty(PacketHandle readPacket) => readPacket == default; + internal bool IsPacketEmpty(PacketHandle readPacket) + { + Debug.Assert(readPacket.Type == PacketHandle.NativePointerType || readPacket.Type == 0, "unexpected packet type when requiring NativePointer"); + return IntPtr.Zero == readPacket.NativePointer; + } internal PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint error) { SNIHandle handle = Handle ?? throw ADP.ClosedConnectionError(); - PacketHandle readPacket = default; - error = SniNativeWrapper.SniReadSyncOverAsync(handle, ref readPacket, timeoutRemaining); - return readPacket; + IntPtr readPacketPtr = IntPtr.Zero; + error = SniNativeWrapper.SniReadSyncOverAsync(handle, ref readPacketPtr, timeoutRemaining); + return PacketHandle.FromNativePointer(readPacketPtr); } internal PacketHandle ReadAsync(SessionHandle handle, out uint error) { - PacketHandle readPacket = default; - error = SniNativeWrapper.SniReadAsync(handle.NativeHandle, ref readPacket); - return readPacket; + IntPtr readPacketPtr = IntPtr.Zero; + error = SniNativeWrapper.SniReadAsync(handle.NativeHandle, ref readPacketPtr); + return PacketHandle.FromNativePointer(readPacketPtr); } internal uint CheckConnection() => SniNativeWrapper.SniCheckConnection(Handle); - internal void ReleasePacket(PacketHandle syncReadPacket) => SniNativeWrapper.SniPacketRelease(syncReadPacket); - + internal void ReleasePacket(PacketHandle syncReadPacket) + { + Debug.Assert(syncReadPacket.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + SniNativeWrapper.SniPacketRelease(syncReadPacket.NativePointer); + } + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] internal int DecrementPendingCallbacks(bool release) { @@ -375,7 +383,13 @@ private void ReadSniError(TdsParserStateObject stateObj, uint error) private uint GetSniPacket(PacketHandle packet, ref uint dataSize) { - return SniNativeWrapper.SniPacketGetData(packet, _inBuff, ref dataSize); + return SniPacketGetData(packet, _inBuff, ref dataSize); + } + + private uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize) + { + Debug.Assert(packet.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + return SniNativeWrapper.SniPacketGetData(packet.NativePointer, _inBuff, ref dataSize); } public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) @@ -409,7 +423,7 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) bool processFinallyBlock = true; try { - Debug.Assert(IntPtr.Zero == packet || IntPtr.Zero != packet && source != null, "AsyncResult null on callback"); + Debug.Assert(CheckPacket(packet, source), "AsyncResult null on callback"); if (_parser.MARSOn) { @@ -479,6 +493,13 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) } } + private bool CheckPacket(PacketHandle packet, TaskCompletionSource source) + { + Debug.Assert(packet.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + IntPtr ptr = packet.NativePointer; + return IntPtr.Zero == ptr || IntPtr.Zero != ptr && source != null; + } + #pragma warning disable 420 // a reference to a volatile field will not be treated as volatile public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError) @@ -652,7 +673,7 @@ internal Task WritePacket(byte flushMode, bool canAccumulate = false) #pragma warning disable 420 // a reference to a volatile field will not be treated as volatile - private Task SNIWritePacket(SNIPacket packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock, bool asyncClose = false) + private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock, bool asyncClose = false) { // Check for a stored exception Exception delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); @@ -694,7 +715,8 @@ private Task SNIWritePacket(SNIPacket packet, out uint sniError, bool canAccumul } finally { - sniError = SniNativeWrapper.SniWritePacket(Handle, packet, sync); + Debug.Assert(packet.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePacket"); + sniError = SniNativeWrapper.SniWritePacket(Handle, packet.NativePacket, sync); } if (sniError == TdsEnums.SNI_SUCCESS_IO_PENDING) @@ -788,7 +810,15 @@ private Task SNIWritePacket(SNIPacket packet, out uint sniError, bool canAccumul #pragma warning restore 420 - internal bool IsValidPacket(PacketHandle packetPointer) => packetPointer != default; + internal 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) + ); + } // Sends an attention signal - executing thread will consume attn. internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = false) @@ -803,10 +833,7 @@ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = fa return; } - SNIPacket attnPacket = new SNIPacket(Handle); - _sniAsyncAttnPacket = attnPacket; - - SniNativeWrapper.SniPacketSetData(attnPacket, SQL.AttentionHeader, TdsEnums.HEADER_LEN, null, null); + PacketHandle attnPacket = CreateAndSetAttentionPacket(); RuntimeHelpers.PrepareConstrainedRegions(); try @@ -866,11 +893,20 @@ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = fa } } + internal PacketHandle CreateAndSetAttentionPacket() + { + SNIPacket attnPacket = new SNIPacket(Handle); + _sniAsyncAttnPacket = attnPacket; + SniNativeWrapper.SniPacketSetData(attnPacket, SQL.AttentionHeader, TdsEnums.HEADER_LEN, null, null); + return PacketHandle.FromNativePacket(attnPacket); + } + private Task WriteSni(bool canAccumulate) { // Prepare packet, and write to packet. - SNIPacket packet = GetResetWritePacket(); - SniNativeWrapper.SniPacketSetData(packet, _outBuff, _outBytesUsed, _securePasswords, _securePasswordOffsetsInBuffer); + PacketHandle packet = GetResetWritePacket(); + SNIPacket nativePacket = packet.NativePacket; + SniNativeWrapper.SniPacketSetData(nativePacket, _outBuff, _outBytesUsed, _securePasswords, _securePasswordOffsetsInBuffer); Debug.Assert(Parser.Connection._parserLock.ThreadMayHaveLock(), "Thread is writing without taking the connection lock"); Task task = SNIWritePacket(packet, out _, canAccumulate, callerHasConnectionLock: true); @@ -921,7 +957,7 @@ private Task WriteSni(bool canAccumulate) return task; } - internal SNIPacket GetResetWritePacket() + internal PacketHandle GetResetWritePacket() { if (_sniPacket != null) { @@ -934,7 +970,7 @@ internal SNIPacket GetResetWritePacket() _sniPacket = _writePacketCache.Take(Handle); } } - return _sniPacket; + return PacketHandle.FromNativePacket(_sniPacket); } internal void ClearAllWritePackets() @@ -951,8 +987,10 @@ internal void ClearAllWritePackets() } } - private IntPtr AddPacketToPendingList(SNIPacket packet) + private PacketHandle AddPacketToPendingList(PacketHandle packetToAdd) { + Debug.Assert(packetToAdd.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePacket"); + SNIPacket packet = packetToAdd.NativePacket; Debug.Assert(packet == _sniPacket, "Adding a packet other than the current packet to the pending list"); _sniPacket = null; IntPtr pointer = packet.DangerousGetHandle(); @@ -962,16 +1000,17 @@ private IntPtr AddPacketToPendingList(SNIPacket packet) _pendingWritePackets.Add(pointer, packet); } - return pointer; + return PacketHandle.FromNativePointer(pointer); } - private void RemovePacketFromPendingList(IntPtr pointer) + private void RemovePacketFromPendingList(PacketHandle ptr) { - SNIPacket recoveredPacket; + Debug.Assert(ptr.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + IntPtr pointer = ptr.NativePointer; lock (_writePacketLockObject) { - if (_pendingWritePackets.TryGetValue(pointer, out recoveredPacket)) + if (_pendingWritePackets.TryGetValue(pointer, out SNIPacket recoveredPacket)) { _pendingWritePackets.Remove(pointer); _writePacketCache.Add(recoveredPacket); @@ -1031,6 +1070,6 @@ private void SniWriteStatisticsAndTracing() SqlClientEventSource.Log.TryAdvancedTraceBinEvent("TdsParser.WritePacket | INFO | ADV | State Object Id {0}, Packet sent. Out buffer: {1}, Out Bytes Used: {2}", ObjectID, _outBuff, _outBytesUsed); } - protected PacketHandle EmptyReadPacket => default; + protected PacketHandle EmptyReadPacket => PacketHandle.FromNativePointer(default); } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserSafeHandles.Windows.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserSafeHandles.Windows.cs index f2f9143ed7..37dc781dbc 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserSafeHandles.Windows.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserSafeHandles.Windows.cs @@ -107,11 +107,7 @@ private static void ReadDispatcher(IntPtr key, IntPtr packet, uint error) if (stateObj != null) { -#if NETFRAMEWORK - stateObj.ReadAsyncCallback(IntPtr.Zero, packet, error); -#else stateObj.ReadAsyncCallback(IntPtr.Zero, PacketHandle.FromNativePointer(packet), error); -#endif // NETFRAMEWORK } } } @@ -132,11 +128,7 @@ private static void WriteDispatcher(IntPtr key, IntPtr packet, uint error) if (stateObj != null) { -#if NETFRAMEWORK - stateObj.WriteAsyncCallback(IntPtr.Zero, packet, error); -#else stateObj.WriteAsyncCallback(IntPtr.Zero, PacketHandle.FromNativePointer(packet), error); -#endif // NETFRAMEWORK } } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.Multiplexer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.Multiplexer.cs index f3847c436e..77bd1c982b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.Multiplexer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.Multiplexer.cs @@ -510,12 +510,7 @@ public void ProcessSniPacketCompat(PacketHandle packet, uint error) else { uint dataSize = 0; - - #if NETFRAMEWORK - uint getDataError = SniNativeWrapper.SniPacketGetData(packet, _inBuff, ref dataSize); - #else uint getDataError = SniPacketGetData(packet, _inBuff, ref dataSize); - #endif if (getDataError == TdsEnums.SNI_SUCCESS) { diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/TdsParserStateObject.TestHarness.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/TdsParserStateObject.TestHarness.cs index b746e7c0ad..e448b66b83 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/TdsParserStateObject.TestHarness.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/TdsParserStateObject.TestHarness.cs @@ -10,13 +10,10 @@ namespace Microsoft.Data.SqlClient { -#if NETFRAMEWORK - using PacketHandle = IntPtr; -#elif NETCOREAPP internal struct PacketHandle { } -#endif + internal partial class TdsParserStateObject { internal int ObjectID = 1; From fc3c31e7cf43676899693f3700c04c70f44b1e7f Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 17 May 2025 14:25:40 +0100 Subject: [PATCH 04/28] Merge IsPacketEmpty --- .../Data/SqlClient/TdsParserStateObject.netcore.cs | 2 -- .../Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs | 6 ------ .../Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs | 7 +++++++ .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 ++ 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 5e1e268b06..aa218dd7f7 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -100,8 +100,6 @@ internal abstract void CreatePhysicalSNIHandle( 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); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index d061e759c9..370ac2f8ef 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -136,12 +136,6 @@ internal void CreatePhysicalSNIHandle( ipPreference, cachedDNSInfo, hostNameInCertificate); } - internal bool IsPacketEmpty(PacketHandle readPacket) - { - Debug.Assert(readPacket.Type == PacketHandle.NativePointerType || readPacket.Type == 0, "unexpected packet type when requiring NativePointer"); - return IntPtr.Zero == readPacket.NativePointer; - } - internal PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint error) { SNIHandle handle = Handle ?? throw ADP.ClosedConnectionError(); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index ea14931af7..47d1ae803b 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Diagnostics; using Interop.Windows.Sni; namespace Microsoft.Data.SqlClient @@ -29,6 +30,12 @@ internal TdsParserStateObjectNative(TdsParser parser) 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"); + return IntPtr.Zero == readPacket.NativePointer; + } + internal override uint SniGetConnectionId(ref Guid clientConnectionId) => SniNativeWrapper.SniGetConnectionId(Handle, ref clientConnectionId); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 61cc18b536..c9307bee2c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -481,6 +481,8 @@ internal long TimeoutTime internal abstract SessionHandle SessionHandle { get; } + internal abstract bool IsPacketEmpty(PacketHandle readPacket); + internal abstract uint SniGetConnectionId(ref Guid clientConnectionId); internal abstract uint DisableSsl(); From c03e518aac74e11141ca888619bdfe50375c9883 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 17 May 2025 14:29:59 +0100 Subject: [PATCH 05/28] Merge ReleasePacket --- .../Data/SqlClient/TdsParserStateObject.netcore.cs | 2 -- .../Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs | 6 ------ .../Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs | 6 ++++++ .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 ++ 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index aa218dd7f7..c145bc06e2 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -106,8 +106,6 @@ internal abstract void CreatePhysicalSNIHandle( 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); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 370ac2f8ef..c4bb09cd47 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -153,12 +153,6 @@ internal PacketHandle ReadAsync(SessionHandle handle, out uint error) internal uint CheckConnection() => SniNativeWrapper.SniCheckConnection(Handle); - internal void ReleasePacket(PacketHandle syncReadPacket) - { - Debug.Assert(syncReadPacket.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); - SniNativeWrapper.SniPacketRelease(syncReadPacket.NativePointer); - } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] internal int DecrementPendingCallbacks(bool release) { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 47d1ae803b..80f5b93cea 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -36,6 +36,12 @@ internal override bool IsPacketEmpty(PacketHandle readPacket) return IntPtr.Zero == readPacket.NativePointer; } + internal override void ReleasePacket(PacketHandle syncReadPacket) + { + Debug.Assert(syncReadPacket.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + SniNativeWrapper.SniPacketRelease(syncReadPacket.NativePointer); + } + internal override uint SniGetConnectionId(ref Guid clientConnectionId) => SniNativeWrapper.SniGetConnectionId(Handle, ref clientConnectionId); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index c9307bee2c..255f17dbce 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -483,6 +483,8 @@ internal long TimeoutTime internal abstract bool IsPacketEmpty(PacketHandle readPacket); + internal abstract void ReleasePacket(PacketHandle syncReadPacket); + internal abstract uint SniGetConnectionId(ref Guid clientConnectionId); internal abstract uint DisableSsl(); From adc1dbfa777ef0993a9e7eb01203940f496f2bc0 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 17 May 2025 14:37:31 +0100 Subject: [PATCH 06/28] Merge ReadAsync, ReadSyncOverAsync Also move ReadSyncOverAsync down in netcore's TdsParserStateObjectNative to aid later merge --- .../SqlClient/TdsParserStateObject.netcore.cs | 4 ---- .../SqlClient/TdsParserStateObjectNative.cs | 20 ++++++++----------- .../SqlClient/TdsParserStateObject.netfx.cs | 15 -------------- .../SqlClient/TdsParserStateObjectNative.cs | 16 +++++++++++++++ .../Data/SqlClient/TdsParserStateObject.cs | 4 ++++ 5 files changed, 28 insertions(+), 31 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index c145bc06e2..7920af4df6 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -100,10 +100,6 @@ internal abstract void CreatePhysicalSNIHandle( internal abstract void DisposePacketCache(); - internal abstract PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint error); - - internal abstract PacketHandle ReadAsync(SessionHandle handle, out uint error); - internal abstract uint CheckConnection(); protected abstract uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize); diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index b8d1b6cccb..477b6b63ea 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -256,18 +256,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; @@ -298,6 +286,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; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index c4bb09cd47..1b231966e9 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -136,21 +136,6 @@ internal void CreatePhysicalSNIHandle( ipPreference, cachedDNSInfo, hostNameInCertificate); } - internal PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint error) - { - SNIHandle handle = Handle ?? throw ADP.ClosedConnectionError(); - IntPtr readPacketPtr = IntPtr.Zero; - error = SniNativeWrapper.SniReadSyncOverAsync(handle, ref readPacketPtr, timeoutRemaining); - return PacketHandle.FromNativePointer(readPacketPtr); - } - - internal PacketHandle ReadAsync(SessionHandle handle, out uint error) - { - IntPtr readPacketPtr = IntPtr.Zero; - error = SniNativeWrapper.SniReadAsync(handle.NativeHandle, ref readPacketPtr); - return PacketHandle.FromNativePointer(readPacketPtr); - } - internal uint CheckConnection() => SniNativeWrapper.SniCheckConnection(Handle); [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 80f5b93cea..b34c39e24e 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -5,6 +5,7 @@ using System; using System.Diagnostics; using Interop.Windows.Sni; +using Microsoft.Data.Common; namespace Microsoft.Data.SqlClient { @@ -42,6 +43,21 @@ internal override void ReleasePacket(PacketHandle syncReadPacket) SniNativeWrapper.SniPacketRelease(syncReadPacket.NativePointer); } + internal override PacketHandle ReadAsync(SessionHandle handle, out uint error) + { + IntPtr readPacketPtr = IntPtr.Zero; + error = SniNativeWrapper.SniReadAsync(handle.NativeHandle, ref readPacketPtr); + 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, timeoutRemaining); + return PacketHandle.FromNativePointer(readPacketPtr); + } + internal override uint SniGetConnectionId(ref Guid clientConnectionId) => SniNativeWrapper.SniGetConnectionId(Handle, ref clientConnectionId); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 255f17dbce..8b5a8edce3 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -485,6 +485,10 @@ internal long TimeoutTime internal abstract void ReleasePacket(PacketHandle syncReadPacket); + internal abstract PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint error); + + internal abstract PacketHandle ReadAsync(SessionHandle handle, out uint error); + internal abstract uint SniGetConnectionId(ref Guid clientConnectionId); internal abstract uint DisableSsl(); From 3b58d9b9d46c83ee7903d59495e63dd27644ac27 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 17 May 2025 14:46:53 +0100 Subject: [PATCH 07/28] Merge IsFailedHandle --- .../Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs | 2 -- .../Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs | 4 ++-- .../Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs | 2 +- .../Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs | 2 ++ .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 ++ 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 7920af4df6..4922f62324 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/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); diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 477b6b63ea..234b54edd7 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -254,12 +254,12 @@ protected override void FreeGcHandle(int remaining, bool release) } } - internal override bool IsFailedHandle() => _sessionHandle.Status != TdsEnums.SNI_SUCCESS; - protected override PacketHandle EmptyReadPacket => PacketHandle.FromNativePointer(default); internal override Guid? SessionId => default; + internal override bool IsFailedHandle() => _sessionHandle.Status != TdsEnums.SNI_SUCCESS; + internal override bool IsPacketEmpty(PacketHandle readPacket) { Debug.Assert(readPacket.Type == PacketHandle.NativePointerType || readPacket.Type == 0, "unexpected packet type when requiring NativePointer"); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 1b231966e9..8b85fe4ba3 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -64,7 +64,7 @@ protected TdsParserStateObject(TdsParser parser, SNIHandle physicalConnection, b SQLFallbackDNSCache.Instance.GetDNSInfo(_parser.FQDNforDNSCache, out cachedDNSInfo); _sessionHandle = new SNIHandle(myInfo, physicalConnection, _parser.Connection.ConnectionOptions.IPAddressPreference, cachedDNSInfo); - if (_sessionHandle.Status != TdsEnums.SNI_SUCCESS) + if (IsFailedHandle()) { AddError(parser.ProcessSNIError(this)); ThrowExceptionAndWarning(); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index b34c39e24e..5928c62191 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -31,6 +31,8 @@ internal TdsParserStateObjectNative(TdsParser parser) internal override Guid? SessionId => default; + internal override bool IsFailedHandle() => _sessionHandle.Status != TdsEnums.SNI_SUCCESS; + internal override bool IsPacketEmpty(PacketHandle readPacket) { Debug.Assert(readPacket.Type == PacketHandle.NativePointerType || readPacket.Type == 0, "unexpected packet type when requiring NativePointer"); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 8b5a8edce3..4be4c229e5 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -481,6 +481,8 @@ internal long TimeoutTime internal abstract SessionHandle SessionHandle { get; } + internal abstract bool IsFailedHandle(); + internal abstract bool IsPacketEmpty(PacketHandle readPacket); internal abstract void ReleasePacket(PacketHandle syncReadPacket); From e06dba95519f717c59e207a174f9f5cb85b8ceb2 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 17 May 2025 18:55:57 +0100 Subject: [PATCH 08/28] Merge SniPacketGetData --- .../Data/SqlClient/TdsParserStateObject.netcore.cs | 2 -- .../Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs | 6 ------ .../Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs | 6 ++++++ .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 ++ 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 4922f62324..25e5da13eb 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -100,8 +100,6 @@ internal abstract void CreatePhysicalSNIHandle( internal abstract uint CheckConnection(); - protected abstract uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize); - internal abstract PacketHandle GetResetWritePacket(int dataSize); internal abstract void ClearAllWritePackets(); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 8b85fe4ba3..51d4992172 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -359,12 +359,6 @@ private uint GetSniPacket(PacketHandle packet, ref uint dataSize) return SniPacketGetData(packet, _inBuff, ref dataSize); } - private uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize) - { - Debug.Assert(packet.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); - return SniNativeWrapper.SniPacketGetData(packet.NativePointer, _inBuff, ref dataSize); - } - public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) { // Key never used. diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 5928c62191..a71a6e96d3 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -31,6 +31,12 @@ internal TdsParserStateObjectNative(TdsParser parser) internal override Guid? SessionId => default; + protected override uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize) + { + Debug.Assert(packet.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + return SniNativeWrapper.SniPacketGetData(packet.NativePointer, _inBuff, ref dataSize); + } + internal override bool IsFailedHandle() => _sessionHandle.Status != TdsEnums.SNI_SUCCESS; internal override bool IsPacketEmpty(PacketHandle readPacket) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 4be4c229e5..e7db84a112 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -481,6 +481,8 @@ internal long TimeoutTime internal abstract SessionHandle SessionHandle { get; } + protected abstract uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize); + internal abstract bool IsFailedHandle(); internal abstract bool IsPacketEmpty(PacketHandle readPacket); From 1ee1a14fda5cc76e15e83312f94407cb0a17f37c Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 17 May 2025 19:26:33 +0100 Subject: [PATCH 09/28] Merge EmptyReadPacket Also reorder member in TdsParserStateObjectNative to simplify later merge --- .../Data/SqlClient/TdsParserStateObject.netcore.cs | 2 -- .../Data/SqlClient/TdsParserStateObjectNative.cs | 12 ++++++++---- .../Data/SqlClient/TdsParserStateObject.netfx.cs | 2 -- .../Data/SqlClient/TdsParserStateObjectNative.cs | 2 ++ .../Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 ++ 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 25e5da13eb..95dd523ad5 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -859,7 +859,5 @@ private void SniWriteStatisticsAndTracing() statistics.RequestNetworkServerTimer(); } } - - protected abstract PacketHandle EmptyReadPacket { get; } } } diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 234b54edd7..52ffe62035 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -54,12 +54,20 @@ internal TdsParserStateObjectNative(TdsParser parser, TdsParserStateObject physi { } + //////////////// + // 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()); @@ -254,10 +262,6 @@ protected override void FreeGcHandle(int remaining, bool release) } } - protected override PacketHandle EmptyReadPacket => PacketHandle.FromNativePointer(default); - - internal override Guid? SessionId => default; - internal override bool IsFailedHandle() => _sessionHandle.Status != TdsEnums.SNI_SUCCESS; internal override bool IsPacketEmpty(PacketHandle readPacket) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 51d4992172..72a74caed2 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -1036,7 +1036,5 @@ private void SniWriteStatisticsAndTracing() } SqlClientEventSource.Log.TryAdvancedTraceBinEvent("TdsParser.WritePacket | INFO | ADV | State Object Id {0}, Packet sent. Out buffer: {1}, Out Bytes Used: {2}", ObjectID, _outBuff, _outBytesUsed); } - - protected PacketHandle EmptyReadPacket => PacketHandle.FromNativePointer(default); } } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index a71a6e96d3..df7032b15f 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -29,6 +29,8 @@ internal TdsParserStateObjectNative(TdsParser parser) internal override SessionHandle SessionHandle => SessionHandle.FromNativeHandle(_sessionHandle); + protected override PacketHandle EmptyReadPacket => PacketHandle.FromNativePointer(default); + internal override Guid? SessionId => default; protected override uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index e7db84a112..ba5f0e0490 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -481,6 +481,8 @@ internal long TimeoutTime internal abstract SessionHandle SessionHandle { get; } + protected abstract PacketHandle EmptyReadPacket { get; } + protected abstract uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize); internal abstract bool IsFailedHandle(); From 51903e611dba499f3812a672110f4de9cd733d8b Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 17 May 2025 19:30:06 +0100 Subject: [PATCH 10/28] Merge CheckPacket --- .../Data/SqlClient/TdsParserStateObject.netcore.cs | 2 -- .../Data/SqlClient/TdsParserStateObject.netfx.cs | 7 ------- .../Data/SqlClient/TdsParserStateObjectNative.cs | 8 ++++++++ .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 ++ 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 95dd523ad5..8f4208201b 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -400,8 +400,6 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) } } - protected abstract bool CheckPacket(PacketHandle packet, TaskCompletionSource source); - public void WriteAsyncCallback(PacketHandle packet, uint sniError) => WriteAsyncCallback(IntPtr.Zero, packet, sniError); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 72a74caed2..dcfa936d8a 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -460,13 +460,6 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) } } - private bool CheckPacket(PacketHandle packet, TaskCompletionSource source) - { - Debug.Assert(packet.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); - IntPtr ptr = packet.NativePointer; - return IntPtr.Zero == ptr || IntPtr.Zero != ptr && source != null; - } - #pragma warning disable 420 // a reference to a volatile field will not be treated as volatile public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index df7032b15f..6667d5b79f 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; +using System.Threading.Tasks; using Interop.Windows.Sni; using Microsoft.Data.Common; @@ -39,6 +40,13 @@ protected override uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, re return SniNativeWrapper.SniPacketGetData(packet.NativePointer, _inBuff, ref dataSize); } + protected override bool CheckPacket(PacketHandle packet, TaskCompletionSource source) + { + Debug.Assert(packet.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + IntPtr ptr = packet.NativePointer; + return IntPtr.Zero == ptr || IntPtr.Zero != ptr && source != null; + } + internal override bool IsFailedHandle() => _sessionHandle.Status != TdsEnums.SNI_SUCCESS; internal override bool IsPacketEmpty(PacketHandle readPacket) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index ba5f0e0490..509df1585b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -485,6 +485,8 @@ internal long TimeoutTime protected abstract uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize); + protected abstract bool CheckPacket(PacketHandle packet, TaskCompletionSource source); + internal abstract bool IsFailedHandle(); internal abstract bool IsPacketEmpty(PacketHandle readPacket); From 93311c905236d67e457c12ca6dbdcb7725374cb7 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 17 May 2025 19:42:35 +0100 Subject: [PATCH 11/28] Merge WritePacket, IsValidPacket --- .../SqlClient/TdsParserStateObject.netcore.cs | 4 ---- .../Data/SqlClient/TdsParserStateObject.netfx.cs | 13 +------------ .../Data/SqlClient/TdsParserStateObjectNative.cs | 16 ++++++++++++++++ .../Data/SqlClient/TdsParserStateObject.cs | 4 ++++ 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 8f4208201b..19c165e180 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -705,10 +705,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) { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index dcfa936d8a..ab82bad80c 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -675,8 +675,7 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu } finally { - Debug.Assert(packet.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePacket"); - sniError = SniNativeWrapper.SniWritePacket(Handle, packet.NativePacket, sync); + sniError = WritePacket(packet, sync); } if (sniError == TdsEnums.SNI_SUCCESS_IO_PENDING) @@ -770,16 +769,6 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu #pragma warning restore 420 - internal 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) - ); - } - // Sends an attention signal - executing thread will consume attn. internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = false) { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 6667d5b79f..9b4f7a015c 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -76,6 +76,22 @@ internal override PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint return PacketHandle.FromNativePointer(readPacketPtr); } + internal override uint WritePacket(PacketHandle packet, bool sync) + { + Debug.Assert(packet.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePacket"); + return SniNativeWrapper.SniWritePacket(Handle, packet.NativePacket, sync); + } + + 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) + ); + } + internal override uint SniGetConnectionId(ref Guid clientConnectionId) => SniNativeWrapper.SniGetConnectionId(Handle, ref clientConnectionId); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 509df1585b..ff9878b383 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -495,6 +495,10 @@ internal long TimeoutTime internal abstract PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint error); + internal abstract uint WritePacket(PacketHandle packet, bool sync); + + internal abstract bool IsValidPacket(PacketHandle packetPointer); + internal abstract PacketHandle ReadAsync(SessionHandle handle, out uint error); internal abstract uint SniGetConnectionId(ref Guid clientConnectionId); From 95c84144d6221a4336328fb4908b0b0d05053e10 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 17 May 2025 19:59:06 +0100 Subject: [PATCH 12/28] Merge GetResetWritePacket --- .../SqlClient/TdsParserStateObject.netcore.cs | 2 -- .../SqlClient/TdsParserStateObject.netfx.cs | 18 +----------------- .../SqlClient/TdsParserStateObjectNative.cs | 16 ++++++++++++++++ .../Data/SqlClient/TdsParserStateObject.cs | 2 ++ 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 19c165e180..5bcb5b86b7 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -100,8 +100,6 @@ internal abstract void CreatePhysicalSNIHandle( internal abstract uint CheckConnection(); - internal abstract PacketHandle GetResetWritePacket(int dataSize); - internal abstract void ClearAllWritePackets(); internal abstract PacketHandle AddPacketToPendingList(PacketHandle packet); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index ab82bad80c..043b373288 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -853,7 +853,7 @@ internal PacketHandle CreateAndSetAttentionPacket() private Task WriteSni(bool canAccumulate) { // Prepare packet, and write to packet. - PacketHandle packet = GetResetWritePacket(); + PacketHandle packet = GetResetWritePacket(_outBytesUsed); SNIPacket nativePacket = packet.NativePacket; SniNativeWrapper.SniPacketSetData(nativePacket, _outBuff, _outBytesUsed, _securePasswords, _securePasswordOffsetsInBuffer); @@ -906,22 +906,6 @@ private Task WriteSni(bool canAccumulate) return task; } - internal PacketHandle GetResetWritePacket() - { - if (_sniPacket != null) - { - SniNativeWrapper.SniPacketReset(Handle, IoType.WRITE, _sniPacket, ConsumerNumber.SNI_Consumer_SNI); - } - else - { - lock (_writePacketLockObject) - { - _sniPacket = _writePacketCache.Take(Handle); - } - } - return PacketHandle.FromNativePacket(_sniPacket); - } - internal void ClearAllWritePackets() { if (_sniPacket != null) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 9b4f7a015c..2259ba37bd 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -92,6 +92,22 @@ internal override bool IsValidPacket(PacketHandle packetPointer) ); } + internal override PacketHandle GetResetWritePacket(int dataSize) + { + if (_sniPacket != null) + { + SniNativeWrapper.SniPacketReset(Handle, IoType.WRITE, _sniPacket, ConsumerNumber.SNI_Consumer_SNI); + } + else + { + lock (_writePacketLockObject) + { + _sniPacket = _writePacketCache.Take(Handle); + } + } + return PacketHandle.FromNativePacket(_sniPacket); + } + internal override uint SniGetConnectionId(ref Guid clientConnectionId) => SniNativeWrapper.SniGetConnectionId(Handle, ref clientConnectionId); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index ff9878b383..114868a732 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -483,6 +483,8 @@ internal long TimeoutTime protected abstract PacketHandle EmptyReadPacket { get; } + internal abstract PacketHandle GetResetWritePacket(int dataSize); + protected abstract uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize); protected abstract bool CheckPacket(PacketHandle packet, TaskCompletionSource source); From c21d451110bd0f6e017637177ec620daf9ebb0f2 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 17 May 2025 20:20:17 +0100 Subject: [PATCH 13/28] Merge ClearAllWritePackets, AddPacketToPendingList, RemovePacketFromPendingList --- .../SqlClient/TdsParserStateObject.netcore.cs | 6 -- .../SqlClient/TdsParserStateObject.netfx.cs | 56 +------------------ .../SqlClient/TdsParserStateObjectNative.cs | 54 ++++++++++++++++++ .../Data/SqlClient/TdsParserStateObject.cs | 6 ++ 4 files changed, 62 insertions(+), 60 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 5bcb5b86b7..8c70cd8508 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -100,12 +100,6 @@ internal abstract void CreatePhysicalSNIHandle( internal abstract uint CheckConnection(); - 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); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 043b373288..2c8e89ab23 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -23,10 +23,9 @@ internal partial class TdsParserStateObject protected SNIHandle _sessionHandle = null; // the SNI handle we're to work on // SNI variables // multiple resultsets in one batch. - private SNIPacket _sniPacket = null; // Will have to re-vamp this for MARS + protected SNIPacket _sniPacket = null; // Will have to re-vamp this for MARS 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 - private readonly Dictionary _pendingWritePackets = new Dictionary(); // Stores write packets that have been sent to SNI, but have not yet finished writing (i.e. we are waiting for SNI's callback) + protected readonly WritePacketCache _writePacketCache = new WritePacketCache(); // Store write packets that are ready to be re-used // Async variables private GCHandle _gcHandle; // keeps this object alive until we're closed. @@ -906,57 +905,6 @@ private Task WriteSni(bool canAccumulate) return task; } - internal void ClearAllWritePackets() - { - if (_sniPacket != null) - { - _sniPacket.Dispose(); - _sniPacket = null; - } - lock (_writePacketLockObject) - { - Debug.Assert(_pendingWritePackets.Count == 0 && _asyncWriteCount == 0, "Should not clear all write packets if there are packets pending"); - _writePacketCache.Clear(); - } - } - - private PacketHandle AddPacketToPendingList(PacketHandle packetToAdd) - { - Debug.Assert(packetToAdd.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePacket"); - SNIPacket packet = packetToAdd.NativePacket; - Debug.Assert(packet == _sniPacket, "Adding a packet other than the current packet to the pending list"); - _sniPacket = null; - IntPtr pointer = packet.DangerousGetHandle(); - - lock (_writePacketLockObject) - { - _pendingWritePackets.Add(pointer, packet); - } - - return PacketHandle.FromNativePointer(pointer); - } - - private void RemovePacketFromPendingList(PacketHandle ptr) - { - Debug.Assert(ptr.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); - IntPtr pointer = ptr.NativePointer; - - lock (_writePacketLockObject) - { - if (_pendingWritePackets.TryGetValue(pointer, out SNIPacket recoveredPacket)) - { - _pendingWritePackets.Remove(pointer); - _writePacketCache.Add(recoveredPacket); - } -#if DEBUG - else - { - Debug.Assert(false, "Removing a packet from the pending list that was never added to it"); - } -#endif - } - } - ////////////////////////////////////////////// // Statistics, Tracing, and related methods // ////////////////////////////////////////////// diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 2259ba37bd..40956551fd 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Generic; using System.Diagnostics; using System.Threading.Tasks; using Interop.Windows.Sni; @@ -12,6 +13,8 @@ namespace Microsoft.Data.SqlClient { internal class TdsParserStateObjectNative : TdsParserStateObject { + private readonly Dictionary _pendingWritePackets = new Dictionary(); // 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.Handle, async) { @@ -47,6 +50,27 @@ protected override bool CheckPacket(PacketHandle packet, TaskCompletionSource _sessionHandle.Status != TdsEnums.SNI_SUCCESS; internal override bool IsPacketEmpty(PacketHandle readPacket) @@ -82,6 +106,22 @@ internal override uint WritePacket(PacketHandle packet, bool sync) return SniNativeWrapper.SniWritePacket(Handle, packet.NativePacket, sync); } + internal override PacketHandle AddPacketToPendingList(PacketHandle packetToAdd) + { + Debug.Assert(packetToAdd.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePacket"); + SNIPacket packet = packetToAdd.NativePacket; + Debug.Assert(packet == _sniPacket, "Adding a packet other than the current packet to the pending list"); + _sniPacket = null; + IntPtr pointer = packet.DangerousGetHandle(); + + lock (_writePacketLockObject) + { + _pendingWritePackets.Add(pointer, packet); + } + + return PacketHandle.FromNativePointer(pointer); + } + internal override bool IsValidPacket(PacketHandle packetPointer) { Debug.Assert(packetPointer.Type == PacketHandle.NativePointerType || packetPointer.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePointer"); @@ -108,6 +148,20 @@ internal override PacketHandle GetResetWritePacket(int dataSize) return PacketHandle.FromNativePacket(_sniPacket); } + internal override void ClearAllWritePackets() + { + if (_sniPacket != null) + { + _sniPacket.Dispose(); + _sniPacket = null; + } + lock (_writePacketLockObject) + { + Debug.Assert(_pendingWritePackets.Count == 0 && _asyncWriteCount == 0, "Should not clear all write packets if there are packets pending"); + _writePacketCache.Clear(); + } + } + internal override uint SniGetConnectionId(ref Guid clientConnectionId) => SniNativeWrapper.SniGetConnectionId(Handle, ref clientConnectionId); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 114868a732..d7de640162 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -499,6 +499,12 @@ internal long TimeoutTime internal abstract uint WritePacket(PacketHandle packet, bool sync); + internal abstract PacketHandle AddPacketToPendingList(PacketHandle packet); + + protected abstract void RemovePacketFromPendingList(PacketHandle pointer); + + internal abstract void ClearAllWritePackets(); + internal abstract bool IsValidPacket(PacketHandle packetPointer); internal abstract PacketHandle ReadAsync(SessionHandle handle, out uint error); From 857afe6c2454c1d7b74da6e76843dc4254dea4ea Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Sat, 17 May 2025 20:24:02 +0100 Subject: [PATCH 14/28] Improve diff between versions of TdsParserStateObjectNative --- .../Data/SqlClient/TdsParserStateObjectNative.cs | 13 ++++++++----- .../Data/SqlClient/TdsParserStateObjectNative.cs | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 52ffe62035..653db9c27a 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -43,14 +43,17 @@ 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 _pendingWritePackets = new Dictionary(); // 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 _pendingWritePackets = new Dictionary(); // 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) { } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 40956551fd..1e69faeb84 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -20,7 +20,7 @@ internal TdsParserStateObjectNative(TdsParser parser, TdsParserStateObject physi { } - internal TdsParserStateObjectNative(TdsParser parser) + public TdsParserStateObjectNative(TdsParser parser) : base(parser) { } From 0767cd77650be307e0334d3942e9ed59e048d5f2 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Wed, 21 May 2025 05:34:43 +0100 Subject: [PATCH 15/28] PR feedback from #3353 - format IsValidPacket --- .../Data/SqlClient/TdsParserStateObjectNative.cs | 8 +++----- .../Data/SqlClient/TdsParserStateObjectNative.cs | 8 +++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 653db9c27a..bbbe0f0416 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -335,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) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 1e69faeb84..49c1990401 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -125,11 +125,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) From fe5d81c7291241e10e39d830d929a60446f82779 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 23 May 2025 19:47:01 +0100 Subject: [PATCH 16/28] Address merge conflicts --- .../Data/SqlClient/TdsParserStateObject.netfx.cs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 49f6bdbb2d..2c8e89ab23 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -358,12 +358,6 @@ private uint GetSniPacket(PacketHandle packet, ref uint dataSize) return SniPacketGetData(packet, _inBuff, ref dataSize); } - private uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize) - { - Debug.Assert(packet.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); - return SniNativeWrapper.SniPacketGetData(packet.NativePointer, _inBuff, ref dataSize); - } - public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) { // Key never used. @@ -465,13 +459,6 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) } } - private bool CheckPacket(PacketHandle packet, TaskCompletionSource source) - { - Debug.Assert(packet.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); - IntPtr ptr = packet.NativePointer; - return IntPtr.Zero == ptr || IntPtr.Zero != ptr && source != null; - } - #pragma warning disable 420 // a reference to a volatile field will not be treated as volatile public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError) From bd5a153cee975c97e33c5392c094065cc3db1478 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 23 May 2025 20:43:48 +0100 Subject: [PATCH 17/28] Merge DisposePacketCache This also allows _writePacketCache to be migrated to TdsParserStateObjectNative in netfx. --- .../SqlClient/TdsParserStateObject.netcore.cs | 2 -- .../SqlClient/TdsParserStateObjectNative.cs | 12 +++++++++-- .../SqlClient/TdsParserStateObject.netfx.cs | 16 +-------------- .../SqlClient/TdsParserStateObjectNative.cs | 20 +++++++++++++++++++ .../Data/SqlClient/TdsParserStateObject.cs | 2 ++ 5 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 8c70cd8508..bb3b07b0e8 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -96,8 +96,6 @@ internal abstract void CreatePhysicalSNIHandle( internal abstract void Dispose(); - internal abstract void DisposePacketCache(); - internal abstract uint CheckConnection(); internal int DecrementPendingCallbacks(bool release) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index bbbe0f0416..10e9d96eca 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -445,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 + } } } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 2c8e89ab23..cf54e0340f 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -25,7 +25,6 @@ internal partial class TdsParserStateObject // SNI variables // multiple resultsets in one batch. protected SNIPacket _sniPacket = null; // Will have to re-vamp this for MARS internal SNIPacket _sniAsyncAttnPacket = null; // Packet to use to send Attn - protected readonly WritePacketCache _writePacketCache = new WritePacketCache(); // Store write packets that are ready to be re-used // Async variables private GCHandle _gcHandle; // keeps this object alive until we're closed. @@ -196,20 +195,7 @@ internal void Dispose() } } - if (_writePacketCache != null) - { - lock (_writePacketLockObject) - { - RuntimeHelpers.PrepareConstrainedRegions(); - try - { } - finally - { - _writePacketCache.Dispose(); - // Do not set _writePacketCache to null, just in case a WriteAsyncCallback completes after this point - } - } - } + DisposePacketCache(); } /// diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 49c1990401..2ad67cf2f4 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Runtime.CompilerServices; using System.Threading.Tasks; using Interop.Windows.Sni; using Microsoft.Data.Common; @@ -13,6 +14,8 @@ namespace Microsoft.Data.SqlClient { internal class TdsParserStateObjectNative : TdsParserStateObject { + private readonly WritePacketCache _writePacketCache = new WritePacketCache(); // Store write packets that are ready to be re-used + private readonly Dictionary _pendingWritePackets = new Dictionary(); // 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) @@ -172,6 +175,23 @@ internal override uint EnableMars(ref uint info) internal override uint SetConnectionBufferSize(ref uint unsignedPacketSize) => SniNativeWrapper.SniSetInfo(Handle, QueryType.SNI_QUERY_CONN_BUFSIZE, ref unsignedPacketSize); + internal override void DisposePacketCache() + { + lock (_writePacketLockObject) + { +#if NETFRAMEWORK + RuntimeHelpers.PrepareConstrainedRegions(); +#endif + try + { } + finally + { + _writePacketCache.Dispose(); + // Do not set _writePacketCache to null, just in case a WriteAsyncCallback completes after this point + } + } + } + internal override SspiContextProvider CreateSspiContextProvider() => new NativeSspiContextProvider(); } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index d7de640162..2eef6adbe0 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -519,6 +519,8 @@ internal long TimeoutTime internal abstract uint SetConnectionBufferSize(ref uint unsignedPacketSize); + internal abstract void DisposePacketCache(); + internal int GetTimeoutRemaining() { int remaining; From 469d8404d7b7c7f6342c690592e15d218fa0a8d1 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Thu, 29 May 2025 06:58:30 +0100 Subject: [PATCH 18/28] Merge CreateSessionHandle --- .../Data/SqlClient/TdsParserStateObject.netcore.cs | 4 +--- .../Data/SqlClient/TdsParserStateObject.netfx.cs | 10 +++------- .../Data/SqlClient/TdsParserStateObjectNative.cs | 14 +++++++++++++- .../Data/SqlClient/TdsParserStateObject.cs | 2 ++ 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index bb3b07b0e8..f3338544f6 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -31,7 +31,7 @@ internal static void PrepareConstrainedRegions() // Constructors // ////////////////// - internal TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalConnection, bool async) + protected TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalConnection, bool async) { // Construct a MARS session Debug.Assert(parser != null, "no parser?"); @@ -86,8 +86,6 @@ internal abstract void CreatePhysicalSNIHandle( internal abstract void AssignPendingDNSInfo(string userProtocol, string DNSCacheKey, ref SQLDNSInfo pendingDNSInfo); - protected abstract void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async); - protected abstract void FreeGcHandle(int remaining, bool release); internal abstract uint EnableSsl(ref uint info, bool tlsFirst, string serverCertificateFilename); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index cf54e0340f..e90bdcd23f 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -39,7 +39,7 @@ internal partial class TdsParserStateObject // Constructors // ////////////////// - protected TdsParserStateObject(TdsParser parser, SNIHandle physicalConnection, bool async) + protected TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalConnection, bool async) { // Construct a MARS session Debug.Assert(parser != null, "no parser?"); @@ -56,12 +56,8 @@ protected TdsParserStateObject(TdsParser parser, SNIHandle physicalConnection, b // Determine packet size based on physical connection buffer lengths. SetPacketSize(_parser._physicalStateObj._outBuff.Length); - ConsumerInfo myInfo = CreateConsumerInfo(async); - SQLDNSInfo cachedDNSInfo; - - SQLFallbackDNSCache.Instance.GetDNSInfo(_parser.FQDNforDNSCache, out cachedDNSInfo); + CreateSessionHandle(physicalConnection, async); - _sessionHandle = new SNIHandle(myInfo, physicalConnection, _parser.Connection.ConnectionOptions.IPAddressPreference, cachedDNSInfo); if (IsFailedHandle()) { AddError(parser.ProcessSNIError(this)); @@ -90,7 +86,7 @@ internal SNIHandle Handle // General methods // ///////////////////// - private ConsumerInfo CreateConsumerInfo(bool async) + protected ConsumerInfo CreateConsumerInfo(bool async) { ConsumerInfo myInfo = new ConsumerInfo(); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 2ad67cf2f4..b7954dcdbf 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -19,7 +19,7 @@ internal class TdsParserStateObjectNative : TdsParserStateObject private readonly Dictionary _pendingWritePackets = new Dictionary(); // 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.Handle, async) + : base(parser, physicalConnection, async) { } @@ -40,6 +40,18 @@ public TdsParserStateObjectNative(TdsParser parser) 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()); + TdsParserStateObjectNative nativeSNIObject = physicalConnection as TdsParserStateObjectNative; + ConsumerInfo myInfo = CreateConsumerInfo(async); + + SQLDNSInfo cachedDNSInfo; + bool ret = SQLFallbackDNSCache.Instance.GetDNSInfo(_parser.FQDNforDNSCache, out cachedDNSInfo); + + _sessionHandle = new SNIHandle(myInfo, nativeSNIObject.Handle, _parser.Connection.ConnectionOptions.IPAddressPreference, cachedDNSInfo); + } + protected override uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize) { Debug.Assert(packet.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 79a620ce36..7331f16d9d 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -483,6 +483,8 @@ internal long TimeoutTime protected abstract PacketHandle EmptyReadPacket { get; } + protected abstract void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async); + internal abstract PacketHandle GetResetWritePacket(int dataSize); protected abstract uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize); From 596ee2551f356965afd41bbb290f7493db86ec76 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Thu, 29 May 2025 07:08:29 +0100 Subject: [PATCH 19/28] Move AssignPendingDNSInfo from TdsParser, merge --- .../SqlClient/TdsParserStateObject.netcore.cs | 2 - .../SqlClient/TdsParserStateObjectNative.cs | 3 ++ .../src/Microsoft/Data/SqlClient/TdsParser.cs | 4 +- .../Data/SqlClient/TdsParser.netfx.cs | 54 ------------------- .../SqlClient/TdsParserStateObjectNative.cs | 54 +++++++++++++++++++ .../Data/SqlClient/TdsParserStateObject.cs | 2 + 6 files changed, 61 insertions(+), 58 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index f3338544f6..497426d32d 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -84,8 +84,6 @@ internal abstract void CreatePhysicalSNIHandle( string hostNameInCertificate = "", string serverCertificateFilename = ""); - internal abstract void AssignPendingDNSInfo(string userProtocol, string DNSCacheKey, ref SQLDNSInfo pendingDNSInfo); - protected abstract void FreeGcHandle(int remaining, bool release); internal abstract uint EnableSsl(ref uint info, bool tlsFirst, string serverCertificateFilename); diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 10e9d96eca..1cedb298dc 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -83,6 +83,9 @@ protected override void CreateSessionHandle(TdsParserStateObject physicalConnect _sessionHandle = new SNIHandle(myInfo, nativeSNIObject.Handle, _parser.Connection.ConnectionOptions.IPAddressPreference, cachedDNSInfo); } + // Retrieve the IP and port number from native SNI for TCP protocol. The IP information is stored temporarily in the + // pendingSQLDNSObject but not in the DNS Cache at this point. We only add items to the DNS Cache after we receive the + // IsSupported flag as true in the feature ext ack from server. internal override void AssignPendingDNSInfo(string userProtocol, string DNSCacheKey, ref SQLDNSInfo pendingDNSInfo) { uint result; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 3e9266f897..9396961059 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -559,7 +559,7 @@ internal void Connect(ServerInfo serverInfo, Debug.Assert(result == TdsEnums.SNI_SUCCESS, "Unexpected failure state upon calling SniGetConnectionId"); // for DNS Caching phase 1 - AssignPendingDNSInfo(serverInfo.UserProtocol, FQDNforDNSCache); + _physicalStateObj.AssignPendingDNSInfo(serverInfo.UserProtocol, FQDNforDNSCache, ref _connHandler.pendingSQLDNSObject); if (!ClientOSEncryptionSupport) { @@ -626,7 +626,7 @@ internal void Connect(ServerInfo serverInfo, SqlClientEventSource.Log.TryTraceEvent(" Sending prelogin handshake"); // for DNS Caching phase 1 - AssignPendingDNSInfo(serverInfo.UserProtocol, FQDNforDNSCache); + _physicalStateObj.AssignPendingDNSInfo(serverInfo.UserProtocol, FQDNforDNSCache, ref _connHandler.pendingSQLDNSObject); SendPreLoginHandshake(instanceName, encrypt, integratedSecurity, serverCertificateFilename); status = ConsumePreLoginHandshake( diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.netfx.cs index 690d0351ec..83cf7b1065 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.netfx.cs @@ -44,60 +44,6 @@ internal void BestEffortCleanup() } } - // Retrieve the IP and port number from native SNI for TCP protocol. The IP information is stored temporarily in the - // pendingSQLDNSObject but not in the DNS Cache at this point. We only add items to the DNS Cache after we receive the - // IsSupported flag as true in the feature ext ack from server. - internal void AssignPendingDNSInfo(string userProtocol, string DNSCacheKey) - { - uint result; - ushort portFromSNI = 0; - string IPStringFromSNI = string.Empty; - IPAddress IPFromSNI; - isTcpProtocol = false; - Provider providerNumber = Provider.INVALID_PROV; - - if (string.IsNullOrEmpty(userProtocol)) - { - - result = SniNativeWrapper.SniGetProviderNumber(_physicalStateObj.Handle, ref providerNumber); - Debug.Assert(result == TdsEnums.SNI_SUCCESS, "Unexpected failure state upon calling SniGetProviderNumber"); - isTcpProtocol = (providerNumber == Provider.TCP_PROV); - } - else if (userProtocol == TdsEnums.TCP) - { - isTcpProtocol = true; - } - - // serverInfo.UserProtocol could be empty - if (isTcpProtocol) - { - result = SniNativeWrapper.SniGetConnectionPort(_physicalStateObj.Handle, ref portFromSNI); - Debug.Assert(result == TdsEnums.SNI_SUCCESS, "Unexpected failure state upon calling SniGetConnectionPort"); - - - result = SniNativeWrapper.SniGetConnectionIpString(_physicalStateObj.Handle, ref IPStringFromSNI); - Debug.Assert(result == TdsEnums.SNI_SUCCESS, "Unexpected failure state upon calling SniGetConnectionIPString"); - - _connHandler.pendingSQLDNSObject = new SQLDNSInfo(DNSCacheKey, null, null, portFromSNI.ToString()); - - if (IPAddress.TryParse(IPStringFromSNI, out IPFromSNI)) - { - if (System.Net.Sockets.AddressFamily.InterNetwork == IPFromSNI.AddressFamily) - { - _connHandler.pendingSQLDNSObject.AddrIPv4 = IPStringFromSNI; - } - else if (System.Net.Sockets.AddressFamily.InterNetworkV6 == IPFromSNI.AddressFamily) - { - _connHandler.pendingSQLDNSObject.AddrIPv6 = IPStringFromSNI; - } - } - } - else - { - _connHandler.pendingSQLDNSObject = null; - } - } - internal bool RunReliably(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj) { RuntimeHelpers.PrepareConstrainedRegions(); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index b7954dcdbf..a9de1b91f0 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Net; using System.Runtime.CompilerServices; using System.Threading.Tasks; using Interop.Windows.Sni; @@ -52,6 +53,59 @@ protected override void CreateSessionHandle(TdsParserStateObject physicalConnect _sessionHandle = new SNIHandle(myInfo, nativeSNIObject.Handle, _parser.Connection.ConnectionOptions.IPAddressPreference, cachedDNSInfo); } + // Retrieve the IP and port number from native SNI for TCP protocol. The IP information is stored temporarily in the + // pendingSQLDNSObject but not in the DNS Cache at this point. We only add items to the DNS Cache after we receive the + // IsSupported flag as true in the feature ext ack from server. + internal override void AssignPendingDNSInfo(string userProtocol, string DNSCacheKey, ref SQLDNSInfo pendingDNSInfo) + { + uint result; + ushort portFromSNI = 0; + string IPStringFromSNI = string.Empty; + IPAddress IPFromSNI; + _parser.isTcpProtocol = false; + Provider providerNumber = Provider.INVALID_PROV; + + if (string.IsNullOrEmpty(userProtocol)) + { + + result = SniNativeWrapper.SniGetProviderNumber(Handle, ref providerNumber); + Debug.Assert(result == TdsEnums.SNI_SUCCESS, "Unexpected failure state upon calling SniGetProviderNumber"); + _parser.isTcpProtocol = (providerNumber == Provider.TCP_PROV); + } + else if (userProtocol == TdsEnums.TCP) + { + _parser.isTcpProtocol = true; + } + + // serverInfo.UserProtocol could be empty + if (_parser.isTcpProtocol) + { + result = SniNativeWrapper.SniGetConnectionPort(Handle, ref portFromSNI); + Debug.Assert(result == TdsEnums.SNI_SUCCESS, "Unexpected failure state upon calling SniGetConnectionPort"); + + result = SniNativeWrapper.SniGetConnectionIpString(Handle, ref IPStringFromSNI); + Debug.Assert(result == TdsEnums.SNI_SUCCESS, "Unexpected failure state upon calling SniGetConnectionIPString"); + + pendingDNSInfo = new SQLDNSInfo(DNSCacheKey, null, null, portFromSNI.ToString()); + + if (IPAddress.TryParse(IPStringFromSNI, out IPFromSNI)) + { + if (System.Net.Sockets.AddressFamily.InterNetwork == IPFromSNI.AddressFamily) + { + pendingDNSInfo.AddrIPv4 = IPStringFromSNI; + } + else if (System.Net.Sockets.AddressFamily.InterNetworkV6 == IPFromSNI.AddressFamily) + { + pendingDNSInfo.AddrIPv6 = IPStringFromSNI; + } + } + } + else + { + pendingDNSInfo = null; + } + } + protected override uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize) { Debug.Assert(packet.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 7331f16d9d..c233680088 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -485,6 +485,8 @@ internal long TimeoutTime protected abstract void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async); + internal abstract void AssignPendingDNSInfo(string userProtocol, string DNSCacheKey, ref SQLDNSInfo pendingDNSInfo); + internal abstract PacketHandle GetResetWritePacket(int dataSize); protected abstract uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize); From 3b8f8508efbdee8cf16bd7ed409e96e1f1243c50 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Thu, 29 May 2025 07:40:36 +0100 Subject: [PATCH 20/28] Sync method signature for CreatePhysicalSNIHandle --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 4 ++++ .../Data/SqlClient/TdsParserStateObject.netcore.cs | 2 ++ .../Data/SqlClient/TdsParserStateObjectManaged.cs | 2 ++ .../Data/SqlClient/TdsParserStateObjectNative.cs | 6 ++++-- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 14 ++++++++++++-- .../Data/SqlClient/TdsParserStateObject.netfx.cs | 11 ++++++++--- 6 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index c5d32a9a41..fe4327e44d 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -450,6 +450,8 @@ internal void Connect(ServerInfo serverInfo, false, true, fParallel, + TransparentNetworkResolutionState.DisabledMode, + -1, _connHandler.ConnectionOptions.IPAddressPreference, FQDNforDNSCache, ref _connHandler.pendingSQLDNSObject, @@ -550,6 +552,8 @@ internal void Connect(ServerInfo serverInfo, true, true, fParallel, + TransparentNetworkResolutionState.DisabledMode, + -1, _connHandler.ConnectionOptions.IPAddressPreference, FQDNforDNSCache, ref _connHandler.pendingSQLDNSObject, diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 497426d32d..9c6abeaf18 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -75,6 +75,8 @@ internal abstract void CreatePhysicalSNIHandle( bool flushCache, bool async, bool fParallel, + TransparentNetworkResolutionState transparentNetworkResolutionState, + int totalTimeout, SqlConnectionIPAddressPreference iPAddressPreference, string cachedFQDN, ref SQLDNSInfo pendingDNSInfo, diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs index e6dddc79f9..75e715f4dd 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs @@ -85,6 +85,8 @@ internal override void CreatePhysicalSNIHandle( bool flushCache, bool async, bool parallel, + TransparentNetworkResolutionState transparentNetworkResolutionState, + int totalTimeout, SqlConnectionIPAddressPreference iPAddressPreference, string cachedFQDN, ref SQLDNSInfo pendingDNSInfo, diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 1cedb298dc..0667625d89 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -162,7 +162,9 @@ internal override void CreatePhysicalSNIHandle( bool flushCache, bool async, bool fParallel, - SqlConnectionIPAddressPreference ipPreference, + TransparentNetworkResolutionState transparentNetworkResolutionState, + int totalTimeout, + SqlConnectionIPAddressPreference iPAddressPreference, string cachedFQDN, ref SQLDNSInfo pendingDNSInfo, string serverSPN, @@ -191,7 +193,7 @@ internal override void CreatePhysicalSNIHandle( bool ret = SQLFallbackDNSCache.Instance.GetDNSInfo(cachedFQDN, out cachedDNSInfo); _sessionHandle = new SNIHandle(myInfo, serverName, ref serverSPN, timeout.MillisecondsRemainingInt, out instanceName, - flushCache, !async, fParallel, ipPreference, cachedDNSInfo, hostNameInCertificate); + flushCache, !async, fParallel, iPAddressPreference, cachedDNSInfo, hostNameInCertificate); spns = new[] { serverSPN.TrimEnd() }; } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 9396961059..b1b2cc8a81 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -516,7 +516,12 @@ internal void Connect(ServerInfo serverInfo, totalTimeout, _connHandler.ConnectionOptions.IPAddressPreference, FQDNforDNSCache, - hostNameInCertificate); + ref _connHandler.pendingSQLDNSObject, + serverInfo.ServerSPN, + integratedSecurity || authType == SqlAuthenticationMethod.ActiveDirectoryIntegrated, + isTlsFirst, + hostNameInCertificate, + serverCertificateFilename); _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this); @@ -610,7 +615,12 @@ internal void Connect(ServerInfo serverInfo, totalTimeout, _connHandler.ConnectionOptions.IPAddressPreference, serverInfo.ResolvedServerName, - hostNameInCertificate); + ref _connHandler.pendingSQLDNSObject, + serverInfo.ServerSPN, + integratedSecurity, + isTlsFirst, + hostNameInCertificate, + serverCertificateFilename); _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index e90bdcd23f..fc014617cc 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -114,9 +114,14 @@ internal void CreatePhysicalSNIHandle( bool fParallel, TransparentNetworkResolutionState transparentNetworkResolutionState, int totalTimeout, - SqlConnectionIPAddressPreference ipPreference, + SqlConnectionIPAddressPreference iPAddressPreference, string cachedFQDN, - string hostNameInCertificate = "") + ref SQLDNSInfo pendingDNSInfo, + string serverSPN, + bool isIntegratedSecurity = false, + bool tlsFirst = false, + string hostNameInCertificate = "", + string serverCertificateFilename = "") { ConsumerInfo myInfo = CreateConsumerInfo(async); @@ -127,7 +132,7 @@ internal void CreatePhysicalSNIHandle( _sessionHandle = new SNIHandle(myInfo, serverName, ref spn, timeout.MillisecondsRemainingInt, out instanceName, flushCache, !async, fParallel, transparentNetworkResolutionState, totalTimeout, - ipPreference, cachedDNSInfo, hostNameInCertificate); + iPAddressPreference, cachedDNSInfo, hostNameInCertificate); } internal uint CheckConnection() => SniNativeWrapper.SniCheckConnection(Handle); From dfeb3605da5529b2702895862db1a4fefe3b44b5 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Tue, 3 Jun 2025 07:04:54 +0100 Subject: [PATCH 21/28] Port netcore _serverSpn array to netfx --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 25 ++++++++----------- .../SqlClient/TdsParserStateObject.netfx.cs | 18 +++++++++++-- .../SqlClient/SSPI/SspiContextProvider.cs | 11 -------- 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index b1b2cc8a81..be747aec59 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -121,7 +121,7 @@ internal sealed partial class TdsParser private bool _is2022 = false; - private string _serverSpn = null; + private string[] _serverSpn = null; // SqlStatistics private SqlStatistics _statistics = null; @@ -395,6 +395,12 @@ internal void Connect(ServerInfo serverInfo, ThrowExceptionAndWarning(_physicalStateObj); Debug.Fail("SNI returned status != success, but no error thrown?"); } + else + { + _serverSpn = null; + SqlClientEventSource.Log.TryTraceEvent("TdsParser.Connect | SEC | Connection Object Id {0}, Authentication Mode: {1}", _connHandler.ObjectID, + authType == SqlAuthenticationMethod.NotSpecified ? SqlAuthenticationMethod.SqlPassword.ToString() : authType.ToString()); + } //Create LocalDB instance if necessary if (connHandler.ConnectionOptions.LocalDBInstance != null) @@ -408,28 +414,17 @@ internal void Connect(ServerInfo serverInfo, } } + _serverSpn = null; + // AD Integrated behaves like Windows integrated when connecting to a non-fedAuth server if (integratedSecurity || authType == SqlAuthenticationMethod.ActiveDirectoryIntegrated) { _authenticationProvider = _physicalStateObj.CreateSspiContextProvider(); - - if (!string.IsNullOrEmpty(serverInfo.ServerSPN)) - { - _serverSpn = serverInfo.ServerSPN; - SqlClientEventSource.Log.TryTraceEvent(" Server SPN `{0}` from the connection string is used.", serverInfo.ServerSPN); - } - else - { - // Empty signifies to interop layer that SPN needs to be generated - _serverSpn = string.Empty; - } - SqlClientEventSource.Log.TryTraceEvent(" SSPI or Active Directory Authentication Library for SQL Server based integrated authentication"); } else { _authenticationProvider = null; - _serverSpn = null; switch (authType) { @@ -614,7 +609,7 @@ internal void Connect(ServerInfo serverInfo, transparentNetworkResolutionState, totalTimeout, _connHandler.ConnectionOptions.IPAddressPreference, - serverInfo.ResolvedServerName, + FQDNforDNSCache, ref _connHandler.pendingSQLDNSObject, serverInfo.ServerSPN, integratedSecurity, diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index fc014617cc..7a30640a6b 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -108,7 +108,7 @@ internal void CreatePhysicalSNIHandle( string serverName, TimeoutTimer timeout, out byte[] instanceName, - ref string spn, + ref string[] spns, bool flushCache, bool async, bool fParallel, @@ -123,6 +123,19 @@ internal void CreatePhysicalSNIHandle( string hostNameInCertificate = "", string serverCertificateFilename = "") { + if (isIntegratedSecurity) + { + if (!string.IsNullOrEmpty(serverSPN)) + { + SqlClientEventSource.Log.TryTraceEvent(" Server SPN `{0}` from the connection string is used.", serverSPN); + } + else + { + // Empty signifies to interop layer that SPN needs to be generated + serverSPN = string.Empty; + } + } + ConsumerInfo myInfo = CreateConsumerInfo(async); // serverName : serverInfo.ExtendedServerName @@ -130,9 +143,10 @@ internal void CreatePhysicalSNIHandle( _ = SQLFallbackDNSCache.Instance.GetDNSInfo(cachedFQDN, out SQLDNSInfo cachedDNSInfo); - _sessionHandle = new SNIHandle(myInfo, serverName, ref spn, timeout.MillisecondsRemainingInt, + _sessionHandle = new SNIHandle(myInfo, serverName, ref serverSPN, timeout.MillisecondsRemainingInt, out instanceName, flushCache, !async, fParallel, transparentNetworkResolutionState, totalTimeout, iPAddressPreference, cachedDNSInfo, hostNameInCertificate); + spns = new[] { serverSPN.TrimEnd() }; } internal uint CheckConnection() => SniNativeWrapper.SniCheckConnection(Handle); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs index ff83422f10..60872cf416 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs @@ -27,17 +27,6 @@ private protected virtual void Initialize() protected abstract bool GenerateSspiClientContext(ReadOnlySpan incomingBlob, IBufferWriter outgoingBlobWriter, SspiAuthenticationParameters authParams); - internal void SSPIData(ReadOnlySpan receivedBuff, IBufferWriter outgoingBlobWriter, string serverSpn) - { - using var _ = TrySNIEventScope.Create(nameof(SspiContextProvider)); - - if (!RunGenerateSspiClientContext(receivedBuff, outgoingBlobWriter, serverSpn)) - { - // If we've hit here, the SSPI context provider implementation failed to generate the SSPI context. - SSPIError(SQLMessage.SSPIGenerateError(), TdsEnums.GEN_CLIENT_CONTEXT); - } - } - internal void SSPIData(ReadOnlySpan receivedBuff, IBufferWriter outgoingBlobWriter, ReadOnlySpan serverSpns) { using var _ = TrySNIEventScope.Create(nameof(SspiContextProvider)); From 4ab9be14928b4e522981dfde497901f841c89c61 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Tue, 3 Jun 2025 07:10:35 +0100 Subject: [PATCH 22/28] Merge CreatePhysicalSNIHandle --- .../SqlClient/TdsParserStateObject.netcore.cs | 19 -------- .../SqlClient/TdsParserStateObject.netfx.cs | 45 ------------------ .../SqlClient/TdsParserStateObjectNative.cs | 46 +++++++++++++++++++ .../Data/SqlClient/TdsParserStateObject.cs | 21 +++++++++ 4 files changed, 67 insertions(+), 64 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 9c6abeaf18..71ec6d2039 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -67,25 +67,6 @@ protected TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalCo // General methods // ///////////////////// - internal abstract void CreatePhysicalSNIHandle( - string serverName, - TimeoutTimer timeout, - out byte[] instanceName, - ref string[] spns, - bool flushCache, - bool async, - bool fParallel, - TransparentNetworkResolutionState transparentNetworkResolutionState, - int totalTimeout, - SqlConnectionIPAddressPreference iPAddressPreference, - string cachedFQDN, - ref SQLDNSInfo pendingDNSInfo, - string serverSPN, - bool isIntegratedSecurity = false, - bool tlsFirst = false, - string hostNameInCertificate = "", - string serverCertificateFilename = ""); - protected abstract void FreeGcHandle(int remaining, bool release); internal abstract uint EnableSsl(ref uint info, bool tlsFirst, string serverCertificateFilename); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 7a30640a6b..eb6021b838 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -104,51 +104,6 @@ protected ConsumerInfo CreateConsumerInfo(bool async) return myInfo; } - internal void CreatePhysicalSNIHandle( - string serverName, - TimeoutTimer timeout, - out byte[] instanceName, - ref string[] spns, - bool flushCache, - bool async, - bool fParallel, - TransparentNetworkResolutionState transparentNetworkResolutionState, - int totalTimeout, - SqlConnectionIPAddressPreference iPAddressPreference, - string cachedFQDN, - ref SQLDNSInfo pendingDNSInfo, - string serverSPN, - bool isIntegratedSecurity = false, - bool tlsFirst = false, - string hostNameInCertificate = "", - string serverCertificateFilename = "") - { - if (isIntegratedSecurity) - { - if (!string.IsNullOrEmpty(serverSPN)) - { - SqlClientEventSource.Log.TryTraceEvent(" Server SPN `{0}` from the connection string is used.", serverSPN); - } - else - { - // Empty signifies to interop layer that SPN needs to be generated - serverSPN = string.Empty; - } - } - - ConsumerInfo myInfo = CreateConsumerInfo(async); - - // serverName : serverInfo.ExtendedServerName - // may not use this serverName as key - - _ = SQLFallbackDNSCache.Instance.GetDNSInfo(cachedFQDN, out SQLDNSInfo cachedDNSInfo); - - _sessionHandle = new SNIHandle(myInfo, serverName, ref serverSPN, timeout.MillisecondsRemainingInt, - out instanceName, flushCache, !async, fParallel, transparentNetworkResolutionState, totalTimeout, - iPAddressPreference, cachedDNSInfo, hostNameInCertificate); - spns = new[] { serverSPN.TrimEnd() }; - } - internal uint CheckConnection() => SniNativeWrapper.SniCheckConnection(Handle); [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index a9de1b91f0..728923eeb4 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -10,6 +10,7 @@ using System.Threading.Tasks; using Interop.Windows.Sni; using Microsoft.Data.Common; +using Microsoft.Data.ProviderBase; namespace Microsoft.Data.SqlClient { @@ -106,6 +107,51 @@ internal override void AssignPendingDNSInfo(string userProtocol, string DNSCache } } + internal override void CreatePhysicalSNIHandle( + string serverName, + TimeoutTimer timeout, + out byte[] instanceName, + ref string[] spns, + bool flushCache, + bool async, + bool fParallel, + TransparentNetworkResolutionState transparentNetworkResolutionState, + int totalTimeout, + SqlConnectionIPAddressPreference iPAddressPreference, + string cachedFQDN, + ref SQLDNSInfo pendingDNSInfo, + string serverSPN, + bool isIntegratedSecurity = false, + bool tlsFirst = false, + string hostNameInCertificate = "", + string serverCertificateFilename = "") + { + if (isIntegratedSecurity) + { + if (!string.IsNullOrEmpty(serverSPN)) + { + SqlClientEventSource.Log.TryTraceEvent(" Server SPN `{0}` from the connection string is used.", serverSPN); + } + else + { + // Empty signifies to interop layer that SPN needs to be generated + serverSPN = string.Empty; + } + } + + ConsumerInfo myInfo = CreateConsumerInfo(async); + + // serverName : serverInfo.ExtendedServerName + // may not use this serverName as key + + _ = SQLFallbackDNSCache.Instance.GetDNSInfo(cachedFQDN, out SQLDNSInfo cachedDNSInfo); + + _sessionHandle = new SNIHandle(myInfo, serverName, ref serverSPN, timeout.MillisecondsRemainingInt, + out instanceName, flushCache, !async, fParallel, transparentNetworkResolutionState, totalTimeout, + iPAddressPreference, cachedDNSInfo, hostNameInCertificate); + spns = new[] { serverSPN.TrimEnd() }; + } + protected override uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize) { Debug.Assert(packet.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index c233680088..9768badec0 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -12,6 +12,8 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.Data.Common; +using Microsoft.Data.ProviderBase; + #if NETFRAMEWORK using System.Runtime.ConstrainedExecution; #endif @@ -487,6 +489,25 @@ internal long TimeoutTime internal abstract void AssignPendingDNSInfo(string userProtocol, string DNSCacheKey, ref SQLDNSInfo pendingDNSInfo); + internal abstract void CreatePhysicalSNIHandle( + string serverName, + TimeoutTimer timeout, + out byte[] instanceName, + ref string[] spns, + bool flushCache, + bool async, + bool fParallel, + TransparentNetworkResolutionState transparentNetworkResolutionState, + int totalTimeout, + SqlConnectionIPAddressPreference iPAddressPreference, + string cachedFQDN, + ref SQLDNSInfo pendingDNSInfo, + string serverSPN, + bool isIntegratedSecurity = false, + bool tlsFirst = false, + string hostNameInCertificate = "", + string serverCertificateFilename = ""); + internal abstract PacketHandle GetResetWritePacket(int dataSize); protected abstract uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize); From 1e4724e10be659ce24ac30288133851dad7689fc Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Tue, 3 Jun 2025 07:29:12 +0100 Subject: [PATCH 23/28] Refactor to insert FreeGcHandle --- .../Data/SqlClient/TdsParserStateObject.netcore.cs | 4 ++-- .../Data/SqlClient/TdsParserStateObject.netfx.cs | 8 ++------ .../Data/SqlClient/TdsParserStateObjectNative.cs | 12 ++++++++++++ .../Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 ++ 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 71ec6d2039..93caeeb000 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -67,8 +67,6 @@ protected TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalCo // General methods // ///////////////////// - protected abstract void FreeGcHandle(int remaining, bool release); - internal abstract uint EnableSsl(ref uint info, bool tlsFirst, string serverCertificateFilename); internal abstract uint WaitForSSLHandShakeToComplete(out int protocolVersion); @@ -81,7 +79,9 @@ internal int DecrementPendingCallbacks(bool release) { int remaining = Interlocked.Decrement(ref _pendingCallbacks); SqlClientEventSource.Log.TryAdvancedTraceEvent("TdsParserStateObject.DecrementPendingCallbacks | ADV | State Object Id {0}, after decrementing _pendingCallbacks: {1}", _objectID, _pendingCallbacks); + FreeGcHandle(remaining, release); + // NOTE: TdsParserSessionPool may call DecrementPendingCallbacks on a TdsParserStateObject which is already disposed // This is not dangerous (since the stateObj is no longer in use), but we need to add a workaround in the assert for it Debug.Assert((remaining == -1 && SessionHandle.IsNull) || (0 <= remaining && remaining < 3), $"_pendingCallbacks values is invalid after decrementing: {remaining}"); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index eb6021b838..04d2e696c2 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -27,7 +27,7 @@ internal partial class TdsParserStateObject internal SNIPacket _sniAsyncAttnPacket = null; // Packet to use to send Attn // Async variables - private GCHandle _gcHandle; // keeps this object alive until we're closed. + protected GCHandle _gcHandle; // keeps this object alive until we're closed. // Used for blanking out password in trace. internal int _tracePasswordOffset = 0; @@ -112,11 +112,7 @@ internal int DecrementPendingCallbacks(bool release) int remaining = Interlocked.Decrement(ref _pendingCallbacks); SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, after decrementing _pendingCallbacks: {1}", ObjectID, _pendingCallbacks); - if ((0 == remaining || release) && _gcHandle.IsAllocated) - { - SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, FREEING HANDLE!", ObjectID); - _gcHandle.Free(); - } + FreeGcHandle(remaining, release); // NOTE: TdsParserSessionPool may call DecrementPendingCallbacks on a TdsParserStateObject which is already disposed // This is not dangerous (since the stateObj is no longer in use), but we need to add a workaround in the assert for it diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 728923eeb4..5a9723850d 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -7,6 +7,8 @@ using System.Diagnostics; using System.Net; using System.Runtime.CompilerServices; +using System.Runtime.ConstrainedExecution; +using System.Runtime.InteropServices; using System.Threading.Tasks; using Interop.Windows.Sni; using Microsoft.Data.Common; @@ -186,6 +188,16 @@ protected override void RemovePacketFromPendingList(PacketHandle ptr) } } + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + protected override void FreeGcHandle(int remaining, bool release) + { + if ((0 == remaining || release) && _gcHandle.IsAllocated) + { + SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, FREEING HANDLE!", ObjectID); + _gcHandle.Free(); + } + } + internal override bool IsFailedHandle() => _sessionHandle.Status != TdsEnums.SNI_SUCCESS; internal override bool IsPacketEmpty(PacketHandle readPacket) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 9768badec0..5614734543 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -514,6 +514,8 @@ internal abstract void CreatePhysicalSNIHandle( protected abstract bool CheckPacket(PacketHandle packet, TaskCompletionSource source); + protected abstract void FreeGcHandle(int remaining, bool release); + internal abstract bool IsFailedHandle(); internal abstract bool IsPacketEmpty(PacketHandle readPacket); From f824db70ec1d85cc0b8735a560637ba8a5d60d58 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Tue, 3 Jun 2025 07:32:59 +0100 Subject: [PATCH 24/28] Move CreateConsumerInfo and _gcHandle to TdsParserStateObjectNative --- .../SqlClient/TdsParserStateObject.netfx.cs | 21 ------------------- .../SqlClient/TdsParserStateObjectNative.cs | 20 ++++++++++++++++++ 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 04d2e696c2..28225da70e 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -26,9 +26,6 @@ internal partial class TdsParserStateObject protected SNIPacket _sniPacket = null; // Will have to re-vamp this for MARS internal SNIPacket _sniAsyncAttnPacket = null; // Packet to use to send Attn - // Async variables - protected GCHandle _gcHandle; // keeps this object alive until we're closed. - // Used for blanking out password in trace. internal int _tracePasswordOffset = 0; internal int _tracePasswordLength = 0; @@ -86,24 +83,6 @@ internal SNIHandle Handle // General methods // ///////////////////// - protected ConsumerInfo CreateConsumerInfo(bool async) - { - ConsumerInfo myInfo = new ConsumerInfo(); - - Debug.Assert(_outBuff.Length == _inBuff.Length, "Unexpected unequal buffers."); - - myInfo.defaultBufferSize = _outBuff.Length; // Obtain packet size from outBuff size. - - if (async) - { - myInfo.readDelegate = SNILoadHandle.SingletonInstance.ReadAsyncCallbackDispatcher; - myInfo.writeDelegate = SNILoadHandle.SingletonInstance.WriteAsyncCallbackDispatcher; - _gcHandle = GCHandle.Alloc(this, GCHandleType.Normal); - myInfo.key = (IntPtr)_gcHandle; - } - return myInfo; - } - internal uint CheckConnection() => SniNativeWrapper.SniCheckConnection(Handle); [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 5a9723850d..24ceb1dae2 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -20,6 +20,8 @@ internal class TdsParserStateObjectNative : TdsParserStateObject { private readonly WritePacketCache _writePacketCache = new WritePacketCache(); // Store write packets that are ready to be re-used + private GCHandle _gcHandle; // keeps this object alive until we're closed. + private readonly Dictionary _pendingWritePackets = new Dictionary(); // 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) @@ -109,6 +111,24 @@ internal override void AssignPendingDNSInfo(string userProtocol, string DNSCache } } + private ConsumerInfo CreateConsumerInfo(bool async) + { + ConsumerInfo myInfo = new ConsumerInfo(); + + Debug.Assert(_outBuff.Length == _inBuff.Length, "Unexpected unequal buffers."); + + myInfo.defaultBufferSize = _outBuff.Length; // Obtain packet size from outBuff size. + + if (async) + { + myInfo.readDelegate = SNILoadHandle.SingletonInstance.ReadAsyncCallbackDispatcher; + myInfo.writeDelegate = SNILoadHandle.SingletonInstance.WriteAsyncCallbackDispatcher; + _gcHandle = GCHandle.Alloc(this, GCHandleType.Normal); + myInfo.key = (IntPtr)_gcHandle; + } + return myInfo; + } + internal override void CreatePhysicalSNIHandle( string serverName, TimeoutTimer timeout, From ab275139edcc503cf397f1d7e4ea9c431d927bcf Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Tue, 3 Jun 2025 07:43:17 +0100 Subject: [PATCH 25/28] Merge Dispose --- .../SqlClient/TdsParserStateObject.netcore.cs | 2 - .../SqlClient/TdsParserStateObject.netfx.cs | 44 ------------------- .../SqlClient/TdsParserStateObjectNative.cs | 44 +++++++++++++++++++ .../Data/SqlClient/TdsParserStateObject.cs | 2 + 4 files changed, 46 insertions(+), 46 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 93caeeb000..7ad67b522b 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -71,8 +71,6 @@ protected TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalCo internal abstract uint WaitForSSLHandShakeToComplete(out int protocolVersion); - internal abstract void Dispose(); - internal abstract uint CheckConnection(); internal int DecrementPendingCallbacks(bool release) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 28225da70e..1e645ceadd 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -99,50 +99,6 @@ internal int DecrementPendingCallbacks(bool release) return remaining; } - internal void Dispose() - { - - SafeHandle packetHandle = _sniPacket; - SafeHandle sessionHandle = _sessionHandle; - SafeHandle asyncAttnPacket = _sniAsyncAttnPacket; - _sniPacket = null; - _sessionHandle = null; - _sniAsyncAttnPacket = null; - - DisposeCounters(); - - if (sessionHandle != null || packetHandle != null) - { - // Comment CloseMARSSession - // UNDONE - if there are pending reads or writes on logical connections, we need to block - // here for the callbacks!!! This only applies to async. Should be fixed by async fixes for - // AD unload/exit. - - // TODO: Make this a BID trace point! - RuntimeHelpers.PrepareConstrainedRegions(); - try - { } - finally - { - if (packetHandle != null) - { - packetHandle.Dispose(); - } - if (asyncAttnPacket != null) - { - asyncAttnPacket.Dispose(); - } - if (sessionHandle != null) - { - sessionHandle.Dispose(); - DecrementPendingCallbacks(true); // Will dispose of GC handle. - } - } - } - - DisposePacketCache(); - } - /// /// Checks to see if the underlying connection is still valid (used by idle connection resiliency - for active connections) /// NOTE: This is not safe to do on a connection that is currently in use diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 24ceb1dae2..22f59718ab 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -208,6 +208,50 @@ protected override void RemovePacketFromPendingList(PacketHandle ptr) } } + internal override void Dispose() + { + + SafeHandle packetHandle = _sniPacket; + SafeHandle sessionHandle = _sessionHandle; + SafeHandle asyncAttnPacket = _sniAsyncAttnPacket; + _sniPacket = null; + _sessionHandle = null; + _sniAsyncAttnPacket = null; + + DisposeCounters(); + + if (sessionHandle != null || packetHandle != null) + { + // Comment CloseMARSSession + // UNDONE - if there are pending reads or writes on logical connections, we need to block + // here for the callbacks!!! This only applies to async. Should be fixed by async fixes for + // AD unload/exit. + + // TODO: Make this a BID trace point! + RuntimeHelpers.PrepareConstrainedRegions(); + try + { } + finally + { + if (packetHandle != null) + { + packetHandle.Dispose(); + } + if (asyncAttnPacket != null) + { + asyncAttnPacket.Dispose(); + } + if (sessionHandle != null) + { + sessionHandle.Dispose(); + DecrementPendingCallbacks(true); // Will dispose of GC handle. + } + } + } + + DisposePacketCache(); + } + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] protected override void FreeGcHandle(int remaining, bool release) { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 5614734543..fd5417f6e5 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -514,6 +514,8 @@ internal abstract void CreatePhysicalSNIHandle( protected abstract bool CheckPacket(PacketHandle packet, TaskCompletionSource source); + internal abstract void Dispose(); + protected abstract void FreeGcHandle(int remaining, bool release); internal abstract bool IsFailedHandle(); From 72d9c8ddeff0e544e713d94e90a9bafd92ac6da5 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Mon, 23 Jun 2025 20:34:34 +0100 Subject: [PATCH 26/28] Add region to both TdsParserStateObjectNative files --- .../Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs | 6 +++--- .../Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 0667625d89..32e5b6f8b4 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -57,9 +57,7 @@ public TdsParserStateObjectNative(TdsParser parser) { } - //////////////// - // Properties // - //////////////// + #region Properties internal SNIHandle Handle => _sessionHandle; @@ -71,6 +69,8 @@ public TdsParserStateObjectNative(TdsParser parser) internal override Guid? SessionId => default; + #endregion + protected override void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async) { Debug.Assert(physicalConnection is TdsParserStateObjectNative, "Expected a stateObject of type " + this.GetType()); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 22f59718ab..dc887ec6cd 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -34,9 +34,7 @@ public TdsParserStateObjectNative(TdsParser parser) { } - //////////////// - // Properties // - //////////////// + #region Properties internal override uint Status => _sessionHandle != null ? _sessionHandle.Status : TdsEnums.SNI_UNINITIALIZED; @@ -46,6 +44,8 @@ public TdsParserStateObjectNative(TdsParser parser) internal override Guid? SessionId => default; + #endregion + protected override void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async) { Debug.Assert(physicalConnection is TdsParserStateObjectNative, "Expected a stateObject of type " + this.GetType()); From 76a97d39e9610b5a1019ce86932a90c0e85dd14b Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Mon, 23 Jun 2025 20:36:18 +0100 Subject: [PATCH 27/28] Make public constructor internal --- .../src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs | 2 +- .../src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 32e5b6f8b4..312c1eb368 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -52,7 +52,7 @@ internal TdsParserStateObjectNative(TdsParser parser, TdsParserStateObject physi { } - public TdsParserStateObjectNative(TdsParser parser) + internal TdsParserStateObjectNative(TdsParser parser) : base(parser) { } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index dc887ec6cd..005d2c5763 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -29,7 +29,7 @@ internal TdsParserStateObjectNative(TdsParser parser, TdsParserStateObject physi { } - public TdsParserStateObjectNative(TdsParser parser) + internal TdsParserStateObjectNative(TdsParser parser) : base(parser) { } From 879246a2bdb08641a3fef196acb2c480f4674e95 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Wed, 2 Jul 2025 21:32:52 +0100 Subject: [PATCH 28/28] Remove redundant assignment to _serverSpn --- .../netcore/src/Microsoft/Data/SqlClient/TdsParser.cs | 1 - .../netfx/src/Microsoft/Data/SqlClient/TdsParser.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index 8ffc884d61..a0cf118574 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -395,7 +395,6 @@ internal void Connect(ServerInfo serverInfo, } else { - _serverSpn = null; SqlClientEventSource.Log.TryTraceEvent("TdsParser.Connect | SEC | Connection Object Id {0}, Authentication Mode: {1}", _connHandler.ObjectID, authType == SqlAuthenticationMethod.NotSpecified ? SqlAuthenticationMethod.SqlPassword.ToString() : authType.ToString()); } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 4868affbd7..33f4cedf9a 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -397,7 +397,6 @@ internal void Connect(ServerInfo serverInfo, } else { - _serverSpn = null; SqlClientEventSource.Log.TryTraceEvent("TdsParser.Connect | SEC | Connection Object Id {0}, Authentication Mode: {1}", _connHandler.ObjectID, authType == SqlAuthenticationMethod.NotSpecified ? SqlAuthenticationMethod.SqlPassword.ToString() : authType.ToString()); }