From 81f549eedfbd8b94b94495c19152ef7f046f4a98 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 18:57:30 +0200 Subject: [PATCH] Merging of TdsParserStateObject.ReadAsyncCallback (note: ports parts of #378 and #528 to netfx). --- .../Data/SqlClient/TdsParserStateObject.cs | 151 ----------------- .../Data/SqlClient/TdsParserStateObject.cs | 142 +--------------- .../Data/SqlClient/TdsParserStateObject.cs | 156 ++++++++++++++++++ 3 files changed, 158 insertions(+), 291 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index b13042a535..21f36c4774 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -19,8 +19,6 @@ internal abstract partial class TdsParserStateObject { private static bool UseManagedSNI => TdsParserStateObjectFactory.UseManagedSNI; - private static readonly ContextCallback s_readAsyncCallbackComplete = ReadAsyncCallbackComplete; - // Timeout variables private readonly WeakReference _cancellationOwner = new WeakReference(null); @@ -394,157 +392,8 @@ private void SetBufferSecureStrings() public void ReadAsyncCallback(PacketHandle packet, uint error) => ReadAsyncCallback(IntPtr.Zero, packet, 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 - // could occur in the native->managed transition, which would - // have two impacts: - // 1) user event not called - // 2) DecrementPendingCallbacks not called, which would mean this object would be leaked due - // to the outstanding GCRoot until AppDomain.Unload. - // We live with the above for the time being due to the constraints of the current - // reliability infrastructure provided by the CLR. - SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObject.ReadAsyncCallback | Info | State Object Id {0}, received error {1} on idle connection", _objectID, (int)error); - - TaskCompletionSource source = _networkPacketTaskSource; -#if DEBUG - if ((s_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) - { - source = _realNetworkPacketTaskSource; - } -#endif - - // The mars physical connection can get a callback - // with a packet but no result after the connection is closed. - if (source == null && _parser._pMarsPhysicalConObj == this) - { - return; - } - - bool processFinallyBlock = true; - try - { - Debug.Assert(CheckPacket(packet, source) && source != null, "AsyncResult null on callback"); - - if (_parser.MARSOn) - { - // Only take reset lock on MARS and Async. - CheckSetResetConnectionState(error, CallbackType.Read); - } - - ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); - - // The timer thread may be unreliable under high contention scenarios. It cannot be - // assumed that the timeout has happened on the timer thread callback. Check the timeout - // synchrnously and then call OnTimeoutSync to force an atomic change of state. - if (TimeoutHasExpired) - { - OnTimeoutSync(true); - } - - // try to change to the stopped state but only do so if currently in the running state - // and use cmpexch so that all changes out of the running state are atomic - int previousState = Interlocked.CompareExchange(ref _timeoutState, TimeoutState.Stopped, TimeoutState.Running); - - // if the state is anything other than running then this query has reached an end so - // set the correlation _timeoutIdentityValue to 0 to prevent late callbacks executing - if (_timeoutState != TimeoutState.Running) - { - _timeoutIdentityValue = 0; - } - - ProcessSniPacket(packet, error); - } - catch (Exception e) - { - processFinallyBlock = ADP.IsCatchableExceptionType(e); - throw; - } - finally - { - // pendingCallbacks may be 2 after decrementing, this indicates that a fatal timeout is occurring, and therefore we shouldn't complete the task - int pendingCallbacks = DecrementPendingCallbacks(false); // may dispose of GC handle. - if ((processFinallyBlock) && (source != null) && (pendingCallbacks < 2)) - { - if (error == 0) - { - if (_executionContext != null) - { - ExecutionContext.Run(_executionContext, s_readAsyncCallbackComplete, source); - } - else - { - source.TrySetResult(null); - } - } - else - { - if (_executionContext != null) - { - ExecutionContext.Run(_executionContext, state => ReadAsyncCallbackCaptureException((TaskCompletionSource)state), source); - } - else - { - ReadAsyncCallbackCaptureException(source); - } - } - } - - AssertValidState(); - } - } - - private static void ReadAsyncCallbackComplete(object state) - { - TaskCompletionSource source = (TaskCompletionSource)state; - source.TrySetResult(null); - } - protected abstract bool CheckPacket(PacketHandle packet, TaskCompletionSource source); - private void ReadAsyncCallbackCaptureException(TaskCompletionSource source) - { - bool captureSuccess = false; - try - { - if (_hasErrorOrWarning) - { - // Do the close on another thread, since we don't want to block the callback thread - ThrowExceptionAndWarning(asyncClose: true); - } - else if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken)) - { - // Connection was closed by another thread before we parsed the packet, so no error was added to the collection - throw ADP.ClosedConnectionError(); - } - } - catch (Exception ex) - { - if (source.TrySetException(ex)) - { - // There was an exception, and it was successfully stored in the task - captureSuccess = true; - } - } - - if (!captureSuccess) - { - // Either there was no exception, or the task was already completed - // This is unusual, but possible if a fatal timeout occurred on another thread (which should mean that the connection is now broken) - Debug.Assert(_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed || _parser.Connection.IsConnectionDoomed, "Failed to capture exception while the connection was still healthy"); - - // The safest thing to do is to ensure that the connection is broken and attempt to cancel the task - // This must be done from another thread to not block the callback thread - Task.Factory.StartNew(() => - { - _parser.State = TdsParserState.Broken; - _parser.Connection.BreakConnection(); - source.TrySetCanceled(); - }); - } - } - 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.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index b3e8ac2f64..ad5a38ee9d 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -464,6 +464,8 @@ private IntPtr ReadSyncOverAsync(int timeout, out uint error) private bool IsValidPacket(IntPtr packet) => packet != IntPtr.Zero; + private bool CheckPacket(IntPtr packet, TaskCompletionSource source) => IntPtr.Zero == packet || IntPtr.Zero != packet && source != null; + private void ReleasePacket(IntPtr packet) => SNINativeMethodWrapper.SNIPacketRelease(packet); private IntPtr ReadAsync(SessionHandle handle, out uint error) @@ -484,146 +486,6 @@ private uint SNIPacketGetData(IntPtr packet, byte[] _inBuff, ref uint dataSize) return SNINativeMethodWrapper.SNIPacketGetData(packet, _inBuff, ref dataSize); } - public void ReadAsyncCallback(IntPtr key, IntPtr packet, uint error) - { // Key never used. - // Note - it's possible that when native calls managed that an asynchronous exception - // could occur in the native->managed transition, which would - // have two impacts: - // 1) user event not called - // 2) DecrementPendingCallbacks not called, which would mean this object would be leaked due - // to the outstanding GCRoot until AppDomain.Unload. - // We live with the above for the time being due to the constraints of the current - // reliability infrastructure provided by the CLR. - - TaskCompletionSource source = _networkPacketTaskSource; -#if DEBUG - if ((s_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) - { - source = _realNetworkPacketTaskSource; - } -#endif - - // The mars physical connection can get a callback - // with a packet but no result after the connection is closed. - if (source == null && _parser._pMarsPhysicalConObj == this) - { - return; - } - - RuntimeHelpers.PrepareConstrainedRegions(); - bool processFinallyBlock = true; - try - { - Debug.Assert(IntPtr.Zero == packet || IntPtr.Zero != packet && source != null, "AsyncResult null on callback"); - if (_parser.MARSOn) - { // Only take reset lock on MARS and Async. - CheckSetResetConnectionState(error, CallbackType.Read); - } - - ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); - - // The timer thread may be unreliable under high contention scenarios. It cannot be - // assumed that the timeout has happened on the timer thread callback. Check the timeout - // synchrnously and then call OnTimeoutSync to force an atomic change of state. - if (TimeoutHasExpired) - { - OnTimeoutSync(asyncClose: true); - } - - // try to change to the stopped state but only do so if currently in the running state - // and use cmpexch so that all changes out of the running state are atomic - int previousState = Interlocked.CompareExchange(ref _timeoutState, TimeoutState.Stopped, TimeoutState.Running); - - // if the state is anything other than running then this query has reached an end so - // set the correlation _timeoutIdentityValue to 0 to prevent late callbacks executing - if (_timeoutState != TimeoutState.Running) - { - _timeoutIdentityValue = 0; - } - - ProcessSniPacket(packet, error); - } - catch (Exception e) - { - processFinallyBlock = ADP.IsCatchableExceptionType(e); - throw; - } - finally - { - // pendingCallbacks may be 2 after decrementing, this indicates that a fatal timeout is occuring, and therefore we shouldn't complete the task - int pendingCallbacks = DecrementPendingCallbacks(false); // may dispose of GC handle. - if ((processFinallyBlock) && (source != null) && (pendingCallbacks < 2)) - { - if (error == 0) - { - if (_executionContext != null) - { - ExecutionContext.Run(_executionContext, (state) => source.TrySetResult(null), null); - } - else - { - source.TrySetResult(null); - } - } - else - { - if (_executionContext != null) - { - ExecutionContext.Run(_executionContext, (state) => ReadAsyncCallbackCaptureException(source), null); - } - else - { - ReadAsyncCallbackCaptureException(source); - } - } - } - - AssertValidState(); - } - } - - private void ReadAsyncCallbackCaptureException(TaskCompletionSource source) - { - bool captureSuccess = false; - try - { - if (_hasErrorOrWarning) - { - // Do the close on another thread, since we don't want to block the callback thread - ThrowExceptionAndWarning(asyncClose: true); - } - else if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken)) - { - // Connection was closed by another thread before we parsed the packet, so no error was added to the collection - throw ADP.ClosedConnectionError(); - } - } - catch (Exception ex) - { - if (source.TrySetException(ex)) - { - // There was an exception, and it was successfully stored in the task - captureSuccess = true; - } - } - - if (!captureSuccess) - { - // Either there was no exception, or the task was already completed - // This is unusual, but possible if a fatal timeout occurred on another thread (which should mean that the connection is now broken) - Debug.Assert(_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed || _parser.Connection.IsConnectionDoomed, "Failed to capture exception while the connection was still healthy"); - - // The safest thing to do is to ensure that the connection is broken and attempt to cancel the task - // This must be done from another thread to not block the callback thread - Task.Factory.StartNew(() => - { - _parser.State = TdsParserState.Broken; - _parser.Connection.BreakConnection(); - source.TrySetCanceled(); - }); - } - } - #pragma warning disable 420 // a reference to a volatile field will not be treated as volatile public void WriteAsyncCallback(IntPtr key, IntPtr packet, uint sniError) 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 afb5e02f0c..626654e141 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -24,6 +24,8 @@ sealed internal class LastIOTimer partial class TdsParserStateObject { + private static readonly ContextCallback s_readAsyncCallbackComplete = ReadAsyncCallbackComplete; + private static int s_objectTypeCount; // EventSource counter internal readonly int _objectID = Interlocked.Increment(ref s_objectTypeCount); @@ -2622,6 +2624,160 @@ private void ChangeNetworkPacketTimeout(int dueTime, int period) } } + 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 + // could occur in the native->managed transition, which would + // have two impacts: + // 1) user event not called + // 2) DecrementPendingCallbacks not called, which would mean this object would be leaked due + // to the outstanding GCRoot until AppDomain.Unload. + // We live with the above for the time being due to the constraints of the current + // reliability infrastructure provided by the CLR. +#if !NETFRAMEWORK + SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObject.ReadAsyncCallback | Info | State Object Id {0}, received error {1} on idle connection", _objectID, (int)error); +#endif + + TaskCompletionSource source = _networkPacketTaskSource; +#if DEBUG + if ((s_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) + { + source = _realNetworkPacketTaskSource; + } +#endif + + // The mars physical connection can get a callback + // with a packet but no result after the connection is closed. + if (source == null && _parser._pMarsPhysicalConObj == this) + { + return; + } + +#if NETFRAMEWORK + System.Runtime.CompilerServices.RuntimeHelpers.PrepareConstrainedRegions(); +#endif + bool processFinallyBlock = true; + try + { + Debug.Assert(CheckPacket(packet, source) && source != null, "AsyncResult null on callback"); + + if (_parser.MARSOn) + { + // Only take reset lock on MARS and Async. + CheckSetResetConnectionState(error, CallbackType.Read); + } + + ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); + + // The timer thread may be unreliable under high contention scenarios. It cannot be + // assumed that the timeout has happened on the timer thread callback. Check the timeout + // synchrnously and then call OnTimeoutSync to force an atomic change of state. + if (TimeoutHasExpired) + { + OnTimeoutSync(true); + } + + // try to change to the stopped state but only do so if currently in the running state + // and use cmpexch so that all changes out of the running state are atomic + int previousState = Interlocked.CompareExchange(ref _timeoutState, TimeoutState.Stopped, TimeoutState.Running); + + // if the state is anything other than running then this query has reached an end so + // set the correlation _timeoutIdentityValue to 0 to prevent late callbacks executing + if (_timeoutState != TimeoutState.Running) + { + _timeoutIdentityValue = 0; + } + + ProcessSniPacket(packet, error); + } + catch (Exception e) + { + processFinallyBlock = ADP.IsCatchableExceptionType(e); + throw; + } + finally + { + // pendingCallbacks may be 2 after decrementing, this indicates that a fatal timeout is occurring, and therefore we shouldn't complete the task + int pendingCallbacks = DecrementPendingCallbacks(false); // may dispose of GC handle. + if ((processFinallyBlock) && (source != null) && (pendingCallbacks < 2)) + { + if (error == 0) + { + if (_executionContext != null) + { + ExecutionContext.Run(_executionContext, s_readAsyncCallbackComplete, source); + } + else + { + source.TrySetResult(null); + } + } + else + { + if (_executionContext != null) + { + ExecutionContext.Run(_executionContext, state => ReadAsyncCallbackCaptureException((TaskCompletionSource)state), source); + } + else + { + ReadAsyncCallbackCaptureException(source); + } + } + } + + AssertValidState(); + } + } + + private static void ReadAsyncCallbackComplete(object state) + { + TaskCompletionSource source = (TaskCompletionSource)state; + source.TrySetResult(null); + } + + private void ReadAsyncCallbackCaptureException(TaskCompletionSource source) + { + bool captureSuccess = false; + try + { + if (_hasErrorOrWarning) + { + // Do the close on another thread, since we don't want to block the callback thread + ThrowExceptionAndWarning(asyncClose: true); + } + else if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken)) + { + // Connection was closed by another thread before we parsed the packet, so no error was added to the collection + throw ADP.ClosedConnectionError(); + } + } + catch (Exception ex) + { + if (source.TrySetException(ex)) + { + // There was an exception, and it was successfully stored in the task + captureSuccess = true; + } + } + + if (!captureSuccess) + { + // Either there was no exception, or the task was already completed + // This is unusual, but possible if a fatal timeout occurred on another thread (which should mean that the connection is now broken) + Debug.Assert(_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed || _parser.Connection.IsConnectionDoomed, "Failed to capture exception while the connection was still healthy"); + + // The safest thing to do is to ensure that the connection is broken and attempt to cancel the task + // This must be done from another thread to not block the callback thread + Task.Factory.StartNew(() => + { + _parser.State = TdsParserState.Broken; + _parser.Connection.BreakConnection(); + source.TrySetCanceled(); + }); + } + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;)