diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlCommand.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlCommand.cs index 53abc5286adb..6416f49e4589 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlCommand.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlCommand.cs @@ -51,9 +51,9 @@ private enum EXECTYPE // The OnReturnValue function will test this flag to determine whether the returned value is a _prepareHandle or something else. // // _prepareHandle - the handle of a prepared command. Apparently there can be multiple prepared commands at a time - a feature that we do not support yet. - + private static readonly object s_cachedInvalidPrepareHandle = (object)-1; private bool _inPrepare = false; - private int _prepareHandle = -1; + private object _prepareHandle = s_cachedInvalidPrepareHandle; // this is an int which is used in the object typed SqlParameter.Value field, avoid repeated boxing by storing in a box private bool _hiddenPrepare = false; private int _preparedConnectionCloseCount = -1; private int _preparedConnectionReconnectCount = -1; @@ -83,7 +83,7 @@ internal bool InPrepare } // Cached info for async executions - private class CachedAsyncState + private sealed class CachedAsyncState { private int _cachedAsyncCloseCount = -1; // value of the connection's CloseCount property when the asyncResult was set; tracks when connections are closed after an async operation private TaskCompletionSource _cachedAsyncResult = null; @@ -261,7 +261,7 @@ private SqlCommand(SqlCommand from) : this() // Don't allow the connection to be changed while in an async operation. if (_activeConnection != value && _activeConnection != null) { // If new value... - if (cachedAsyncState.PendingAsyncOperation) + if (_cachedAsyncState != null && _cachedAsyncState.PendingAsyncOperation) { // If in pending async state, throw. throw SQL.CannotModifyPropertyAsyncOperationInProgress(); } @@ -292,7 +292,7 @@ private SqlCommand(SqlCommand from) : this() finally { // clean prepare status (even successful Unprepare does not do that) - _prepareHandle = -1; + _prepareHandle = s_cachedInvalidPrepareHandle; _execType = EXECTYPE.UNPREPARED; } } @@ -672,7 +672,7 @@ internal void Unprepare() if ((_activeConnection.CloseCount != _preparedConnectionCloseCount) || (_activeConnection.ReconnectCount != _preparedConnectionReconnectCount)) { // reset our handle - _prepareHandle = -1; + _prepareHandle = s_cachedInvalidPrepareHandle; } _cachedMetaData = null; @@ -918,7 +918,13 @@ public IAsyncResult BeginExecuteNonQuery(AsyncCallback callback, object stateObj cachedAsyncState.SetActiveConnectionAndResult(completion, nameof(EndExecuteNonQuery), _activeConnection); if (execNQ != null) { - AsyncHelper.ContinueTask(execNQ, completion, () => BeginExecuteNonQueryInternalReadStage(completion)); + AsyncHelper.ContinueTaskWithState(execNQ, completion, + state: Tuple.Create(this, completion), + onSuccess: state => { + var parameters = (Tuple>)state; + parameters.Item1.BeginExecuteNonQueryInternalReadStage(parameters.Item2); + } + ); } else { @@ -941,7 +947,10 @@ public IAsyncResult BeginExecuteNonQuery(AsyncCallback callback, object stateObj // Add callback after work is done to avoid overlapping Begin\End methods if (callback != null) { - completion.Task.ContinueWith((t) => callback(t), TaskScheduler.Default); + completion.Task.ContinueWith( + (task,state) => ((AsyncCallback)state)(task), + state: callback + ); } return completion.Task; @@ -1170,7 +1179,10 @@ private Task InternalExecuteNonQuery(TaskCompletionSource completion, bo { if (task != null) { - task = AsyncHelper.CreateContinuationTask(task, () => reader.Close()); + task = AsyncHelper.CreateContinuationTaskWithState(task, + state: reader, + onSuccess: state => ((SqlDataReader)state).Close() + ); } else { @@ -1265,7 +1277,13 @@ public IAsyncResult BeginExecuteXmlReader(AsyncCallback callback, object stateOb cachedAsyncState.SetActiveConnectionAndResult(completion, nameof(EndExecuteXmlReader), _activeConnection); if (writeTask != null) { - AsyncHelper.ContinueTask(writeTask, completion, () => BeginExecuteXmlReaderInternalReadStage(completion)); + AsyncHelper.ContinueTaskWithState(writeTask, completion, + state: Tuple.Create(this, completion), + onSuccess: state => { + var parameters = (Tuple>)state; + parameters.Item1.BeginExecuteXmlReaderInternalReadStage(parameters.Item2); + } + ); } else { @@ -1528,7 +1546,13 @@ internal IAsyncResult BeginExecuteReader(CommandBehavior behavior, AsyncCallback cachedAsyncState.SetActiveConnectionAndResult(completion, nameof(EndExecuteReader), _activeConnection); if (writeTask != null) { - AsyncHelper.ContinueTask(writeTask, completion, () => BeginExecuteReaderInternalReadStage(completion)); + AsyncHelper.ContinueTaskWithState(writeTask, completion, + state: Tuple.Create(this, completion), + onSuccess: state => { + var parameters = (Tuple>)state; + parameters.Item1.BeginExecuteReaderInternalReadStage(parameters.Item2); + } + ); } else { @@ -2328,27 +2352,7 @@ private Task RunExecuteNonQueryTds(string methodName, bool async, int timeout, b TaskCompletionSource completion = new TaskCompletionSource(); _activeConnection.RegisterWaitingForReconnect(completion.Task); _reconnectionCompletionSource = completion; - CancellationTokenSource timeoutCTS = new CancellationTokenSource(); - AsyncHelper.SetTimeoutException(completion, timeout, SQL.CR_ReconnectTimeout, timeoutCTS.Token); - AsyncHelper.ContinueTask(reconnectTask, completion, - () => - { - if (completion.Task.IsCompleted) - { - return; - } - Interlocked.CompareExchange(ref _reconnectionCompletionSource, null, completion); - timeoutCTS.Cancel(); - Task subTask = RunExecuteNonQueryTds(methodName, async, TdsParserStaticMethods.GetRemainingTimeout(timeout, reconnectionStart), asyncWrite); - if (subTask == null) - { - completion.SetResult(null); - } - else - { - AsyncHelper.ContinueTask(subTask, completion, () => completion.SetResult(null)); - } - }, connectionToAbort: _activeConnection); + RunExecuteNonQueryTdsSetupReconnnectContinuation(methodName, async, timeout, asyncWrite, reconnectTask, reconnectionStart, completion); return completion.Task; } else @@ -2401,6 +2405,31 @@ private Task RunExecuteNonQueryTds(string methodName, bool async, int timeout, b return null; } + // This is in its own method to avoid always allocating the lambda in RunExecuteNonQueryTds, cannot use ContinueTaskWithState because of MarshalByRef and the CompareExchange + private void RunExecuteNonQueryTdsSetupReconnnectContinuation(string methodName, bool async, int timeout, bool asyncWrite, Task reconnectTask, long reconnectionStart, TaskCompletionSource completion) + { + CancellationTokenSource timeoutCTS = new CancellationTokenSource(); + AsyncHelper.SetTimeoutException(completion, timeout, SQL.CR_ReconnectTimeout, timeoutCTS.Token); + AsyncHelper.ContinueTask(reconnectTask, completion, + () => + { + if (completion.Task.IsCompleted) + { + return; + } + Interlocked.CompareExchange(ref _reconnectionCompletionSource, null, completion); + timeoutCTS.Cancel(); + Task subTask = RunExecuteNonQueryTds(methodName, async, TdsParserStaticMethods.GetRemainingTimeout(timeout, reconnectionStart), asyncWrite); + if (subTask == null) + { + completion.SetResult(null); + } + else + { + AsyncHelper.ContinueTask(subTask, completion, () => completion.SetResult(null)); + } + }, connectionToAbort: _activeConnection); + } internal SqlDataReader RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, bool returnStream, [CallerMemberName] string method = "") { @@ -2470,28 +2499,7 @@ private SqlDataReader RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavi TaskCompletionSource completion = new TaskCompletionSource(); _activeConnection.RegisterWaitingForReconnect(completion.Task); _reconnectionCompletionSource = completion; - CancellationTokenSource timeoutCTS = new CancellationTokenSource(); - AsyncHelper.SetTimeoutException(completion, timeout, SQL.CR_ReconnectTimeout, timeoutCTS.Token); - AsyncHelper.ContinueTask(reconnectTask, completion, - () => - { - if (completion.Task.IsCompleted) - { - return; - } - Interlocked.CompareExchange(ref _reconnectionCompletionSource, null, completion); - timeoutCTS.Cancel(); - Task subTask; - RunExecuteReaderTds(cmdBehavior, runBehavior, returnStream, async, TdsParserStaticMethods.GetRemainingTimeout(timeout, reconnectionStart), out subTask, asyncWrite, ds); - if (subTask == null) - { - completion.SetResult(null); - } - else - { - AsyncHelper.ContinueTask(subTask, completion, () => completion.SetResult(null)); - } - }, connectionToAbort: _activeConnection); + RunExecuteReaderTdsSetupReconnectContinuation(cmdBehavior, runBehavior, returnStream, async, timeout, asyncWrite, ds, reconnectTask, reconnectionStart, completion); task = completion.Task; return ds; } @@ -2568,7 +2576,7 @@ private SqlDataReader RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavi if (_execType == EXECTYPE.PREPARED) { - Debug.Assert(this.IsPrepared && (_prepareHandle != -1), "invalid attempt to call sp_execute without a handle!"); + Debug.Assert(this.IsPrepared && ((int)_prepareHandle != -1), "invalid attempt to call sp_execute without a handle!"); rpc = BuildExecute(inSchema); } else if (_execType == EXECTYPE.PREPAREPENDING) @@ -2627,15 +2635,7 @@ private SqlDataReader RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavi decrementAsyncCountOnFailure = false; if (writeTask != null) { - task = AsyncHelper.CreateContinuationTask(writeTask, () => - { - _activeConnection.GetOpenTdsConnection(); // it will throw if connection is closed - cachedAsyncState.SetAsyncReaderState(ds, runBehavior, optionSettings); - }, - onFailure: (exc) => - { - _activeConnection.GetOpenTdsConnection().DecrementAsyncCount(); - }); + task = RunExecuteReaderTdsSetupContinuation(runBehavior, ds, optionSettings, writeTask); } else { @@ -2674,6 +2674,48 @@ private SqlDataReader RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavi return ds; } + // This is in its own method to avoid always allocating the lambda in RunExecuteReaderTds + private Task RunExecuteReaderTdsSetupContinuation(RunBehavior runBehavior, SqlDataReader ds, string optionSettings, Task writeTask) + { + Task task = AsyncHelper.CreateContinuationTask(writeTask, () => + { + _activeConnection.GetOpenTdsConnection(); // it will throw if connection is closed + cachedAsyncState.SetAsyncReaderState(ds, runBehavior, optionSettings); + }, + onFailure: (exc) => + { + _activeConnection.GetOpenTdsConnection().DecrementAsyncCount(); + }); + return task; + } + + // This is in its own method to avoid always allocating the lambda in RunExecuteReaderTds + private void RunExecuteReaderTdsSetupReconnectContinuation(CommandBehavior cmdBehavior, RunBehavior runBehavior, bool returnStream, bool async, int timeout, bool asyncWrite, SqlDataReader ds, Task reconnectTask, long reconnectionStart, TaskCompletionSource completion) + { + CancellationTokenSource timeoutCTS = new CancellationTokenSource(); + AsyncHelper.SetTimeoutException(completion, timeout, SQL.CR_ReconnectTimeout, timeoutCTS.Token); + AsyncHelper.ContinueTask(reconnectTask, completion, + () => + { + if (completion.Task.IsCompleted) + { + return; + } + Interlocked.CompareExchange(ref _reconnectionCompletionSource, null, completion); + timeoutCTS.Cancel(); + Task subTask; + RunExecuteReaderTds(cmdBehavior, runBehavior, returnStream, async, TdsParserStaticMethods.GetRemainingTimeout(timeout, reconnectionStart), out subTask, asyncWrite, ds); + if (subTask == null) + { + completion.SetResult(null); + } + else + { + AsyncHelper.ContinueTask(subTask, completion, () => completion.SetResult(null)); + } + }, connectionToAbort: _activeConnection + ); + } private SqlDataReader CompleteAsyncExecuteReader() { @@ -2863,16 +2905,16 @@ private void ValidateCommand(bool async, [CallerMemberName] string method = "") private void ValidateAsyncCommand() { - if (cachedAsyncState.PendingAsyncOperation) + if (_cachedAsyncState != null && _cachedAsyncState.PendingAsyncOperation) { // Enforce only one pending async execute at a time. - if (cachedAsyncState.IsActiveConnectionValid(_activeConnection)) + if (_cachedAsyncState.IsActiveConnectionValid(_activeConnection)) { throw SQL.PendingBeginXXXExists(); } else { _stateObj = null; // Session was re-claimed by session pool upon connection close. - cachedAsyncState.ResetAsyncState(); + _cachedAsyncState.ResetAsyncState(); } } } @@ -3371,7 +3413,7 @@ private void BuildRPC(bool inSchema, SqlParameterCollection parameters, ref _Sql private _SqlRPC BuildExecute(bool inSchema) { - Debug.Assert(_prepareHandle != -1, "Invalid call to sp_execute without a valid handle!"); + Debug.Assert((int)_prepareHandle != -1, "Invalid call to sp_execute without a valid handle!"); int j = 1; int count = CountSendableParameters(_parameters); @@ -3401,7 +3443,7 @@ private _SqlRPC BuildExecute(bool inSchema) private void BuildExecuteSql(CommandBehavior behavior, string commandText, SqlParameterCollection parameters, ref _SqlRPC rpc) { - Debug.Assert(_prepareHandle == -1, "This command has an existing handle, use sp_execute!"); + Debug.Assert((int)_prepareHandle == -1, "This command has an existing handle, use sp_execute!"); Debug.Assert(CommandType.Text == this.CommandType, "invalid use of sp_executesql for stored proc invocation!"); int j; SqlParameter sqlParam; diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlConnection.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlConnection.cs index 28676a079b91..80ffa5bb1e40 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlConnection.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlConnection.cs @@ -911,7 +911,8 @@ internal Task ValidateAndReconnect(Action beforeDisconnect, int timeout) catch (SqlException) { } - runningReconnect = Task.Run(() => ReconnectAsync(timeout)); + // use Task.Factory.StartNew with state overload instead of Task.Run to avoid anonymous closure context capture in method scope and avoid the unneeded allocation + runningReconnect = Task.Factory.StartNew(state => ReconnectAsync((int)state), timeout, CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default); // if current reconnect is not null, somebody already started reconnection task - some kind of race condition Debug.Assert(_currentReconnectionTask == null, "Duplicate reconnection tasks detected"); _currentReconnectionTask = runningReconnect; diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlUtil.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlUtil.cs index 56f49f253980..2882b3ea6d62 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlUtil.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlUtil.cs @@ -34,11 +34,30 @@ internal static Task CreateContinuationTask(Task task, Action onSuccess, SqlInte } } + internal static Task CreateContinuationTaskWithState(Task task, object state, Action onSuccess, Action onFailure = null) + { + if (task == null) + { + onSuccess(state); + return null; + } + else + { + var completion = new TaskCompletionSource(); + ContinueTaskWithState(task, completion, state, + onSuccess: (continueState) => { onSuccess(continueState); completion.SetResult(null); }, + onFailure: onFailure + ); + return completion.Task; + } + } + internal static Task CreateContinuationTask(Task task, Action onSuccess, T1 arg1, T2 arg2, SqlInternalConnectionTds connectionToDoom = null, Action onFailure = null) { return CreateContinuationTask(task, () => onSuccess(arg1, arg2), connectionToDoom, onFailure); } + internal static void ContinueTask(Task task, TaskCompletionSource completion, Action onSuccess, @@ -101,6 +120,61 @@ internal static void ContinueTask(Task task, ); } + // the same logic as ContinueTask but with an added state parameter to allow the caller to avoid the use of a closure + // the parameter allocation cannot be avoided here and using closure names is clearer than Tuple numbered properties + internal static void ContinueTaskWithState(Task task, + TaskCompletionSource completion, + object state, + Action onSuccess, + Action onFailure = null, + Action onCancellation = null, + Func exceptionConverter = null + ) + { + task.ContinueWith( + tsk => + { + if (tsk.Exception != null) + { + Exception exc = tsk.Exception.InnerException; + if (exceptionConverter != null) + { + exc = exceptionConverter(exc); + } + try + { + onFailure?.Invoke(exc, state); + } + finally + { + completion.TrySetException(exc); + } + } + else if (tsk.IsCanceled) + { + try + { + onCancellation?.Invoke(state); + } + finally + { + completion.TrySetCanceled(); + } + } + else + { + try + { + onSuccess(state); + } + catch (Exception e) + { + completion.SetException(e); + } + } + }, TaskScheduler.Default + ); + } internal static void WaitForCompletion(Task task, int timeout, Action onTimeout = null, bool rethrowExceptions = true) { diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs index e7e35d351c0e..14e997883644 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs @@ -136,7 +136,7 @@ internal sealed partial class TdsParser internal TdsParser(bool MARS, bool fAsynchronous) { _fMARS = MARS; // may change during Connect to pre Yukon servers - + _physicalStateObj = TdsParserStateObjectFactory.Singleton.CreateTdsParserStateObject(this); } @@ -388,7 +388,7 @@ internal void Connect(ServerInfo serverInfo, SqlInternalConnectionTds connHandle Debug.Assert(retCode == TdsEnums.SNI_SUCCESS, "Unexpected failure state upon calling SniGetConnectionId"); SendPreLoginHandshake(instanceName, encrypt); - status = ConsumePreLoginHandshake(encrypt, trustServerCert, integratedSecurity, out marsCapable, out _connHandler._fedAuthRequired); + status = ConsumePreLoginHandshake(encrypt, trustServerCert, integratedSecurity, out marsCapable, out _connHandler._fedAuthRequired); // Don't need to check for Sphinx failure, since we've already consumed // one pre-login packet and know we are connecting to Shiloh. @@ -424,7 +424,7 @@ internal void RemoveEncryption() // create a new packet encryption changes the internal packet size _physicalStateObj.ClearAllWritePackets(); - + } internal void EnableMars() @@ -434,7 +434,7 @@ internal void EnableMars() // Cache physical stateObj and connection. _pMarsPhysicalConObj = _physicalStateObj; - if(TdsParserStateObjectFactory.UseManagedSNI) _pMarsPhysicalConObj.IncrementPendingCallbacks(); + if (TdsParserStateObjectFactory.UseManagedSNI) _pMarsPhysicalConObj.IncrementPendingCallbacks(); uint info = 0; uint error = _pMarsPhysicalConObj.EnableMars(ref info); @@ -658,7 +658,7 @@ private void SendPreLoginHandshake(byte[] instanceName, bool encrypt) _physicalStateObj.WritePacket(TdsEnums.HARDFLUSH); } - private PreLoginHandshakeStatus ConsumePreLoginHandshake(bool encrypt, bool trustServerCert, bool integratedSecurity, out bool marsCapable, out bool fedAuthRequired ) + private PreLoginHandshakeStatus ConsumePreLoginHandshake(bool encrypt, bool trustServerCert, bool integratedSecurity, out bool marsCapable, out bool fedAuthRequired) { marsCapable = _fMARS; // Assign default value fedAuthRequired = false; @@ -802,7 +802,7 @@ private PreLoginHandshakeStatus ConsumePreLoginHandshake(bool encrypt, bool trus } WaitForSSLHandShakeToComplete(ref error); - + // create a new packet encryption changes the internal packet size _physicalStateObj.ClearAllWritePackets(); } @@ -1149,7 +1149,7 @@ internal SqlError ProcessSNIError(TdsParserStateObject stateObj) Debug.Assert(SniContext.Undefined != stateObj.DebugOnlyCopyOfSniContext || ((_fMARS) && ((_state == TdsParserState.Closed) || (_state == TdsParserState.Broken))), "SniContext must not be None"); #endif SNIErrorDetails details = GetSniErrorDetails(); - + if (details.sniErrorNumber != 0) { // handle special SNI error codes that are converted into exception which is not a SqlException. @@ -2135,7 +2135,7 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead { // Dev11 #344723: SqlClient stress hang System_Data!Tcp::ReadSync via a call to SqlDataReader::Close // Spin until SendAttention has cleared _attentionSending, this prevents a race condition between receiving the attention ACK and setting _attentionSent - SpinWait.SpinUntil(() => !stateObj._attentionSending); + TryRunSetupSpinWaitContinuation(stateObj); Debug.Assert(stateObj._attentionSent, "Attention ACK has been received without attention sent"); if (stateObj._attentionSent) @@ -2159,6 +2159,9 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead return true; } + // This is in its own method to avoid always allocating the lambda in TryRun + private static void TryRunSetupSpinWaitContinuation(TdsParserStateObject stateObj) => SpinWait.SpinUntil(() => !stateObj._attentionSending); + private bool TryProcessEnvChange(int tokenLength, TdsParserStateObject stateObj, out SqlEnvChange[] sqlEnvChange) { // There could be multiple environment change messages following this token. @@ -2281,7 +2284,7 @@ private bool TryProcessEnvChange(int tokenLength, TdsParserStateObject stateObj, { return false; } - + // Give the parser the new collation values in case parameters don't specify one _defaultCollation = env.newCollation; @@ -6217,7 +6220,7 @@ internal void TdsLogin(SqlLogin rec, TdsEnums.FeatureExtension requestedFeatures _physicalStateObj.SniContext = SniContext.Snix_LoginSspi; SSPIData(null, 0, ref outSSPIBuff, ref outSSPILength); - + if (outSSPILength > int.MaxValue) { throw SQL.InvalidSSPIPacketSize(); // SqlBu 332503 @@ -6541,7 +6544,7 @@ private void SNISSPIData(byte[] receivedBuff, uint receivedLength, ref byte[] se } } else - { + { if (receivedBuff == null) { // if we do not have SSPI data coming from server, send over 0's for pointer and length @@ -6972,29 +6975,34 @@ internal Task TdsExecuteSQLBatch(string text, int timeout, SqlNotificationReques // Need to wait for flush - continuation will unlock the connection bool taskReleaseConnectionLock = releaseConnectionLock; releaseConnectionLock = false; - return executeTask.ContinueWith(t => - { - Debug.Assert(!t.IsCanceled, "Task should not be canceled"); - try - { - if (t.IsFaulted) - { - FailureCleanup(stateObj, t.Exception.InnerException); - throw t.Exception.InnerException; - } - else + return executeTask.ContinueWith( + (task, state) => + { + Debug.Assert(!task.IsCanceled, "Task should not be canceled"); + var parameters = (Tuple)state; + TdsParser parser = parameters.Item1; + TdsParserStateObject tdsParserStateObject = parameters.Item2; + SqlInternalConnectionTds internalConnectionTds = parameters.Item3; + try { - stateObj.SniContext = SniContext.Snix_Read; + if (task.IsFaulted) + { + parser.FailureCleanup(tdsParserStateObject, task.Exception.InnerException); + throw task.Exception.InnerException; + } + else + { + tdsParserStateObject.SniContext = SniContext.Snix_Read; + } } - } - finally - { - if (taskReleaseConnectionLock) + finally { - _connHandler._parserLock.Release(); + internalConnectionTds?._parserLock.Release(); } - } - }, TaskScheduler.Default); + }, + Tuple.Create(this, stateObj, taskReleaseConnectionLock ? _connHandler : null), + TaskScheduler.Default + ); } // Finished sync @@ -7494,11 +7502,19 @@ internal Task TdsExecuteRPC(_SqlRPC[] rpcArray, int timeout, bool inSchema, SqlN task = completion.Task; } - AsyncHelper.ContinueTask(writeParamTask, completion, - () => TdsExecuteRPC(rpcArray, timeout, inSchema, notificationRequest, stateObj, isCommandProc, sync, completion, - startRpc: ii, startParam: i + 1), - connectionToDoom: _connHandler, - onFailure: exc => TdsExecuteRPC_OnFailure(exc, stateObj)); + TDSExecuteRPCParameterSetupWriteCompletion( + rpcArray, + timeout, + inSchema, + notificationRequest, + stateObj, + isCommandProc, + sync, + completion, + ii, + i+1, + writeParamTask + ); // Take care of releasing the locks if (releaseConnectionLock) @@ -7547,10 +7563,9 @@ internal Task TdsExecuteRPC(_SqlRPC[] rpcArray, int timeout, bool inSchema, SqlN task = completion.Task; } - bool taskReleaseConnectionLock = releaseConnectionLock; - execFlushTask.ContinueWith(tsk => ExecuteFlushTaskCallback(tsk, stateObj, completion, taskReleaseConnectionLock), TaskScheduler.Default); + TDSExecuteRPCParameterSetupFlushCompletion(stateObj, completion, execFlushTask, releaseConnectionLock); - // ExecuteFlushTaskCallback will take care of the locks for us + // TDSExecuteRPCParameterSetupFlushCompletion calling ExecuteFlushTaskCallback will take care of the locks for us releaseConnectionLock = false; return task; @@ -7597,6 +7612,36 @@ internal Task TdsExecuteRPC(_SqlRPC[] rpcArray, int timeout, bool inSchema, SqlN } } + + // This is in its own method to avoid always allocating the lambda in TDSExecuteRPCParameter + private void TDSExecuteRPCParameterSetupWriteCompletion(_SqlRPC[] rpcArray, int timeout, bool inSchema, SqlNotificationRequest notificationRequest, TdsParserStateObject stateObj, bool isCommandProc, bool sync, TaskCompletionSource completion, int startRpc, int startParam, Task writeParamTask) + { + AsyncHelper.ContinueTask( + writeParamTask, + completion, + () => TdsExecuteRPC( + rpcArray, + timeout, + inSchema, + notificationRequest, + stateObj, + isCommandProc, + sync, + completion, + startRpc, + startParam + ), + connectionToDoom: _connHandler, + onFailure: exc => TdsExecuteRPC_OnFailure(exc, stateObj) + ); + } + + // This is in its own method to avoid always allocating the lambda in TDSExecuteRPCParameter + private void TDSExecuteRPCParameterSetupFlushCompletion(TdsParserStateObject stateObj, TaskCompletionSource completion, Task execFlushTask, bool taskReleaseConnectionLock) + { + execFlushTask.ContinueWith(tsk => ExecuteFlushTaskCallback(tsk, stateObj, completion, taskReleaseConnectionLock), TaskScheduler.Default); + } + private void FinalizeExecuteRPC(TdsParserStateObject stateObj) { stateObj.SniContext = SniContext.Snix_Read; 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..a55e4b59ff10 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs @@ -2418,37 +2418,23 @@ internal bool IsConnectionAlive(bool throwOnException) else { uint error; + SniContext = SniContext.Snix_Connect; - object readPacket = EmptyReadPacket; - - try + error = CheckConnection(); + 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; } } } 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..e830fc074de6 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObjectNative.cs @@ -12,6 +12,8 @@ namespace System.Data.SqlClient { internal class TdsParserStateObjectNative : TdsParserStateObject { + private static readonly object s_cachedEmptyReadPacketObjectPointer = (object)IntPtr.Zero; + private SNIHandle _sessionHandle = null; // the SNI handle we're to work on private SNIPacket _sniPacket = null; // Will have to re-vamp this for MARS @@ -35,7 +37,7 @@ internal TdsParserStateObjectNative(TdsParser parser, TdsParserStateObject physi internal override object SessionHandle => _sessionHandle; - protected override object EmptyReadPacket => IntPtr.Zero; + protected override object EmptyReadPacket => s_cachedEmptyReadPacketObjectPointer; protected override void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async) {