From ba78a6136080a5c87190db1023d9d262ddb3fae2 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Wed, 12 Dec 2018 21:54:13 +0000 Subject: [PATCH 01/10] change TdsParserStateObject to pass packets using a ref struct to avoid boxing of IntPtr in native mode --- .../Data/SqlClient/SNI/SNIMarsHandle.cs | 6 +- .../System/Data/SqlClient/SNI/SNIPacket.cs | 8 +- .../Data/SqlClient/TdsParser.Windows.cs | 16 +- .../Data/SqlClient/TdsParserSafeHandles.cs | 22 +-- .../Data/SqlClient/TdsParserStateObject.cs | 168 ++++++++++++++---- .../TdsParserStateObjectFactory.Windows.cs | 6 + .../SqlClient/TdsParserStateObjectManaged.cs | 58 +++--- .../SqlClient/TdsParserStateObjectNative.cs | 101 +++++++---- 8 files changed, 256 insertions(+), 129 deletions(-) diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsHandle.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsHandle.cs index a8b36737a614..d7b3242b0c5f 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsHandle.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIMarsHandle.cs @@ -318,7 +318,7 @@ public void HandleReceiveError(SNIPacket packet) _packetEvent.Set(); } - ((TdsParserStateObject)_callbackObject).ReadAsyncCallback(packet, 1); + ((TdsParserStateObject)_callbackObject).ReadAsyncCallback(PacketHandle.FromManagedPacket(packet), 1); } /// @@ -332,7 +332,7 @@ public void HandleSendComplete(SNIPacket packet, uint sniErrorCode) { Debug.Assert(_callbackObject != null); - ((TdsParserStateObject)_callbackObject).WriteAsyncCallback(packet, sniErrorCode); + ((TdsParserStateObject)_callbackObject).WriteAsyncCallback(PacketHandle.FromManagedPacket(packet), sniErrorCode); } } @@ -378,7 +378,7 @@ public void HandleReceiveComplete(SNIPacket packet, SNISMUXHeader header) _asyncReceives--; Debug.Assert(_callbackObject != null); - ((TdsParserStateObject)_callbackObject).ReadAsyncCallback(packet, 0); + ((TdsParserStateObject)_callbackObject).ReadAsyncCallback(PacketHandle.FromManagedPacket(packet), 0); } } diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIPacket.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIPacket.cs index 84a47740fabf..acc03819182e 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIPacket.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNIPacket.cs @@ -20,8 +20,6 @@ internal class SNIPacket : IDisposable, IEquatable private int _offset; private string _description; private SNIAsyncCallback _completionCallback; - - private ArrayPool _arrayPool = ArrayPool.Shared; private bool _isBufferFromArrayPool = false; public SNIPacket() { } @@ -98,14 +96,14 @@ public void Allocate(int capacity) { if (_isBufferFromArrayPool) { - _arrayPool.Return(_data); + ArrayPool.Shared.Return(_data); } _data = null; } if (_data == null) { - _data = _arrayPool.Rent(capacity); + _data = ArrayPool.Shared.Rent(capacity); _isBufferFromArrayPool = true; } @@ -221,7 +219,7 @@ public void Release() { if(_isBufferFromArrayPool) { - _arrayPool.Return(_data); + ArrayPool.Shared.Return(_data); } _data = null; _capacity = 0; diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.Windows.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.Windows.cs index 12ce515bf93d..6fec58ad11dd 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.Windows.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.Windows.cs @@ -20,20 +20,22 @@ internal void PostReadAsyncForMars() // Have to post read to initialize MARS - will get callback on this when connection goes // down or is closed. - IntPtr temp = IntPtr.Zero; + PacketHandle temp = default; uint error = TdsEnums.SNI_SUCCESS; _pMarsPhysicalConObj.IncrementPendingCallbacks(); - object handle = _pMarsPhysicalConObj.SessionHandle; - temp = (IntPtr)_pMarsPhysicalConObj.ReadAsync(out error, ref handle); + SessionHandle handle = _pMarsPhysicalConObj.SessionHandle; + temp = _pMarsPhysicalConObj.ReadAsync(handle, out error); - if (temp != IntPtr.Zero) + Debug.Assert(temp.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + + if (temp.NativePointer != IntPtr.Zero) { // Be sure to release packet, otherwise it will be leaked by native. _pMarsPhysicalConObj.ReleasePacket(temp); } - - Debug.Assert(IntPtr.Zero == temp, "unexpected syncReadPacket without corresponding SNIPacketRelease"); + + Debug.Assert(IntPtr.Zero == temp.NativePointer, "unexpected syncReadPacket without corresponding SNIPacketRelease"); if (TdsEnums.SNI_SUCCESS_IO_PENDING != error) { Debug.Assert(TdsEnums.SNI_SUCCESS != error, "Unexpected successful read async on physical connection before enabling MARS!"); @@ -118,4 +120,4 @@ private SNIErrorDetails GetSniErrorDetails() } } // tdsparser -}//namespace \ No newline at end of file +}//namespace diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserSafeHandles.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserSafeHandles.cs index 55cd8a1c5fb5..4360ab64d5ab 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserSafeHandles.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserSafeHandles.cs @@ -104,7 +104,7 @@ private static void ReadDispatcher(IntPtr key, IntPtr packet, uint error) if (null != stateObj) { - stateObj.ReadAsyncCallback(IntPtr.Zero, packet, error); + stateObj.ReadAsyncCallback(IntPtr.Zero, PacketHandle.FromNativePointer(packet), error); } } } @@ -125,7 +125,7 @@ private static void WriteDispatcher(IntPtr key, IntPtr packet, uint error) if (null != stateObj) { - stateObj.WriteAsyncCallback(IntPtr.Zero, packet, error); + stateObj.WriteAsyncCallback(IntPtr.Zero, PacketHandle.FromNativePointer(packet), error); } } } @@ -206,9 +206,9 @@ internal uint Status } } - internal sealed class SNIPacket : SafeHandle + internal sealed class SNIPacketHandle : SafeHandle { - internal SNIPacket(SafeHandle sniHandle) : base(IntPtr.Zero, true) + internal SNIPacketHandle(SafeHandle sniHandle) : base(IntPtr.Zero, true) { SNINativeMethodWrapper.SNIPacketAllocate(sniHandle, SNINativeMethodWrapper.IOType.WRITE, ref base.handle); if (IntPtr.Zero == base.handle) @@ -241,17 +241,17 @@ override protected bool ReleaseHandle() internal sealed class WritePacketCache : IDisposable { private bool _disposed; - private Stack _packets; + private Stack _packets; public WritePacketCache() { _disposed = false; - _packets = new Stack(); + _packets = new Stack(); } - public SNIPacket Take(SNIHandle sniHandle) + public SNIPacketHandle Take(SNIHandle sniHandle) { - SNIPacket packet; + SNIPacketHandle packet; if (_packets.Count > 0) { // Success - reset the packet @@ -261,12 +261,12 @@ public SNIPacket Take(SNIHandle sniHandle) else { // Failed to take a packet - create a new one - packet = new SNIPacket(sniHandle); + packet = new SNIPacketHandle(sniHandle); } return packet; } - public void Add(SNIPacket packet) + public void Add(SNIPacketHandle packet) { if (!_disposed) { @@ -296,4 +296,4 @@ public void Dispose() } } } -} \ No newline at end of file +} diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs index dcc7eae1744e..aa5aa847a895 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs @@ -20,6 +20,102 @@ sealed internal class LastIOTimer internal long _value; } + internal readonly ref struct PacketHandle + { + public const int NativePointerType = 1; + public const int NativePacketType = 2; + public const int ManagedPacketType = 3; + +#if FEATURE_INTEROPSNI + public readonly IntPtr NativePointer; + public readonly SNIPacketHandle NativePacket; +#endif + public readonly SNI.SNIPacket ManagedPacket; + public readonly int Type; + + private PacketHandle(IntPtr nativePointer, +#if FEATURE_INTEROPSNI + SNIPacketHandle +#else + int +#endif + + nativePacket, SNI.SNIPacket managedPacket, int type) + { + Type = type; + ManagedPacket = managedPacket; +#if FEATURE_INTEROPSNI + NativePointer = nativePointer; + NativePacket = nativePacket; +#endif + } + + public static PacketHandle FromManagedPacket(SNI.SNIPacket managedPacket) + { + return new PacketHandle(default, default, managedPacket, ManagedPacketType); + } + +#if FEATURE_INTEROPSNI + public static PacketHandle FromNativePointer(IntPtr nativePointer) + { + return new PacketHandle(nativePointer,default,default, NativePointerType); + } + + public static PacketHandle FromNativePacket(SNIPacketHandle nativePacket) + { + return new PacketHandle(default, nativePacket, default, NativePacketType); + } +#endif + + + } + + internal readonly ref struct SessionHandle + { + public const int NativeHandleType = 1; + public const int ManagedHandleType = 2; + + public readonly SNI.SNIHandle ManagedHandle; +#if FEATURE_INTEROPSNI + public readonly SNIHandle NativeHandle; +#endif + public readonly int Type; + + public SessionHandle(SNI.SNIHandle managedHandle, +#if FEATURE_INTEROPSNI + SNIHandle +#else + int +#endif + nativeHandle, + int type + ) + { + Type = type; + ManagedHandle = managedHandle; +#if FEATURE_INTEROPSNI + NativeHandle = nativeHandle; +#endif + } + + public bool IsNull => +#if FEATURE_INTEROPSNI + (Type == NativeHandleType) ? NativeHandle == null : +#endif + ManagedHandle == null; + + public static SessionHandle FromManagedSession(SNI.SNIHandle managedSessionHandle) + { + return new SessionHandle(managedSessionHandle, default, ManagedHandleType); + } +#if FEATURE_INTEROPSNI + public static SessionHandle FromNativeHandle(SNIHandle nativeSessionHandle) + { + return new SessionHandle(default, nativeSessionHandle, NativeHandleType); + } +#endif + } + internal abstract class TdsParserStateObject { private const int AttentionTimeoutSeconds = 5; @@ -392,7 +488,7 @@ internal abstract uint Status get; } - internal abstract object SessionHandle + internal abstract SessionHandle SessionHandle { get; } @@ -761,27 +857,27 @@ private void ResetCancelAndProcessAttention() internal abstract void DisposePacketCache(); - internal abstract bool IsPacketEmpty(object readPacket); + internal abstract bool IsPacketEmpty(PacketHandle readPacket); - internal abstract object ReadSyncOverAsync(int timeoutRemaining, out uint error); + internal abstract PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint error); - internal abstract object ReadAsync(out uint error, ref object handle); + internal abstract PacketHandle ReadAsync(SessionHandle handle, out uint error); internal abstract uint CheckConnection(); internal abstract uint SetConnectionBufferSize(ref uint unsignedPacketSize); - internal abstract void ReleasePacket(object syncReadPacket); + internal abstract void ReleasePacket(PacketHandle syncReadPacket); - protected abstract uint SNIPacketGetData(object packet, byte[] _inBuff, ref uint dataSize); + protected abstract uint SNIPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize); - internal abstract object GetResetWritePacket(); + internal abstract PacketHandle GetResetWritePacket(); internal abstract void ClearAllWritePackets(); - internal abstract object AddPacketToPendingList(object packet); + internal abstract PacketHandle AddPacketToPendingList(PacketHandle packet); - protected abstract void RemovePacketFromPendingList(object pointer); + protected abstract void RemovePacketFromPendingList(PacketHandle pointer); internal abstract uint GenerateSspiClientContext(byte[] receivedBuff, uint receivedLength, ref byte[] sendBuff, ref uint sendLength, byte[] _sniSpnBuffer); @@ -855,7 +951,7 @@ internal int DecrementPendingCallbacks(bool 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 == null) || (0 <= remaining && remaining < 3), string.Format("_pendingCallbacks values is invalid after decrementing: {0}", remaining)); + Debug.Assert((remaining == -1 && SessionHandle.IsNull) || (0 <= remaining && remaining < 3), string.Format("_pendingCallbacks values is invalid after decrementing: {0}", remaining)); return remaining; } @@ -2069,7 +2165,7 @@ internal void ReadSniSyncOverAsync() throw ADP.ClosedConnectionError(); } - object readPacket = null; + PacketHandle readPacket = default; uint error; @@ -2291,7 +2387,7 @@ internal void ReadSni(TaskCompletionSource completion) #endif - object readPacket = null; + PacketHandle readPacket = default; uint error = 0; @@ -2317,16 +2413,14 @@ internal void ReadSni(TaskCompletionSource completion) ChangeNetworkPacketTimeout(msecsRemaining, Timeout.Infinite); } - object handle = null; - Interlocked.Increment(ref _readingCount); - handle = SessionHandle; - if (handle != null) + SessionHandle handle = SessionHandle; + if (!handle.IsNull) { IncrementPendingCallbacks(); - readPacket = ReadAsync(out error, ref handle); + readPacket = ReadAsync(handle, out error); if (!(TdsEnums.SNI_SUCCESS == error || TdsEnums.SNI_SUCCESS_IO_PENDING == error)) { @@ -2335,8 +2429,8 @@ internal void ReadSni(TaskCompletionSource completion) } Interlocked.Decrement(ref _readingCount); - - if (handle == null) + + if (handle.IsNull) { throw ADP.ClosedConnectionError(); } @@ -2419,7 +2513,7 @@ internal bool IsConnectionAlive(bool throwOnException) { uint error; - object readPacket = EmptyReadPacket; + PacketHandle readPacket = EmptyReadPacket; try { @@ -2512,7 +2606,7 @@ private void ReadSniError(TdsParserStateObject stateObj, uint error) { stateObj.SendAttention(mustTakeWriteLock: true); - object syncReadPacket = null; + PacketHandle syncReadPacket = default; bool shouldDecrement = false; try @@ -2584,7 +2678,7 @@ private void ReadSniError(TdsParserStateObject stateObj, uint error) AssertValidState(); } - public void ProcessSniPacket(object packet, uint error) + public void ProcessSniPacket(PacketHandle packet, uint error) { if (error != 0) { @@ -2683,13 +2777,12 @@ private void SetBufferSecureStrings() } } - public void ReadAsyncCallback(T packet, uint error) + public void ReadAsyncCallback(PacketHandle packet, uint error) { ReadAsyncCallback(IntPtr.Zero, packet, error); } - - public void ReadAsyncCallback(IntPtr key, T packet, uint error) + public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) { // Key never used. // Note - it's possible that when native calls managed that an asynchronous exception @@ -2769,7 +2862,7 @@ public void ReadAsyncCallback(IntPtr key, T packet, uint error) } } - protected abstract bool CheckPacket(object packet, TaskCompletionSource source); + protected abstract bool CheckPacket(PacketHandle packet, TaskCompletionSource source); private void ReadAsyncCallbackCaptureException(TaskCompletionSource source) { @@ -2815,12 +2908,12 @@ private void ReadAsyncCallbackCaptureException(TaskCompletionSource sour #pragma warning disable 0420 // a reference to a volatile field will not be treated as volatile - public void WriteAsyncCallback(T packet, uint sniError) + public void WriteAsyncCallback(PacketHandle packet, uint sniError) { WriteAsyncCallback(IntPtr.Zero, packet, sniError); } - public void WriteAsyncCallback(IntPtr key, T packet, uint sniError) + public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError) { // Key never used. RemovePacketFromPendingList(packet); try @@ -3189,7 +3282,7 @@ private void CancelWritePacket() #pragma warning disable 0420 // a reference to a volatile field will not be treated as volatile - private Task SNIWritePacket(object packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock) + private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock) { // Check for a stored exception var delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); @@ -3201,7 +3294,7 @@ private Task SNIWritePacket(object packet, out uint sniError, bool canAccumulate Task task = null; _writeCompletionSource = null; - object packetPointer = EmptyReadPacket; + PacketHandle packetPointer = EmptyReadPacket; bool sync = !_parser._asyncWrite; if (sync && _asyncWriteCount > 0) @@ -3322,8 +3415,9 @@ private Task SNIWritePacket(object packet, out uint sniError, bool canAccumulate return task; } - internal abstract bool IsValidPacket(object packetPointer); - internal abstract uint WritePacket(object packet, bool sync); + internal abstract bool IsValidPacket(PacketHandle packetPointer); + + internal abstract uint WritePacket(PacketHandle packet, bool sync); #pragma warning restore 0420 @@ -3340,7 +3434,7 @@ internal void SendAttention(bool mustTakeWriteLock = false) return; } - object attnPacket = CreateAndSetAttentionPacket(); + PacketHandle attnPacket = CreateAndSetAttentionPacket(); try { @@ -3398,14 +3492,14 @@ internal void SendAttention(bool mustTakeWriteLock = false) } } - internal abstract object CreateAndSetAttentionPacket(); + internal abstract PacketHandle CreateAndSetAttentionPacket(); - internal abstract void SetPacketData(object packet, byte[] buffer, int bytesUsed); + internal abstract void SetPacketData(PacketHandle packet, byte[] buffer, int bytesUsed); private Task WriteSni(bool canAccumulate) { // Prepare packet, and write to packet. - object packet = GetResetWritePacket(); + PacketHandle packet = GetResetWritePacket(); SetBufferSecureStrings(); SetPacketData(packet, _outBuff, _outBytesUsed); @@ -3617,7 +3711,7 @@ internal int WarningCount } } - protected abstract object EmptyReadPacket { get; } + protected abstract PacketHandle EmptyReadPacket { get; } /// /// Gets the full list of errors and warnings (including the pre-attention ones), then wipes all error and warning lists diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectFactory.Windows.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectFactory.Windows.cs index 96832fb8b46f..9d767d6d7f8f 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectFactory.Windows.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectFactory.Windows.cs @@ -17,9 +17,15 @@ internal sealed class TdsParserStateObjectFactory // Temporary disabling App Context switching for managed SNI. // If the appcontext switch is set then Use Managed SNI based on the value. Otherwise Managed SNI should always be used. //private static bool shouldUseLegacyNetorking; + private static Lazy shouldUseLegacyNetorking = new Lazy(() => bool.TrueString.Equals(Environment.GetEnvironmentVariable("System.Data.SqlClient.UseLegacyNetworkingOnWindows"), StringComparison.InvariantCultureIgnoreCase)); //public static bool UseManagedSNI { get; } = AppContext.TryGetSwitch(UseLegacyNetworkingOnWindows, out shouldUseLegacyNetorking) ? !shouldUseLegacyNetorking : true; + +#if DEBUG + public static bool UseManagedSNI => shouldUseLegacyNetorking.Value; +#else public static bool UseManagedSNI { get; } = false; +#endif public EncryptionOptions EncryptionOptions { diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs index 103e6fb163c5..e70613dbb5b4 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs @@ -31,14 +31,14 @@ internal TdsParserStateObjectManaged(TdsParser parser, TdsParserStateObject phys internal override uint Status => _sessionHandle != null ? _sessionHandle.Status : TdsEnums.SNI_UNINITIALIZED; - internal override object SessionHandle => _sessionHandle; + internal override SessionHandle SessionHandle => SessionHandle.FromManagedSession(_sessionHandle); - protected override object EmptyReadPacket => null; + protected override PacketHandle EmptyReadPacket => default; - protected override bool CheckPacket(object packet, TaskCompletionSource source) + protected override bool CheckPacket(PacketHandle packet, TaskCompletionSource source) { - SNIPacket p = packet as SNIPacket; - return p.IsInvalid || (!p.IsInvalid && source != null); + SNIPacket p = packet.ManagedPacket; + return p.IsInvalid || source != null; } protected override void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async) @@ -54,7 +54,7 @@ internal SNIMarsHandle CreateMarsSession(object callbackObject, bool async) return _marsConnection.CreateMarsSession(callbackObject, async); } - protected override uint SNIPacketGetData(object packet, byte[] _inBuff, ref uint dataSize) => SNIProxy.Singleton.PacketGetData(packet as SNIPacket, _inBuff, ref dataSize); + protected override uint SNIPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize) => SNIProxy.Singleton.PacketGetData(packet.ManagedPacket, _inBuff, ref dataSize); internal override void CreatePhysicalSNIHandle(string serverName, bool ignoreSniOpenTimeout, long timerExpire, out byte[] instanceName, ref byte[] spnBuffer, bool flushCache, bool async, bool parallel, bool isIntegratedSecurity) { @@ -72,11 +72,11 @@ internal override void CreatePhysicalSNIHandle(string serverName, bool ignoreSni } } - internal void ReadAsyncCallback(SNIPacket packet, uint error) => ReadAsyncCallback(IntPtr.Zero, packet, error); + internal void ReadAsyncCallback(SNIPacket packet, uint error) => ReadAsyncCallback(IntPtr.Zero, PacketHandle.FromManagedPacket(packet), error); - internal void WriteAsyncCallback(SNIPacket packet, uint sniError) => WriteAsyncCallback(IntPtr.Zero, packet, sniError); + internal void WriteAsyncCallback(SNIPacket packet, uint sniError) => WriteAsyncCallback(IntPtr.Zero, PacketHandle.FromManagedPacket(packet), sniError); - protected override void RemovePacketFromPendingList(object packet) + protected override void RemovePacketFromPendingList(PacketHandle packet) { // No-Op } @@ -125,7 +125,7 @@ protected override void FreeGcHandle(int remaining, bool release) internal override bool IsFailedHandle() => _sessionHandle.Status != TdsEnums.SNI_SUCCESS; - internal override object ReadSyncOverAsync(int timeoutRemaining, out uint error) + internal override PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint error) { SNIHandle handle = Handle; if (handle == null) @@ -134,17 +134,17 @@ internal override object ReadSyncOverAsync(int timeoutRemaining, out uint error) } SNIPacket packet = null; error = SNIProxy.Singleton.ReadSyncOverAsync(handle, out packet, timeoutRemaining); - return packet; + return PacketHandle.FromManagedPacket(packet); } - internal override bool IsPacketEmpty(object packet) + internal override bool IsPacketEmpty(PacketHandle packet) { - return packet == null; + return packet.ManagedPacket == null; } - internal override void ReleasePacket(object syncReadPacket) + internal override void ReleasePacket(PacketHandle syncReadPacket) { - ((SNIPacket)syncReadPacket).Dispose(); + syncReadPacket.ManagedPacket?.Dispose(); } internal override uint CheckConnection() @@ -153,38 +153,38 @@ internal override uint CheckConnection() return handle == null ? TdsEnums.SNI_SUCCESS : SNIProxy.Singleton.CheckConnection(handle); } - internal override object ReadAsync(out uint error, ref object handle) + internal override PacketHandle ReadAsync(SessionHandle handle, out uint error) { SNIPacket packet; - error = SNIProxy.Singleton.ReadAsync((SNIHandle)handle, out packet); - return packet; + error = SNIProxy.Singleton.ReadAsync(handle.ManagedHandle, out packet); + return PacketHandle.FromManagedPacket(packet); } - internal override object CreateAndSetAttentionPacket() + internal override PacketHandle CreateAndSetAttentionPacket() { if (_sniAsyncAttnPacket == null) { SNIPacket attnPacket = new SNIPacket(); - SetPacketData(attnPacket, SQL.AttentionHeader, TdsEnums.HEADER_LEN); + SetPacketData(PacketHandle.FromManagedPacket(attnPacket), SQL.AttentionHeader, TdsEnums.HEADER_LEN); _sniAsyncAttnPacket = attnPacket; } - return _sniAsyncAttnPacket; + return PacketHandle.FromManagedPacket(_sniAsyncAttnPacket); } - internal override uint WritePacket(object packet, bool sync) + internal override uint WritePacket(PacketHandle packet, bool sync) { - return SNIProxy.Singleton.WritePacket((SNIHandle)Handle, (SNIPacket)packet, sync); + return SNIProxy.Singleton.WritePacket(Handle, packet.ManagedPacket, sync); } - internal override object AddPacketToPendingList(object packet) + internal override PacketHandle AddPacketToPendingList(PacketHandle packet) { // No-Op return packet; } - internal override bool IsValidPacket(object packetPointer) => (SNIPacket)packetPointer != null && !((SNIPacket)packetPointer).IsInvalid; + internal override bool IsValidPacket(PacketHandle packet) => packet.ManagedPacket?.IsInvalid ?? false; - internal override object GetResetWritePacket() + internal override PacketHandle GetResetWritePacket() { if (_sniPacket != null) { @@ -197,7 +197,7 @@ internal override object GetResetWritePacket() _sniPacket = _writePacketCache.Take(Handle); } } - return _sniPacket; + return PacketHandle.FromManagedPacket(_sniPacket); } internal override void ClearAllWritePackets() @@ -214,8 +214,8 @@ internal override void ClearAllWritePackets() } } - internal override void SetPacketData(object packet, byte[] buffer, int bytesUsed) => SNIProxy.Singleton.PacketSetData((SNIPacket)packet, buffer, bytesUsed); - + internal override void SetPacketData(PacketHandle packet, byte[] buffer, int bytesUsed) => SNIProxy.Singleton.PacketSetData(packet.ManagedPacket, buffer, bytesUsed); + internal override uint SniGetConnectionId(ref Guid clientConnectionId) => SNIProxy.Singleton.GetConnectionId(Handle, ref clientConnectionId); internal override uint DisabeSsl() => SNIProxy.Singleton.DisableSsl(Handle); diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs index c1fa34bd9c70..27922a2c26ba 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs @@ -14,15 +14,15 @@ internal class TdsParserStateObjectNative : TdsParserStateObject { private SNIHandle _sessionHandle = null; // the SNI handle we're to work on - private SNIPacket _sniPacket = null; // Will have to re-vamp this for MARS - internal SNIPacket _sniAsyncAttnPacket = null; // Packet to use to send Attn + private SNIPacketHandle _sniPacket = null; // Will have to re-vamp this for MARS + internal SNIPacketHandle _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 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) @@ -33,9 +33,9 @@ internal TdsParserStateObjectNative(TdsParser parser, TdsParserStateObject physi internal override uint Status => _sessionHandle != null ? _sessionHandle.Status : TdsEnums.SNI_UNINITIALIZED; - internal override object SessionHandle => _sessionHandle; + internal override SessionHandle SessionHandle => SessionHandle.FromNativeHandle(_sessionHandle); - protected override object EmptyReadPacket => IntPtr.Zero; + protected override PacketHandle EmptyReadPacket => default; protected override void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async) { @@ -97,11 +97,16 @@ internal override void CreatePhysicalSNIHandle(string serverName, bool ignoreSni _sessionHandle = new SNIHandle(myInfo, serverName, spnBuffer, ignoreSniOpenTimeout, checked((int)timeout), out instanceName, flushCache, !async, fParallel); } - protected override uint SNIPacketGetData(object packet, byte[] _inBuff, ref uint dataSize) => SNINativeMethodWrapper.SNIPacketGetData((IntPtr)packet, _inBuff, ref dataSize); + protected override uint SNIPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize) + { + Debug.Assert(packet.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + return SNINativeMethodWrapper.SNIPacketGetData(packet.NativePointer, _inBuff, ref dataSize); + } - protected override bool CheckPacket(object packet, TaskCompletionSource source) + protected override bool CheckPacket(PacketHandle packet, TaskCompletionSource source) { - IntPtr ptr = (IntPtr)(object)packet; + 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; } @@ -109,11 +114,12 @@ protected override bool CheckPacket(object packet, TaskCompletionSource public void WriteAsyncCallback(IntPtr key, IntPtr packet, uint sniError) => WriteAsyncCallback(key, packet, sniError); - protected override void RemovePacketFromPendingList(object ptr) + protected override void RemovePacketFromPendingList(PacketHandle ptr) { - IntPtr pointer = (IntPtr)ptr; + Debug.Assert(ptr.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + IntPtr pointer = ptr.NativePointer; - SNIPacket recoveredPacket; + SNIPacketHandle recoveredPacket; lock (_writePacketLockObject) { @@ -169,7 +175,7 @@ protected override void FreeGcHandle(int remaining, bool release) internal override bool IsFailedHandle() => _sessionHandle.Status != TdsEnums.SNI_SUCCESS; - internal override object ReadSyncOverAsync(int timeoutRemaining, out uint error) + internal override PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint error) { SNIHandle handle = Handle; if (handle == null) @@ -178,12 +184,20 @@ internal override object ReadSyncOverAsync(int timeoutRemaining, out uint error) } IntPtr readPacketPtr = IntPtr.Zero; error = SNINativeMethodWrapper.SNIReadSyncOverAsync(handle, ref readPacketPtr, GetTimeoutRemaining()); - return readPacketPtr; + return PacketHandle.FromNativePointer(readPacketPtr); } - internal override bool IsPacketEmpty(object readPacket) => IntPtr.Zero == (IntPtr)readPacket; + 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 void ReleasePacket(object syncReadPacket) => SNINativeMethodWrapper.SNIPacketRelease((IntPtr)syncReadPacket); + internal override void ReleasePacket(PacketHandle syncReadPacket) + { + Debug.Assert(syncReadPacket.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + SNINativeMethodWrapper.SNIPacketRelease(syncReadPacket.NativePointer); + } internal override uint CheckConnection() { @@ -191,27 +205,33 @@ internal override uint CheckConnection() return handle == null ? TdsEnums.SNI_SUCCESS : SNINativeMethodWrapper.SNICheckConnection(handle); } - internal override object ReadAsync(out uint error, ref object handle) + internal override PacketHandle ReadAsync(SessionHandle handle, out uint error) { + Debug.Assert(handle.Type == SessionHandle.NativeHandleType, "unexpected handle type when requiring NativePointer"); IntPtr readPacketPtr = IntPtr.Zero; - error = SNINativeMethodWrapper.SNIReadAsync((SNIHandle)handle, ref readPacketPtr); - return readPacketPtr; + error = SNINativeMethodWrapper.SNIReadAsync(handle.NativeHandle, ref readPacketPtr); + return PacketHandle.FromNativePointer(readPacketPtr); } - internal override object CreateAndSetAttentionPacket() + internal override PacketHandle CreateAndSetAttentionPacket() { SNIHandle handle = Handle; - SNIPacket attnPacket = new SNIPacket(handle); + SNIPacketHandle attnPacket = new SNIPacketHandle(handle); _sniAsyncAttnPacket = attnPacket; - SetPacketData(attnPacket, SQL.AttentionHeader, TdsEnums.HEADER_LEN); - return attnPacket; + SetPacketData(PacketHandle.FromNativePacket(attnPacket), SQL.AttentionHeader, TdsEnums.HEADER_LEN); + return PacketHandle.FromNativePacket(attnPacket); } - internal override uint WritePacket(object packet, bool sync) => SNINativeMethodWrapper.SNIWritePacket(Handle, (SNIPacket)packet, sync); + internal override uint WritePacket(PacketHandle packet, bool sync) + { + Debug.Assert(packet.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePacket"); + return SNINativeMethodWrapper.SNIWritePacket(Handle, packet.NativePacket, sync); + } - internal override object AddPacketToPendingList(object packetToAdd) + internal override PacketHandle AddPacketToPendingList(PacketHandle packetToAdd) { - SNIPacket packet = (SNIPacket)packetToAdd; + Debug.Assert(packetToAdd.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePacket"); + SNIPacketHandle 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(); @@ -221,12 +241,16 @@ internal override object AddPacketToPendingList(object packetToAdd) _pendingWritePackets.Add(pointer, packet); } - return pointer; + return PacketHandle.FromNativePointer(pointer); } - internal override bool IsValidPacket(object packetPointer) => (IntPtr)packetPointer != IntPtr.Zero; + internal override bool IsValidPacket(PacketHandle packetPointer) + { + Debug.Assert(packetPointer.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + return packetPointer.NativePointer != IntPtr.Zero; + } - internal override object GetResetWritePacket() + internal override PacketHandle GetResetWritePacket() { if (_sniPacket != null) { @@ -239,7 +263,7 @@ internal override object GetResetWritePacket() _sniPacket = _writePacketCache.Take(Handle); } } - return _sniPacket; + return PacketHandle.FromNativePacket(_sniPacket); } internal override void ClearAllWritePackets() @@ -256,8 +280,11 @@ internal override void ClearAllWritePackets() } } - internal override void SetPacketData(object packet, byte[] buffer, int bytesUsed) - => SNINativeMethodWrapper.SNIPacketSetData((SNIPacket)packet, buffer, bytesUsed); + internal override void SetPacketData(PacketHandle packet, byte[] buffer, int bytesUsed) + { + Debug.Assert(packet.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePacket"); + SNINativeMethodWrapper.SNIPacketSetData(packet.NativePacket, buffer, bytesUsed); + } internal override uint SniGetConnectionId(ref Guid clientConnectionId) => SNINativeMethodWrapper.SniGetConnectionId(Handle, ref clientConnectionId); @@ -296,17 +323,17 @@ internal override void DisposePacketCache() internal sealed class WritePacketCache : IDisposable { private bool _disposed; - private Stack _packets; + private Stack _packets; public WritePacketCache() { _disposed = false; - _packets = new Stack(); + _packets = new Stack(); } - public SNIPacket Take(SNIHandle sniHandle) + public SNIPacketHandle Take(SNIHandle sniHandle) { - SNIPacket packet; + SNIPacketHandle packet; if (_packets.Count > 0) { // Success - reset the packet @@ -316,12 +343,12 @@ public SNIPacket Take(SNIHandle sniHandle) else { // Failed to take a packet - create a new one - packet = new SNIPacket(sniHandle); + packet = new SNIPacketHandle(sniHandle); } return packet; } - public void Add(SNIPacket packet) + public void Add(SNIPacketHandle packet) { if (!_disposed) { From 1a8f4476306b3df335cb4c0602eb3c58b1b4f3d9 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Wed, 12 Dec 2018 21:54:51 +0000 Subject: [PATCH 02/10] add project define for FEATURE_INTEROPSNI on windows non uap builds --- src/System.Data.SqlClient/src/System.Data.SqlClient.csproj | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/System.Data.SqlClient/src/System.Data.SqlClient.csproj b/src/System.Data.SqlClient/src/System.Data.SqlClient.csproj index d8b68555bd4a..2b45412b8978 100644 --- a/src/System.Data.SqlClient/src/System.Data.SqlClient.csproj +++ b/src/System.Data.SqlClient/src/System.Data.SqlClient.csproj @@ -1,4 +1,4 @@ - + {D4550556-4745-457F-BA8F-3EBF3836D6B4} System.Data.SqlClient @@ -10,6 +10,7 @@ 4.1.0.0 $(DefineConstants);netcoreapp $(DefineConstants);FEATURE_TCPKEEPALIVE + $(DefineConstants);FEATURE_INTEROPSNI $(NoWarn);CS1572;CS1574;CS1587;CS1589;CS1591;CS1723;CS1734 net461-Windows_NT-Debug;net461-Windows_NT-Release;netcoreapp-Debug;netcoreapp-Release;netcoreapp-Unix-Debug;netcoreapp-Unix-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;netcoreapp2.1-Debug;netcoreapp2.1-Release;netcoreapp2.1-Unix-Debug;netcoreapp2.1-Unix-Release;netcoreapp2.1-Windows_NT-Debug;netcoreapp2.1-Windows_NT-Release;netfx-Windows_NT-Debug;netfx-Windows_NT-Release;netstandard-Debug;netstandard-Release;netstandard-Unix-Debug;netstandard-Unix-Release;netstandard-Windows_NT-Debug;netstandard-Windows_NT-Release;netstandard1.2-Debug;netstandard1.2-Release;netstandard1.3-Debug;netstandard1.3-Release;uap-Windows_NT-Debug;uap-Windows_NT-Release;uap10.0.16299-Windows_NT-Debug;uap10.0.16299-Windows_NT-Release From 7ac55538909a33f64a27e12afede15fe7071ad1c Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Wed, 12 Dec 2018 23:00:44 +0000 Subject: [PATCH 03/10] update interop to use SniPacketHandle type name --- .../src/Interop/SNINativeMethodWrapper.Windows.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/System.Data.SqlClient/src/Interop/SNINativeMethodWrapper.Windows.cs b/src/System.Data.SqlClient/src/Interop/SNINativeMethodWrapper.Windows.cs index 1902b7045a8e..e6e43a382ceb 100644 --- a/src/System.Data.SqlClient/src/Interop/SNINativeMethodWrapper.Windows.cs +++ b/src/System.Data.SqlClient/src/Interop/SNINativeMethodWrapper.Windows.cs @@ -197,7 +197,7 @@ internal struct SNI_Error internal static extern void SNIPacketRelease(IntPtr pPacket); [DllImport(SNI, CallingConvention = CallingConvention.Cdecl, EntryPoint = "SNIPacketResetWrapper")] - internal static extern void SNIPacketReset([In] SNIHandle pConn, IOType IOType, SNIPacket pPacket, ConsumerNumber ConsNum); + internal static extern void SNIPacketReset([In] SNIHandle pConn, IOType IOType, SNIPacketHandle pPacket, ConsumerNumber ConsNum); [DllImport(SNI, CallingConvention = CallingConvention.Cdecl)] internal static extern uint SNIQueryInfo(QTypes QType, ref uint pbQInfo); @@ -256,7 +256,7 @@ private static extern uint SNIOpenWrapper( private static extern uint SNIPacketGetDataWrapper([In] IntPtr packet, [In, Out] byte[] readBuffer, uint readBufferLength, out uint dataSize); [DllImport(SNI, CallingConvention = CallingConvention.Cdecl)] - private static extern unsafe void SNIPacketSetData(SNIPacket pPacket, [In] byte* pbBuf, uint cbBuf); + private static extern unsafe void SNIPacketSetData(SNIPacketHandle pPacket, [In] byte* pbBuf, uint cbBuf); [DllImport(SNI, CallingConvention = CallingConvention.Cdecl)] private static extern unsafe uint SNISecGenClientContextWrapper( @@ -272,10 +272,10 @@ private static extern unsafe uint SNISecGenClientContextWrapper( [MarshalAsAttribute(UnmanagedType.LPWStr)] string pwszPassword); [DllImport(SNI, CallingConvention = CallingConvention.Cdecl)] - private static extern uint SNIWriteAsyncWrapper(SNIHandle pConn, [In] SNIPacket pPacket); + private static extern uint SNIWriteAsyncWrapper(SNIHandle pConn, [In] SNIPacketHandle pPacket); [DllImport(SNI, CallingConvention = CallingConvention.Cdecl)] - private static extern uint SNIWriteSyncOverAsync(SNIHandle pConn, [In] SNIPacket pPacket); + private static extern uint SNIWriteSyncOverAsync(SNIHandle pConn, [In] SNIPacketHandle pPacket); #endregion internal static uint SniGetConnectionId(SNIHandle pConn, ref Guid connId) @@ -347,7 +347,7 @@ internal static unsafe uint SNIPacketGetData(IntPtr packet, byte[] readBuffer, r return SNIPacketGetDataWrapper(packet, readBuffer, (uint)readBuffer.Length, out dataSize); } - internal static unsafe void SNIPacketSetData(SNIPacket packet, byte[] data, int length) + internal static unsafe void SNIPacketSetData(SNIPacketHandle packet, byte[] data, int length) { fixed (byte* pin_data = &data[0]) { @@ -374,7 +374,7 @@ internal static unsafe uint SNISecGenClientContext(SNIHandle pConnectionObject, } } - internal static uint SNIWritePacket(SNIHandle pConn, SNIPacket packet, bool sync) + internal static uint SNIWritePacket(SNIHandle pConn, SNIPacketHandle packet, bool sync) { if (sync) { From 1d6235dc34f6a586834ef88538092f022d5d189a Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Fri, 14 Dec 2018 01:19:17 +0000 Subject: [PATCH 04/10] rename SNIPacketHandle to SNIPacket --- .../Interop/SNINativeMethodWrapper.Windows.cs | 12 +++++----- .../Data/SqlClient/TdsParserSafeHandles.cs | 16 ++++++------- .../Data/SqlClient/TdsParserStateObject.cs | 6 ++--- .../SqlClient/TdsParserStateObjectNative.cs | 24 +++++++++---------- 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/System.Data.SqlClient/src/Interop/SNINativeMethodWrapper.Windows.cs b/src/System.Data.SqlClient/src/Interop/SNINativeMethodWrapper.Windows.cs index e6e43a382ceb..1902b7045a8e 100644 --- a/src/System.Data.SqlClient/src/Interop/SNINativeMethodWrapper.Windows.cs +++ b/src/System.Data.SqlClient/src/Interop/SNINativeMethodWrapper.Windows.cs @@ -197,7 +197,7 @@ internal struct SNI_Error internal static extern void SNIPacketRelease(IntPtr pPacket); [DllImport(SNI, CallingConvention = CallingConvention.Cdecl, EntryPoint = "SNIPacketResetWrapper")] - internal static extern void SNIPacketReset([In] SNIHandle pConn, IOType IOType, SNIPacketHandle pPacket, ConsumerNumber ConsNum); + internal static extern void SNIPacketReset([In] SNIHandle pConn, IOType IOType, SNIPacket pPacket, ConsumerNumber ConsNum); [DllImport(SNI, CallingConvention = CallingConvention.Cdecl)] internal static extern uint SNIQueryInfo(QTypes QType, ref uint pbQInfo); @@ -256,7 +256,7 @@ private static extern uint SNIOpenWrapper( private static extern uint SNIPacketGetDataWrapper([In] IntPtr packet, [In, Out] byte[] readBuffer, uint readBufferLength, out uint dataSize); [DllImport(SNI, CallingConvention = CallingConvention.Cdecl)] - private static extern unsafe void SNIPacketSetData(SNIPacketHandle pPacket, [In] byte* pbBuf, uint cbBuf); + private static extern unsafe void SNIPacketSetData(SNIPacket pPacket, [In] byte* pbBuf, uint cbBuf); [DllImport(SNI, CallingConvention = CallingConvention.Cdecl)] private static extern unsafe uint SNISecGenClientContextWrapper( @@ -272,10 +272,10 @@ private static extern unsafe uint SNISecGenClientContextWrapper( [MarshalAsAttribute(UnmanagedType.LPWStr)] string pwszPassword); [DllImport(SNI, CallingConvention = CallingConvention.Cdecl)] - private static extern uint SNIWriteAsyncWrapper(SNIHandle pConn, [In] SNIPacketHandle pPacket); + private static extern uint SNIWriteAsyncWrapper(SNIHandle pConn, [In] SNIPacket pPacket); [DllImport(SNI, CallingConvention = CallingConvention.Cdecl)] - private static extern uint SNIWriteSyncOverAsync(SNIHandle pConn, [In] SNIPacketHandle pPacket); + private static extern uint SNIWriteSyncOverAsync(SNIHandle pConn, [In] SNIPacket pPacket); #endregion internal static uint SniGetConnectionId(SNIHandle pConn, ref Guid connId) @@ -347,7 +347,7 @@ internal static unsafe uint SNIPacketGetData(IntPtr packet, byte[] readBuffer, r return SNIPacketGetDataWrapper(packet, readBuffer, (uint)readBuffer.Length, out dataSize); } - internal static unsafe void SNIPacketSetData(SNIPacketHandle packet, byte[] data, int length) + internal static unsafe void SNIPacketSetData(SNIPacket packet, byte[] data, int length) { fixed (byte* pin_data = &data[0]) { @@ -374,7 +374,7 @@ internal static unsafe uint SNISecGenClientContext(SNIHandle pConnectionObject, } } - internal static uint SNIWritePacket(SNIHandle pConn, SNIPacketHandle packet, bool sync) + internal static uint SNIWritePacket(SNIHandle pConn, SNIPacket packet, bool sync) { if (sync) { diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserSafeHandles.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserSafeHandles.cs index 4360ab64d5ab..7dce0de70ae5 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserSafeHandles.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserSafeHandles.cs @@ -206,9 +206,9 @@ internal uint Status } } - internal sealed class SNIPacketHandle : SafeHandle + internal sealed class SNIPacket : SafeHandle { - internal SNIPacketHandle(SafeHandle sniHandle) : base(IntPtr.Zero, true) + internal SNIPacket(SafeHandle sniHandle) : base(IntPtr.Zero, true) { SNINativeMethodWrapper.SNIPacketAllocate(sniHandle, SNINativeMethodWrapper.IOType.WRITE, ref base.handle); if (IntPtr.Zero == base.handle) @@ -241,17 +241,17 @@ override protected bool ReleaseHandle() internal sealed class WritePacketCache : IDisposable { private bool _disposed; - private Stack _packets; + private Stack _packets; public WritePacketCache() { _disposed = false; - _packets = new Stack(); + _packets = new Stack(); } - public SNIPacketHandle Take(SNIHandle sniHandle) + public SNIPacket Take(SNIHandle sniHandle) { - SNIPacketHandle packet; + SNIPacket packet; if (_packets.Count > 0) { // Success - reset the packet @@ -261,12 +261,12 @@ public SNIPacketHandle Take(SNIHandle sniHandle) else { // Failed to take a packet - create a new one - packet = new SNIPacketHandle(sniHandle); + packet = new SNIPacket(sniHandle); } return packet; } - public void Add(SNIPacketHandle packet) + public void Add(SNIPacket packet) { if (!_disposed) { diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs index aa5aa847a895..367f0f9ada4b 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs @@ -28,14 +28,14 @@ internal readonly ref struct PacketHandle #if FEATURE_INTEROPSNI public readonly IntPtr NativePointer; - public readonly SNIPacketHandle NativePacket; + public readonly SNIPacket NativePacket; #endif public readonly SNI.SNIPacket ManagedPacket; public readonly int Type; private PacketHandle(IntPtr nativePointer, #if FEATURE_INTEROPSNI - SNIPacketHandle + SNIPacket #else int #endif @@ -61,7 +61,7 @@ public static PacketHandle FromNativePointer(IntPtr nativePointer) return new PacketHandle(nativePointer,default,default, NativePointerType); } - public static PacketHandle FromNativePacket(SNIPacketHandle nativePacket) + public static PacketHandle FromNativePacket(SNIPacket nativePacket) { return new PacketHandle(default, nativePacket, default, NativePacketType); } diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs index 27922a2c26ba..eeb07a251f2c 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs @@ -14,15 +14,15 @@ internal class TdsParserStateObjectNative : TdsParserStateObject { private SNIHandle _sessionHandle = null; // the SNI handle we're to work on - private SNIPacketHandle _sniPacket = null; // Will have to re-vamp this for MARS - internal SNIPacketHandle _sniAsyncAttnPacket = null; // Packet to use to send Attn + private 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 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 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) @@ -119,7 +119,7 @@ protected override void RemovePacketFromPendingList(PacketHandle ptr) Debug.Assert(ptr.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); IntPtr pointer = ptr.NativePointer; - SNIPacketHandle recoveredPacket; + SNIPacket recoveredPacket; lock (_writePacketLockObject) { @@ -216,7 +216,7 @@ internal override PacketHandle ReadAsync(SessionHandle handle, out uint error) internal override PacketHandle CreateAndSetAttentionPacket() { SNIHandle handle = Handle; - SNIPacketHandle attnPacket = new SNIPacketHandle(handle); + SNIPacket attnPacket = new SNIPacket(handle); _sniAsyncAttnPacket = attnPacket; SetPacketData(PacketHandle.FromNativePacket(attnPacket), SQL.AttentionHeader, TdsEnums.HEADER_LEN); return PacketHandle.FromNativePacket(attnPacket); @@ -231,7 +231,7 @@ internal override uint WritePacket(PacketHandle packet, bool sync) internal override PacketHandle AddPacketToPendingList(PacketHandle packetToAdd) { Debug.Assert(packetToAdd.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePacket"); - SNIPacketHandle packet = packetToAdd.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(); @@ -323,17 +323,17 @@ internal override void DisposePacketCache() internal sealed class WritePacketCache : IDisposable { private bool _disposed; - private Stack _packets; + private Stack _packets; public WritePacketCache() { _disposed = false; - _packets = new Stack(); + _packets = new Stack(); } - public SNIPacketHandle Take(SNIHandle sniHandle) + public SNIPacket Take(SNIHandle sniHandle) { - SNIPacketHandle packet; + SNIPacket packet; if (_packets.Count > 0) { // Success - reset the packet @@ -343,12 +343,12 @@ public SNIPacketHandle Take(SNIHandle sniHandle) else { // Failed to take a packet - create a new one - packet = new SNIPacketHandle(sniHandle); + packet = new SNIPacket(sniHandle); } return packet; } - public void Add(SNIPacketHandle packet) + public void Add(SNIPacket packet) { if (!_disposed) { From 1e1f540adefe9915dd0c801271a8579fffeffe20 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Mon, 17 Dec 2018 21:13:43 +0000 Subject: [PATCH 05/10] split PacketHandle and SessionHandle into separate files and implementations --- .../src/System.Data.SqlClient.csproj | 8 +- .../Data/SqlClient/PacketHandle.Unix.cs | 28 ++++++ .../Data/SqlClient/PacketHandle.Windows.cs | 45 +++++++++ .../Data/SqlClient/SessionHandle.Unix.cs | 29 ++++++ .../Data/SqlClient/SessionHandle.Windows.cs | 36 +++++++ .../Data/SqlClient/TdsParserStateObject.cs | 96 ------------------- 6 files changed, 145 insertions(+), 97 deletions(-) create mode 100644 src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Unix.cs create mode 100644 src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Windows.cs create mode 100644 src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Unix.cs create mode 100644 src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Windows.cs diff --git a/src/System.Data.SqlClient/src/System.Data.SqlClient.csproj b/src/System.Data.SqlClient/src/System.Data.SqlClient.csproj index c60ca96e13c2..35c08bd531e1 100644 --- a/src/System.Data.SqlClient/src/System.Data.SqlClient.csproj +++ b/src/System.Data.SqlClient/src/System.Data.SqlClient.csproj @@ -1,4 +1,4 @@ - + {D4550556-4745-457F-BA8F-3EBF3836D6B4} System.Data.SqlClient @@ -276,6 +276,8 @@ + + @@ -285,6 +287,8 @@ + + @@ -479,6 +483,8 @@ + + diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Unix.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Unix.cs new file mode 100644 index 000000000000..770da5ccd33f --- /dev/null +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Unix.cs @@ -0,0 +1,28 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + + +namespace System.Data.SqlClient +{ + internal readonly ref struct PacketHandle + { + public const int NativePointerType = 1; + public const int NativePacketType = 2; + public const int ManagedPacketType = 3; + + public readonly SNI.SNIPacket ManagedPacket; + public readonly int Type; + + private PacketHandle(SNI.SNIPacket managedPacket, int type) + { + Type = type; + ManagedPacket = managedPacket; + } + + public static PacketHandle FromManagedPacket(SNI.SNIPacket managedPacket) + { + return new PacketHandle(managedPacket, ManagedPacketType); + } + } +} diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Windows.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Windows.cs new file mode 100644 index 000000000000..bce060886975 --- /dev/null +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Windows.cs @@ -0,0 +1,45 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + + +namespace System.Data.SqlClient +{ + internal readonly ref struct PacketHandle + { + public const int NativePointerType = 1; + public const int NativePacketType = 2; + public const int ManagedPacketType = 3; + + public readonly IntPtr NativePointer; + public readonly SNIPacket NativePacket; + + public readonly SNI.SNIPacket ManagedPacket; + public readonly int Type; + + private PacketHandle(IntPtr nativePointer, SNIPacket nativePacket, SNI.SNIPacket managedPacket, int type) + { + Type = type; + ManagedPacket = managedPacket; + NativePointer = nativePointer; + NativePacket = nativePacket; + } + + public static PacketHandle FromManagedPacket(SNI.SNIPacket managedPacket) + { + return new PacketHandle(default, default, managedPacket, ManagedPacketType); + } + + public static PacketHandle FromNativePointer(IntPtr nativePointer) + { + return new PacketHandle(nativePointer,default,default, NativePointerType); + } + + public static PacketHandle FromNativePacket(SNIPacket nativePacket) + { + return new PacketHandle(default, nativePacket, default, NativePacketType); + } + + + } +} diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Unix.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Unix.cs new file mode 100644 index 000000000000..233d67ff0f1d --- /dev/null +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Unix.cs @@ -0,0 +1,29 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + + +namespace System.Data.SqlClient +{ + internal readonly ref struct SessionHandle + { + public const int NativeHandleType = 1; + public const int ManagedHandleType = 2; + + public readonly SNI.SNIHandle ManagedHandle; + public readonly int Type; + + public SessionHandle(SNI.SNIHandle managedHandle, int type) + { + Type = type; + ManagedHandle = managedHandle; + } + + public bool IsNull => ManagedHandle is null; + + public static SessionHandle FromManagedSession(SNI.SNIHandle managedSessionHandle) + { + return new SessionHandle(managedSessionHandle, ManagedHandleType); + } + } +} diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Windows.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Windows.cs new file mode 100644 index 000000000000..9c1e00b3aa25 --- /dev/null +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Windows.cs @@ -0,0 +1,36 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + + +namespace System.Data.SqlClient +{ + internal readonly ref struct SessionHandle + { + public const int NativeHandleType = 1; + public const int ManagedHandleType = 2; + + public readonly SNI.SNIHandle ManagedHandle; + public readonly SNIHandle NativeHandle; + + public readonly int Type; + + public SessionHandle(SNI.SNIHandle managedHandle, SNIHandle nativeHandle, int type) + { + Type = type; + ManagedHandle = managedHandle; + NativeHandle = nativeHandle; + } + + public bool IsNull => (Type == NativeHandleType) ? NativeHandle is null : ManagedHandle is null; + + public static SessionHandle FromManagedSession(SNI.SNIHandle managedSessionHandle) + { + return new SessionHandle(managedSessionHandle, default, ManagedHandleType); + } + public static SessionHandle FromNativeHandle(SNIHandle nativeSessionHandle) + { + return new SessionHandle(default, nativeSessionHandle, NativeHandleType); + } + } +} diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs index 466e9fe6bb08..8f33c505d760 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs @@ -20,102 +20,6 @@ sealed internal class LastIOTimer internal long _value; } - internal readonly ref struct PacketHandle - { - public const int NativePointerType = 1; - public const int NativePacketType = 2; - public const int ManagedPacketType = 3; - -#if FEATURE_INTEROPSNI - public readonly IntPtr NativePointer; - public readonly SNIPacket NativePacket; -#endif - public readonly SNI.SNIPacket ManagedPacket; - public readonly int Type; - - private PacketHandle(IntPtr nativePointer, -#if FEATURE_INTEROPSNI - SNIPacket -#else - int -#endif - - nativePacket, SNI.SNIPacket managedPacket, int type) - { - Type = type; - ManagedPacket = managedPacket; -#if FEATURE_INTEROPSNI - NativePointer = nativePointer; - NativePacket = nativePacket; -#endif - } - - public static PacketHandle FromManagedPacket(SNI.SNIPacket managedPacket) - { - return new PacketHandle(default, default, managedPacket, ManagedPacketType); - } - -#if FEATURE_INTEROPSNI - public static PacketHandle FromNativePointer(IntPtr nativePointer) - { - return new PacketHandle(nativePointer,default,default, NativePointerType); - } - - public static PacketHandle FromNativePacket(SNIPacket nativePacket) - { - return new PacketHandle(default, nativePacket, default, NativePacketType); - } -#endif - - - } - - internal readonly ref struct SessionHandle - { - public const int NativeHandleType = 1; - public const int ManagedHandleType = 2; - - public readonly SNI.SNIHandle ManagedHandle; -#if FEATURE_INTEROPSNI - public readonly SNIHandle NativeHandle; -#endif - public readonly int Type; - - public SessionHandle(SNI.SNIHandle managedHandle, -#if FEATURE_INTEROPSNI - SNIHandle -#else - int -#endif - nativeHandle, - int type - ) - { - Type = type; - ManagedHandle = managedHandle; -#if FEATURE_INTEROPSNI - NativeHandle = nativeHandle; -#endif - } - - public bool IsNull => -#if FEATURE_INTEROPSNI - (Type == NativeHandleType) ? NativeHandle == null : -#endif - ManagedHandle == null; - - public static SessionHandle FromManagedSession(SNI.SNIHandle managedSessionHandle) - { - return new SessionHandle(managedSessionHandle, default, ManagedHandleType); - } -#if FEATURE_INTEROPSNI - public static SessionHandle FromNativeHandle(SNIHandle nativeSessionHandle) - { - return new SessionHandle(default, nativeSessionHandle, NativeHandleType); - } -#endif - } - internal abstract class TdsParserStateObject { private const int AttentionTimeoutSeconds = 5; From 69a8337423187f5693f05c702337360eb3847331 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Tue, 18 Dec 2018 20:52:49 +0000 Subject: [PATCH 06/10] add comments to PacketHandle and SessionHandle remove unused packethandle variable in IsConnectionAlive remove identidal overridden implementations of EmptyReadHandle --- .../Data/SqlClient/PacketHandle.Unix.cs | 17 ++++++--- .../Data/SqlClient/PacketHandle.Windows.cs | 23 ++++++----- .../Data/SqlClient/SessionHandle.Unix.cs | 17 ++++++--- .../Data/SqlClient/SessionHandle.Windows.cs | 19 ++++++---- .../Data/SqlClient/TdsParserStateObject.cs | 38 ++++++------------- .../SqlClient/TdsParserStateObjectManaged.cs | 2 - .../SqlClient/TdsParserStateObjectNative.cs | 2 - 7 files changed, 56 insertions(+), 62 deletions(-) diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Unix.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Unix.cs index 770da5ccd33f..f9fd9dc7e904 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Unix.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Unix.cs @@ -5,7 +5,15 @@ namespace System.Data.SqlClient { - internal readonly ref struct PacketHandle + // this structure is used for transporting packet handle references between the TdsParserStateObject + // base class and Managed or Native implementations. + // It prevents the native IntPtr type from being boxed and prevents the need to cast from object which loses compile time type safety + // It carries type information so that assertions about the type of handle can be made in the implemented abstract methods + // it is a ref struct so that it can only be used to transport the handles and not store them + + // N.B. If you change this type you must also change the version for the other platform + + internal readonly ref struct PacketHandle { public const int NativePointerType = 1; public const int NativePacketType = 2; @@ -20,9 +28,6 @@ private PacketHandle(SNI.SNIPacket managedPacket, int type) ManagedPacket = managedPacket; } - public static PacketHandle FromManagedPacket(SNI.SNIPacket managedPacket) - { - return new PacketHandle(managedPacket, ManagedPacketType); - } - } + public static PacketHandle FromManagedPacket(SNI.SNIPacket managedPacket) => new PacketHandle(managedPacket, ManagedPacketType); + } } diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Windows.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Windows.cs index bce060886975..f15d26fd38b0 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Windows.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/PacketHandle.Windows.cs @@ -5,6 +5,14 @@ namespace System.Data.SqlClient { + // this structure is used for transporting packet handle references between the TdsParserStateObject + // base class and Managed or Native implementations. + // It prevents the native IntPtr type from being boxed and prevents the need to cast from object which loses compile time type safety + // It carries type information so that assertions about the type of handle can be made in the implemented abstract methods + // it is a ref struct so that it can only be used to transport the handles and not store them + + // N.B. If you change this type you must also change the version for the other platform + internal readonly ref struct PacketHandle { public const int NativePointerType = 1; @@ -25,20 +33,11 @@ private PacketHandle(IntPtr nativePointer, SNIPacket nativePacket, SNI.SNIPacket NativePacket = nativePacket; } - public static PacketHandle FromManagedPacket(SNI.SNIPacket managedPacket) - { - return new PacketHandle(default, default, managedPacket, ManagedPacketType); - } + public static PacketHandle FromManagedPacket(SNI.SNIPacket managedPacket) => new PacketHandle(default, default, managedPacket, ManagedPacketType); - public static PacketHandle FromNativePointer(IntPtr nativePointer) - { - return new PacketHandle(nativePointer,default,default, NativePointerType); - } + public static PacketHandle FromNativePointer(IntPtr nativePointer) => new PacketHandle(nativePointer, default, default, NativePointerType); - public static PacketHandle FromNativePacket(SNIPacket nativePacket) - { - return new PacketHandle(default, nativePacket, default, NativePacketType); - } + public static PacketHandle FromNativePacket(SNIPacket nativePacket) => new PacketHandle(default, nativePacket, default, NativePacketType); } diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Unix.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Unix.cs index 233d67ff0f1d..5bf099aba58e 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Unix.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Unix.cs @@ -5,7 +5,15 @@ namespace System.Data.SqlClient { - internal readonly ref struct SessionHandle + // this structure is used for transporting packet handle references between the TdsParserStateObject + // base class and Managed or Native implementations. + // It carries type information so that assertions about the type of handle can be made in the + // implemented abstract methods + // it is a ref struct so that it can only be used to transport the handles and not store them + + // N.B. If you change this type you must also change the version for the other platform + + internal readonly ref struct SessionHandle { public const int NativeHandleType = 1; public const int ManagedHandleType = 2; @@ -21,9 +29,6 @@ public SessionHandle(SNI.SNIHandle managedHandle, int type) public bool IsNull => ManagedHandle is null; - public static SessionHandle FromManagedSession(SNI.SNIHandle managedSessionHandle) - { - return new SessionHandle(managedSessionHandle, ManagedHandleType); - } - } + public static SessionHandle FromManagedSession(SNI.SNIHandle managedSessionHandle) => new SessionHandle(managedSessionHandle, ManagedHandleType); + } } diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Windows.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Windows.cs index 9c1e00b3aa25..a7215963c8b2 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Windows.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/SessionHandle.Windows.cs @@ -5,6 +5,14 @@ namespace System.Data.SqlClient { + // this structure is used for transporting packet handle references between the TdsParserStateObject + // base class and Managed or Native implementations. + // It carries type information so that assertions about the type of handle can be made in the + // implemented abstract methods + // it is a ref struct so that it can only be used to transport the handles and not store them + + // N.B. If you change this type you must also change the version for the other platform + internal readonly ref struct SessionHandle { public const int NativeHandleType = 1; @@ -24,13 +32,8 @@ public SessionHandle(SNI.SNIHandle managedHandle, SNIHandle nativeHandle, int ty public bool IsNull => (Type == NativeHandleType) ? NativeHandle is null : ManagedHandle is null; - public static SessionHandle FromManagedSession(SNI.SNIHandle managedSessionHandle) - { - return new SessionHandle(managedSessionHandle, default, ManagedHandleType); - } - public static SessionHandle FromNativeHandle(SNIHandle nativeSessionHandle) - { - return new SessionHandle(default, nativeSessionHandle, NativeHandleType); - } + public static SessionHandle FromManagedSession(SNI.SNIHandle managedSessionHandle) => new SessionHandle(managedSessionHandle, default, ManagedHandleType); + + public static SessionHandle FromNativeHandle(SNIHandle nativeSessionHandle) => new SessionHandle(default, nativeSessionHandle, NativeHandleType); } } diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs index 8f33c505d760..c0600b8a0125 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs @@ -2416,37 +2416,23 @@ internal bool IsConnectionAlive(bool throwOnException) else { uint error; + SniContext = SniContext.Snix_Connect; + error = CheckConnection(); - PacketHandle readPacket = EmptyReadPacket; - - try + if ((error != TdsEnums.SNI_SUCCESS) && (error != TdsEnums.SNI_WAIT_TIMEOUT)) { - SniContext = SniContext.Snix_Connect; - - error = CheckConnection(); - - if ((error != TdsEnums.SNI_SUCCESS) && (error != TdsEnums.SNI_WAIT_TIMEOUT)) - { - // Connection is dead - isAlive = false; - if (throwOnException) - { - // Get the error from SNI so that we can throw the correct exception - AddError(_parser.ProcessSNIError(this)); - ThrowExceptionAndWarning(); - } - } - else + // Connection is dead + isAlive = false; + if (throwOnException) { - _lastSuccessfulIOTimer._value = DateTime.UtcNow.Ticks; + // Get the error from SNI so that we can throw the correct exception + AddError(_parser.ProcessSNIError(this)); + ThrowExceptionAndWarning(); } } - finally + else { - if (!IsPacketEmpty(readPacket)) - { - ReleasePacket(readPacket); - } + _lastSuccessfulIOTimer._value = DateTime.UtcNow.Ticks; } } } @@ -3658,7 +3644,7 @@ internal int WarningCount } } - protected abstract PacketHandle EmptyReadPacket { get; } + protected virtual PacketHandle EmptyReadPacket => default; /// /// Gets the full list of errors and warnings (including the pre-attention ones), then wipes all error and warning lists diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs index e70613dbb5b4..3754f58029ba 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs @@ -33,8 +33,6 @@ internal TdsParserStateObjectManaged(TdsParser parser, TdsParserStateObject phys internal override SessionHandle SessionHandle => SessionHandle.FromManagedSession(_sessionHandle); - protected override PacketHandle EmptyReadPacket => default; - protected override bool CheckPacket(PacketHandle packet, TaskCompletionSource source) { SNIPacket p = packet.ManagedPacket; diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs index eeb07a251f2c..5d62ae425c1b 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs @@ -35,8 +35,6 @@ internal TdsParserStateObjectNative(TdsParser parser, TdsParserStateObject physi internal override SessionHandle SessionHandle => SessionHandle.FromNativeHandle(_sessionHandle); - protected override PacketHandle EmptyReadPacket => default; - protected override void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async) { Debug.Assert(physicalConnection is TdsParserStateObjectNative, "Expected a stateObject of type " + this.GetType()); From 6b1900c4cc21a8586748480cad9488a91ec1d628 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Tue, 18 Dec 2018 21:02:57 +0000 Subject: [PATCH 07/10] move lazy bool into debug region --- .../Data/SqlClient/TdsParserStateObjectFactory.Windows.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectFactory.Windows.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectFactory.Windows.cs index 9d767d6d7f8f..bf60579c3dd2 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectFactory.Windows.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectFactory.Windows.cs @@ -17,11 +17,13 @@ internal sealed class TdsParserStateObjectFactory // Temporary disabling App Context switching for managed SNI. // If the appcontext switch is set then Use Managed SNI based on the value. Otherwise Managed SNI should always be used. //private static bool shouldUseLegacyNetorking; - private static Lazy shouldUseLegacyNetorking = new Lazy(() => bool.TrueString.Equals(Environment.GetEnvironmentVariable("System.Data.SqlClient.UseLegacyNetworkingOnWindows"), StringComparison.InvariantCultureIgnoreCase)); //public static bool UseManagedSNI { get; } = AppContext.TryGetSwitch(UseLegacyNetworkingOnWindows, out shouldUseLegacyNetorking) ? !shouldUseLegacyNetorking : true; - #if DEBUG + private static Lazy shouldUseLegacyNetorking = new Lazy( + () => bool.TrueString.Equals(Environment.GetEnvironmentVariable("System.Data.SqlClient.UseLegacyNetworkingOnWindows"), StringComparison.InvariantCultureIgnoreCase) + ); + public static bool UseManagedSNI => shouldUseLegacyNetorking.Value; #else public static bool UseManagedSNI { get; } = false; From dc2408442bcbe84be36ed47bbeb787d4f99b7ffb Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Wed, 19 Dec 2018 21:10:31 +0000 Subject: [PATCH 08/10] re-add EmptyReadPackt and provide correctly types valid but empty packets in implementations define IsValidPacket implementations more stringently with type checks --- .../src/System/Data/SqlClient/TdsParserStateObject.cs | 2 +- .../Data/SqlClient/TdsParserStateObjectManaged.cs | 11 ++++++++++- .../Data/SqlClient/TdsParserStateObjectNative.cs | 10 ++++++++-- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs index c0600b8a0125..e6ba9a6674a2 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs @@ -3644,7 +3644,7 @@ internal int WarningCount } } - protected virtual PacketHandle EmptyReadPacket => default; + protected abstract PacketHandle EmptyReadPacket { get; } /// /// Gets the full list of errors and warnings (including the pre-attention ones), then wipes all error and warning lists diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs index 3754f58029ba..62fd97c66fd4 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs @@ -135,6 +135,8 @@ internal override PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint return PacketHandle.FromManagedPacket(packet); } + protected override PacketHandle EmptyReadPacket => PacketHandle.FromManagedPacket(null); + internal override bool IsPacketEmpty(PacketHandle packet) { return packet.ManagedPacket == null; @@ -180,7 +182,14 @@ internal override PacketHandle AddPacketToPendingList(PacketHandle packet) return packet; } - internal override bool IsValidPacket(PacketHandle packet) => packet.ManagedPacket?.IsInvalid ?? false; + internal override bool IsValidPacket(PacketHandle packet) + { + return ( + packet.Type == PacketHandle.ManagedPacketType && + packet.ManagedPacket != null && + !packet.ManagedPacket.IsInvalid + ); + } internal override PacketHandle GetResetWritePacket() { diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs index 5d62ae425c1b..5c43bdb07902 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs @@ -185,6 +185,8 @@ internal override PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint return PacketHandle.FromNativePointer(readPacketPtr); } + protected override PacketHandle EmptyReadPacket => PacketHandle.FromNativePointer(default); + internal override bool IsPacketEmpty(PacketHandle readPacket) { Debug.Assert(readPacket.Type == PacketHandle.NativePointerType || readPacket.Type == 0, "unexpected packet type when requiring NativePointer"); @@ -244,8 +246,12 @@ internal override PacketHandle AddPacketToPendingList(PacketHandle packetToAdd) internal override bool IsValidPacket(PacketHandle packetPointer) { - Debug.Assert(packetPointer.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); - return packetPointer.NativePointer != IntPtr.Zero; + 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 PacketHandle GetResetWritePacket() From 4801eb3f327f826e1ebadb8101ed93665dcbd071 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Wed, 19 Dec 2018 21:19:11 +0000 Subject: [PATCH 09/10] change implementation switch name to make more sense --- .../Data/SqlClient/TdsParserStateObjectFactory.Windows.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectFactory.Windows.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectFactory.Windows.cs index bf60579c3dd2..fb57c55f556e 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectFactory.Windows.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectFactory.Windows.cs @@ -20,11 +20,10 @@ internal sealed class TdsParserStateObjectFactory //public static bool UseManagedSNI { get; } = AppContext.TryGetSwitch(UseLegacyNetworkingOnWindows, out shouldUseLegacyNetorking) ? !shouldUseLegacyNetorking : true; #if DEBUG - private static Lazy shouldUseLegacyNetorking = new Lazy( - () => bool.TrueString.Equals(Environment.GetEnvironmentVariable("System.Data.SqlClient.UseLegacyNetworkingOnWindows"), StringComparison.InvariantCultureIgnoreCase) + private static Lazy useManagedSNIOnWindows = new Lazy( + () => bool.TrueString.Equals(Environment.GetEnvironmentVariable("System.Data.SqlClient.UseManagedSNIOnWindows"), StringComparison.InvariantCultureIgnoreCase) ); - - public static bool UseManagedSNI => shouldUseLegacyNetorking.Value; + public static bool UseManagedSNI => useManagedSNIOnWindows.Value; #else public static bool UseManagedSNI { get; } = false; #endif From a76208fd8584a4a4f5805409ad3561bacaf550b3 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Wed, 19 Dec 2018 23:13:34 +0000 Subject: [PATCH 10/10] add packet type assertion in IsValiePacket --- .../src/System/Data/SqlClient/TdsParserStateObjectManaged.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs index 62fd97c66fd4..151d4e554aa0 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectManaged.cs @@ -184,6 +184,7 @@ internal override PacketHandle AddPacketToPendingList(PacketHandle packet) internal override bool IsValidPacket(PacketHandle packet) { + Debug.Assert(packet.Type == PacketHandle.ManagedPacketType, "unexpected packet type when requiring ManagedPacket"); return ( packet.Type == PacketHandle.ManagedPacketType && packet.ManagedPacket != null &&