From 1acb337ac383d3f7d4a30bd3e46ef771459baf03 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Mon, 17 Oct 2022 22:06:49 +0000 Subject: [PATCH] Backport SqlClient 659 to release/3.1: Fix async timeout design leading to intermittent wrong data Backport of https://github.com/dotnet/SqlClient/issues/659 to 3.1 Cherry-picks: - https://github.com/dotnet/SqlClient/commit/717ceda67a93e0bb5c0614045e597862807cf5af - https://github.com/dotnet/SqlClient/commit/2c2f100d8ab9a1f6ee8c048a260d77e6a738e1e7 - https://github.com/dotnet/SqlClient/commit/81055cf4b48f36a48199c7569e9308fc5be96b7c Resolves CVE-2022-41064. --- .../packageIndex.json | 13 +- .../Microsoft.Windows.Compatibility.pkgproj | 6 +- .../Directory.Build.props | 4 +- .../System.Data.SqlClient.sln | 16 +- .../src/System/Data/SqlClient/SqlBulkCopy.cs | 2 +- .../System/Data/SqlClient/SqlDataReader.cs | 2 +- .../src/System/Data/SqlClient/TdsParser.cs | 4 +- .../Data/SqlClient/TdsParserStateObject.cs | 160 +++++++++++--- .../SQL/AsyncTest/AsyncTimeoutTest.cs | 209 ++++++++++++++++++ .../SystemDataInternals/ConnectionHelper.cs | 27 ++- ....Data.SqlClient.ManualTesting.Tests.csproj | 1 + src/packages.builds | 6 + 12 files changed, 402 insertions(+), 48 deletions(-) create mode 100644 src/System.Data.SqlClient/tests/ManualTests/SQL/AsyncTest/AsyncTimeoutTest.cs diff --git a/pkg/Microsoft.Private.PackageBaseline/packageIndex.json b/pkg/Microsoft.Private.PackageBaseline/packageIndex.json index b073e1795ce3..890b50e78634 100644 --- a/pkg/Microsoft.Private.PackageBaseline/packageIndex.json +++ b/pkg/Microsoft.Private.PackageBaseline/packageIndex.json @@ -431,9 +431,10 @@ "3.1.0", "3.1.1", "3.1.2", - "3.1.3" + "3.1.3", + "3.1.4" ], - "BaselineVersion": "3.1.3", + "BaselineVersion": "3.1.4", "InboxOn": {} }, "Microsoft.Windows.Compatibility.Shims": { @@ -1553,9 +1554,10 @@ "4.8.1", "4.8.2", "4.8.3", - "4.8.4" + "4.8.4", + "4.8.5" ], - "BaselineVersion": "4.8.4", + "BaselineVersion": "4.8.5", "InboxOn": { "net461": "4.1.0.0", "monoandroid10": "Any", @@ -1580,7 +1582,8 @@ "4.6.1.1": "4.8.1", "4.6.1.2": "4.8.2", "4.6.1.3": "4.8.3", - "4.6.1.4": "4.8.4" + "4.6.1.4": "4.8.4", + "4.6.1.5": "4.8.5" } }, "System.Data.SqlXml": { diff --git a/pkg/Microsoft.Windows.Compatibility/Microsoft.Windows.Compatibility.pkgproj b/pkg/Microsoft.Windows.Compatibility/Microsoft.Windows.Compatibility.pkgproj index bccc0185cee3..b0c0a93aa5b2 100644 --- a/pkg/Microsoft.Windows.Compatibility/Microsoft.Windows.Compatibility.pkgproj +++ b/pkg/Microsoft.Windows.Compatibility/Microsoft.Windows.Compatibility.pkgproj @@ -3,8 +3,8 @@ <_PreReleasePackageVersion>$(PackageVersion) - 3.1.3 - 4.7.1 + 3.1.4 + 4.7.0 false @@ -30,7 +30,7 @@ - 4.8.4 + 4.8.5 diff --git a/src/System.Data.SqlClient/Directory.Build.props b/src/System.Data.SqlClient/Directory.Build.props index b49fb7c6fa3b..83512e18f142 100644 --- a/src/System.Data.SqlClient/Directory.Build.props +++ b/src/System.Data.SqlClient/Directory.Build.props @@ -2,8 +2,8 @@ - 4.8.4 - 4.6.1.4 + 4.8.5 + 4.6.1.5 4.6.1.0 diff --git a/src/System.Data.SqlClient/System.Data.SqlClient.sln b/src/System.Data.SqlClient/System.Data.SqlClient.sln index 42e3df818cde..6dfcf8dbec13 100644 --- a/src/System.Data.SqlClient/System.Data.SqlClient.sln +++ b/src/System.Data.SqlClient/System.Data.SqlClient.sln @@ -90,20 +90,20 @@ Global {F3E72F35-0351-4D67-9388-725BCAD807BA}.Debug|Any CPU.Build.0 = netcoreapp-Windows_NT-Debug|Any CPU {F3E72F35-0351-4D67-9388-725BCAD807BA}.Release|Any CPU.ActiveCfg = netcoreapp-Windows_NT-Release|Any CPU {F3E72F35-0351-4D67-9388-725BCAD807BA}.Release|Any CPU.Build.0 = netcoreapp-Windows_NT-Release|Any CPU - {D1392B54-998A-4F27-BC17-4CE149117BCC}.Debug|Any CPU.ActiveCfg = netstandard-Debug|Any CPU - {D1392B54-998A-4F27-BC17-4CE149117BCC}.Debug|Any CPU.Build.0 = netstandard-Debug|Any CPU + {D1392B54-998A-4F27-BC17-4CE149117BCC}.Debug|Any CPU.ActiveCfg = netcoreapp-Debug|Any CPU + {D1392B54-998A-4F27-BC17-4CE149117BCC}.Debug|Any CPU.Build.0 = netcoreapp-Debug|Any CPU {D1392B54-998A-4F27-BC17-4CE149117BCC}.Release|Any CPU.ActiveCfg = netstandard-Release|Any CPU {D1392B54-998A-4F27-BC17-4CE149117BCC}.Release|Any CPU.Build.0 = netstandard-Release|Any CPU - {6C88F00F-9597-43AD-9E5F-9B344DA3B16F}.Debug|Any CPU.ActiveCfg = netstandard-Debug|Any CPU - {6C88F00F-9597-43AD-9E5F-9B344DA3B16F}.Debug|Any CPU.Build.0 = netstandard-Debug|Any CPU + {6C88F00F-9597-43AD-9E5F-9B344DA3B16F}.Debug|Any CPU.ActiveCfg = netcoreapp-Debug|Any CPU + {6C88F00F-9597-43AD-9E5F-9B344DA3B16F}.Debug|Any CPU.Build.0 = netcoreapp-Debug|Any CPU {6C88F00F-9597-43AD-9E5F-9B344DA3B16F}.Release|Any CPU.ActiveCfg = netstandard-Release|Any CPU {6C88F00F-9597-43AD-9E5F-9B344DA3B16F}.Release|Any CPU.Build.0 = netstandard-Release|Any CPU - {B73A7063-37C3-415D-AD53-BB3DA20ABD6E}.Debug|Any CPU.ActiveCfg = netstandard-Debug|Any CPU - {B73A7063-37C3-415D-AD53-BB3DA20ABD6E}.Debug|Any CPU.Build.0 = netstandard-Debug|Any CPU + {B73A7063-37C3-415D-AD53-BB3DA20ABD6E}.Debug|Any CPU.ActiveCfg = netcoreapp-Debug|Any CPU + {B73A7063-37C3-415D-AD53-BB3DA20ABD6E}.Debug|Any CPU.Build.0 = netcoreapp-Debug|Any CPU {B73A7063-37C3-415D-AD53-BB3DA20ABD6E}.Release|Any CPU.ActiveCfg = netstandard-Release|Any CPU {B73A7063-37C3-415D-AD53-BB3DA20ABD6E}.Release|Any CPU.Build.0 = netstandard-Release|Any CPU - {E0A6BB21-574B-43D9-890D-6E1144F2EE9E}.Debug|Any CPU.ActiveCfg = netstandard-Debug|Any CPU - {E0A6BB21-574B-43D9-890D-6E1144F2EE9E}.Debug|Any CPU.Build.0 = netstandard-Debug|Any CPU + {E0A6BB21-574B-43D9-890D-6E1144F2EE9E}.Debug|Any CPU.ActiveCfg = netcoreapp-Debug|Any CPU + {E0A6BB21-574B-43D9-890D-6E1144F2EE9E}.Debug|Any CPU.Build.0 = netcoreapp-Debug|Any CPU {E0A6BB21-574B-43D9-890D-6E1144F2EE9E}.Release|Any CPU.ActiveCfg = netstandard-Release|Any CPU {E0A6BB21-574B-43D9-890D-6E1144F2EE9E}.Release|Any CPU.Build.0 = netstandard-Release|Any CPU {45DB5F86-7AE3-45C6-870D-F9357B66BDB5}.Debug|Any CPU.ActiveCfg = netcoreapp-Debug|Any CPU diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlBulkCopy.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlBulkCopy.cs index ac253e827003..864e4412eaff 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlBulkCopy.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlBulkCopy.cs @@ -2598,7 +2598,7 @@ private void CleanUpStateObject(bool isCancelRequested = true) { _stateObj.CancelRequest(); } - _stateObj._internalTimeout = false; + _stateObj.SetTimeoutStateStopped(); _stateObj.CloseSession(); _stateObj._bulkCopyOpperationInProgress = false; _stateObj._bulkCopyWriteTimeout = false; diff --git a/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlDataReader.cs b/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlDataReader.cs index 3e145187aff8..ac40cbf3a0dd 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlDataReader.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlDataReader.cs @@ -892,7 +892,7 @@ private bool TryCloseInternal(bool closeReader) { _sharedState._dataReady = true; // set _sharedState._dataReady to not confuse CleanPartialRead } - _stateObj._internalTimeout = false; + _stateObj.SetTimeoutStateStopped(); if (_sharedState._dataReady) { cleanDataFailed = 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 b98eb83f4c3a..2345bb80ff80 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParser.cs @@ -1590,7 +1590,7 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead // If there is data ready, but we didn't exit the loop, then something is wrong Debug.Assert(!dataReady, "dataReady not expected - did we forget to skip the row?"); - if (stateObj._internalTimeout) + if (stateObj.IsTimeoutStateExpired) { runBehavior = RunBehavior.Attention; } @@ -2157,7 +2157,7 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead stateObj._attentionSent = false; stateObj._attentionReceived = false; - if (RunBehavior.Clean != (RunBehavior.Clean & runBehavior) && !stateObj._internalTimeout) + if (RunBehavior.Clean != (RunBehavior.Clean & runBehavior) && !stateObj.IsTimeoutStateExpired) { // Add attention error to collection - if not RunBehavior.Clean! stateObj.AddError(new SqlError(0, 0, TdsEnums.MIN_ERROR_CLASS, _server, SQLMessage.OperationCancelled(), "", 0)); 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 c1d32cd35612..9d3fd097023d 100644 --- a/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs +++ b/src/System.Data.SqlClient/src/System/Data/SqlClient/TdsParserStateObject.cs @@ -22,6 +22,23 @@ sealed internal class LastIOTimer internal abstract class TdsParserStateObject { + private sealed class TimeoutState + { + public const int Stopped = 0; + public const int Running = 1; + public const int ExpiredAsync = 2; + public const int ExpiredSync = 3; + + private readonly int _value; + + public TimeoutState(int value) + { + _value = value; + } + + public int IdentityValue => _value; + } + private const int AttentionTimeoutSeconds = 5; // Ticks to consider a connection "good" after a successful I/O (10,000 ticks = 1 ms) @@ -85,10 +102,18 @@ internal abstract class TdsParserStateObject // Timeout variables private long _timeoutMilliseconds; private long _timeoutTime; // variable used for timeout computations, holds the value of the hi-res performance counter at which this request should expire + private int _timeoutState; // expected to be one of the constant values TimeoutStopped, TimeoutRunning, TimeoutExpiredAsync, TimeoutExpiredSync + private int _timeoutIdentitySource; + private volatile int _timeoutIdentityValue; internal volatile bool _attentionSent = false; // true if we sent an Attention to the server internal bool _attentionReceived = false; // NOTE: Received is not volatile as it is only ever accessed\modified by TryRun its callees (i.e. single threaded access) internal volatile bool _attentionSending = false; - internal bool _internalTimeout = false; // an internal timeout occurred + + // Below 2 properties are used to enforce timeout delays in code to + // reproduce issues related to theadpool starvation and timeout delay. + // It should always be set to false by default, and only be enabled during testing. + internal bool _enforceTimeoutDelay = false; + internal int _enforcedTimeoutDelayInMilliSeconds = 5000; private readonly LastIOTimer _lastSuccessfulIOTimer; @@ -739,7 +764,7 @@ private void ResetCancelAndProcessAttention() // operations. Parser.ProcessPendingAck(this); } - _internalTimeout = false; + SetTimeoutStateStopped(); } } @@ -1017,7 +1042,7 @@ internal bool TryProcessHeader() return false; } - if (_internalTimeout) + if (IsTimeoutStateExpired) { ThrowExceptionAndWarning(); return true; @@ -2160,11 +2185,63 @@ internal void OnConnectionClosed() } } - private void OnTimeout(object state) + public void SetTimeoutStateStopped() + { + Interlocked.Exchange(ref _timeoutState, TimeoutState.Stopped); + _timeoutIdentityValue = 0; + } + + public bool IsTimeoutStateExpired + { + get + { + int state = _timeoutState; + return state == TimeoutState.ExpiredAsync || state == TimeoutState.ExpiredSync; + } + } + + private void OnTimeoutAsync(object state) + { + if (_enforceTimeoutDelay) + { + Thread.Sleep(_enforcedTimeoutDelayInMilliSeconds); + } + + int currentIdentityValue = _timeoutIdentityValue; + TimeoutState timeoutState = (TimeoutState)state; + if (timeoutState.IdentityValue == _timeoutIdentityValue) + { + // the return value is not useful here because no choice is going to be made using it + // we only want to make this call to set the state knowing that it will be seen later + OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredAsync); + } + else + { + Debug.WriteLine($"OnTimeoutAsync called with identity state={timeoutState.IdentityValue} but current identity is {currentIdentityValue} so it is being ignored"); + } + } + + private bool OnTimeoutSync(bool asyncClose = false) + { + return OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredSync, asyncClose); + } + + /// + /// attempts to change the timout state from the expected state to the target state and if it succeeds + /// will setup the the stateobject into the timeout expired state + /// + /// the state that is the expected current state, state will change only if this is correct + /// the state that will be changed to if the expected state is correct + /// any close action to be taken by an async task to avoid deadlock. + /// boolean value indicating whether the call changed the timeout state + private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose = false) { - if (!_internalTimeout) + Debug.Assert(targetState == TimeoutState.ExpiredAsync || targetState == TimeoutState.ExpiredSync, "OnTimeoutCore must have an expiry state as the targetState"); + + bool retval = false; + if (Interlocked.CompareExchange(ref _timeoutState, targetState, expectedState) == expectedState) { - _internalTimeout = true; + retval = true; // lock protects against Close and Cancel lock (this) { @@ -2190,7 +2267,7 @@ private void OnTimeout(object state) { try { - SendAttention(mustTakeWriteLock: true); + SendAttention(mustTakeWriteLock: true, asyncClose); } catch (Exception e) { @@ -2262,11 +2339,11 @@ private void OnTimeout(object state) } } } + return retval; } internal void ReadSni(TaskCompletionSource completion) { - Debug.Assert(_networkPacketTaskSource == null || ((_asyncReadWithoutSnapshot) && (_networkPacketTaskSource.Task.IsCompleted)), "Pending async call or failed to replay snapshot when calling ReadSni"); _networkPacketTaskSource = completion; @@ -2296,19 +2373,31 @@ internal void ReadSni(TaskCompletionSource completion) { Debug.Assert(completion != null, "Async on but null asyncResult passed"); - if (_networkPacketTimeout == null) + // if the state is currently stopped then change it to running and allocate a new identity value from + // the identity source. The identity value is used to correlate timer callback events to the currently + // running timeout and prevents a late timer callback affecting a result it does not relate to + int previousTimeoutState = Interlocked.CompareExchange(ref _timeoutState, TimeoutState.Running, TimeoutState.Stopped); + Debug.Assert(previousTimeoutState == TimeoutState.Stopped, "previous timeout state was not Stopped"); + if (previousTimeoutState == TimeoutState.Stopped) { - _networkPacketTimeout = ADP.UnsafeCreateTimer( - new TimerCallback(OnTimeout), - null, - Timeout.Infinite, - Timeout.Infinite); + Debug.Assert(_timeoutIdentityValue == 0, "timer was previously stopped without resetting the _identityValue"); + _timeoutIdentityValue = Interlocked.Increment(ref _timeoutIdentitySource); } + _networkPacketTimeout?.Dispose(); + + _networkPacketTimeout = ADP.UnsafeCreateTimer( + new TimerCallback(OnTimeoutAsync), + new TimeoutState(_timeoutIdentityValue), + Timeout.Infinite, + Timeout.Infinite + ); + // -1 == Infinite // 0 == Already timed out (NOTE: To simulate the same behavior as sync we will only timeout on 0 if we receive an IO Pending from SNI) // >0 == Actual timeout remaining int msecsRemaining = GetTimeoutRemaining(); + if (msecsRemaining > 0) { ChangeNetworkPacketTimeout(msecsRemaining, Timeout.Infinite); @@ -2358,12 +2447,15 @@ internal void ReadSni(TaskCompletionSource completion) _networkPacketTaskSource.TrySetResult(null); } // Disable timeout timer on error + SetTimeoutStateStopped(); ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); } else if (msecsRemaining == 0) - { // Got IO Pending, but we have no time left to wait - // Immediately schedule the timeout timer to fire - ChangeNetworkPacketTimeout(0, Timeout.Infinite); + { + // Got IO Pending, but we have no time left to wait + // disable the timer and set the error state by calling OnTimeoutSync + ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); + OnTimeoutSync(); } // DO NOT HANDLE PENDING READ HERE - which is TdsEnums.SNI_SUCCESS_IO_PENDING state. // That is handled by user who initiated async read, or by ReadNetworkPacket which is sync over async. @@ -2477,13 +2569,13 @@ private void ReadSniError(TdsParserStateObject stateObj, uint error) Debug.Assert(_syncOverAsync, "Should never reach here with async on!"); bool fail = false; - if (_internalTimeout) + if (IsTimeoutStateExpired) { // This is now our second timeout - time to give up. fail = true; } else { - stateObj._internalTimeout = true; + stateObj.SetTimeoutStateStopped(); Debug.Assert(_parser.Connection != null, "SqlConnectionInternalTds handler can not be null at this point."); AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, (byte)0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); @@ -2707,6 +2799,24 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) } 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); } @@ -3203,7 +3313,7 @@ private void CancelWritePacket() } } - private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock) + private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock, bool asyncClose = false) { // Check for a stored exception var delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); @@ -3295,7 +3405,7 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu if (error != TdsEnums.SNI_SUCCESS) { AddError(_parser.ProcessSNIError(this)); - ThrowExceptionAndWarning(); + ThrowExceptionAndWarning(false, asyncClose); } AssertValidState(); completion.SetResult(null); @@ -3329,7 +3439,7 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu else { AddError(_parser.ProcessSNIError(this)); - ThrowExceptionAndWarning(callerHasConnectionLock); + ThrowExceptionAndWarning(callerHasConnectionLock, asyncClose); } AssertValidState(); } @@ -3341,7 +3451,7 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu internal abstract uint WritePacket(PacketHandle packet, bool sync); // Sends an attention signal - executing thread will consume attn. - internal void SendAttention(bool mustTakeWriteLock = false) + internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = false) { if (!_attentionSent) { @@ -3384,7 +3494,7 @@ internal void SendAttention(bool mustTakeWriteLock = false) uint sniError; _parser._asyncWrite = false; // stop async write - SNIWritePacket(attnPacket, out sniError, canAccumulate: false, callerHasConnectionLock: false); + SNIWritePacket(attnPacket, out sniError, canAccumulate: false, callerHasConnectionLock: false, asyncClose); } finally { @@ -3730,7 +3840,7 @@ internal void AssertStateIsClean() // Attention\Cancellation\Timeouts Debug.Assert(!_attentionReceived && !_attentionSent && !_attentionSending, $"StateObj is still dealing with attention: Sent: {_attentionSent}, Received: {_attentionReceived}, Sending: {_attentionSending}"); Debug.Assert(!_cancelled, "StateObj still has cancellation set"); - Debug.Assert(!_internalTimeout, "StateObj still has internal timeout set"); + Debug.Assert(_timeoutState == TimeoutState.Stopped, "StateObj still has internal timeout set"); // Errors and Warnings Debug.Assert(!_hasErrorOrWarning, "StateObj still has stored errors or warnings"); } diff --git a/src/System.Data.SqlClient/tests/ManualTests/SQL/AsyncTest/AsyncTimeoutTest.cs b/src/System.Data.SqlClient/tests/ManualTests/SQL/AsyncTest/AsyncTimeoutTest.cs new file mode 100644 index 000000000000..7740f787766d --- /dev/null +++ b/src/System.Data.SqlClient/tests/ManualTests/SQL/AsyncTest/AsyncTimeoutTest.cs @@ -0,0 +1,209 @@ +// 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. + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Data; +using System.Threading.Tasks; +using System.Xml; +using System.Data.SqlClient.ManualTesting.Tests.SystemDataInternals; +using Xunit; + +namespace System.Data.SqlClient.ManualTesting.Tests +{ + public static class AsyncTimeoutTest + { + static string delayQuery2s = "WAITFOR DELAY '00:00:02'"; + static string delayQuery10s = "WAITFOR DELAY '00:00:10'"; + + public enum AsyncAPI + { + ExecuteReaderAsync, + ExecuteScalarAsync, + ExecuteXmlReaderAsync + } + + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + [ClassData(typeof(AsyncTimeoutTestVariations))] + public static void TestDelayedAsyncTimeout(AsyncAPI api, string commonObj, int delayPeriod, bool marsEnabled) => + RunTest(api, commonObj, delayPeriod, marsEnabled); + + public class AsyncTimeoutTestVariations : IEnumerable + { + public IEnumerator GetEnumerator() + { + yield return new object[] { AsyncAPI.ExecuteReaderAsync, "Connection", 8000, true }; + yield return new object[] { AsyncAPI.ExecuteReaderAsync, "Connection", 5000, true }; + yield return new object[] { AsyncAPI.ExecuteReaderAsync, "Connection", 0, true }; + yield return new object[] { AsyncAPI.ExecuteReaderAsync, "Connection", 8000, false }; + yield return new object[] { AsyncAPI.ExecuteReaderAsync, "Connection", 5000, false }; + yield return new object[] { AsyncAPI.ExecuteReaderAsync, "Connection", 0, false }; + + yield return new object[] { AsyncAPI.ExecuteScalarAsync, "Connection", 8000, true }; + yield return new object[] { AsyncAPI.ExecuteScalarAsync, "Connection", 5000, true }; + yield return new object[] { AsyncAPI.ExecuteScalarAsync, "Connection", 0, true }; + yield return new object[] { AsyncAPI.ExecuteScalarAsync, "Connection", 8000, false }; + yield return new object[] { AsyncAPI.ExecuteScalarAsync, "Connection", 5000, false }; + yield return new object[] { AsyncAPI.ExecuteScalarAsync, "Connection", 0, false }; + + yield return new object[] { AsyncAPI.ExecuteXmlReaderAsync, "Connection", 8000, true }; + yield return new object[] { AsyncAPI.ExecuteXmlReaderAsync, "Connection", 5000, true }; + yield return new object[] { AsyncAPI.ExecuteXmlReaderAsync, "Connection", 0, true }; + yield return new object[] { AsyncAPI.ExecuteXmlReaderAsync, "Connection", 8000, false }; + yield return new object[] { AsyncAPI.ExecuteXmlReaderAsync, "Connection", 5000, false }; + yield return new object[] { AsyncAPI.ExecuteXmlReaderAsync, "Connection", 0, false }; + + yield return new object[] { AsyncAPI.ExecuteReaderAsync, "Command", 8000, true }; + yield return new object[] { AsyncAPI.ExecuteReaderAsync, "Command", 5000, true }; + yield return new object[] { AsyncAPI.ExecuteReaderAsync, "Command", 0, true }; + yield return new object[] { AsyncAPI.ExecuteReaderAsync, "Command", 8000, false }; + yield return new object[] { AsyncAPI.ExecuteReaderAsync, "Command", 5000, false }; + yield return new object[] { AsyncAPI.ExecuteReaderAsync, "Command", 0, false }; + + yield return new object[] { AsyncAPI.ExecuteScalarAsync, "Command", 8000, true }; + yield return new object[] { AsyncAPI.ExecuteScalarAsync, "Command", 5000, true }; + yield return new object[] { AsyncAPI.ExecuteScalarAsync, "Command", 0, true }; + yield return new object[] { AsyncAPI.ExecuteScalarAsync, "Command", 8000, false }; + yield return new object[] { AsyncAPI.ExecuteScalarAsync, "Command", 5000, false }; + yield return new object[] { AsyncAPI.ExecuteScalarAsync, "Command", 0, false }; + + yield return new object[] { AsyncAPI.ExecuteXmlReaderAsync, "Command", 8000, true }; + yield return new object[] { AsyncAPI.ExecuteXmlReaderAsync, "Command", 5000, true }; + yield return new object[] { AsyncAPI.ExecuteXmlReaderAsync, "Command", 0, true }; + yield return new object[] { AsyncAPI.ExecuteXmlReaderAsync, "Command", 8000, false }; + yield return new object[] { AsyncAPI.ExecuteXmlReaderAsync, "Command", 5000, false }; + yield return new object[] { AsyncAPI.ExecuteXmlReaderAsync, "Command", 0, false }; + } + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + private static void RunTest(AsyncAPI api, string commonObj, int timeoutDelay, bool marsEnabled) + { + string connString = new SqlConnectionStringBuilder(DataTestUtility.TcpConnStr) + { + MultipleActiveResultSets = marsEnabled + }.ConnectionString; + + using (SqlConnection sqlConnection = new SqlConnection(connString)) + { + sqlConnection.Open(); + if (timeoutDelay != 0) + { + ConnectionHelper.SetEnforcedTimeout(sqlConnection, true, timeoutDelay); + } + switch (commonObj) + { + case "Connection": + QueryAndValidate(api, 1, delayQuery2s, 1, true, true, sqlConnection).Wait(); + QueryAndValidate(api, 2, delayQuery2s, 5, false, true, sqlConnection).Wait(); + QueryAndValidate(api, 3, delayQuery10s, 1, true, true, sqlConnection).Wait(); + QueryAndValidate(api, 4, delayQuery2s, 10, false, true, sqlConnection).Wait(); + break; + case "Command": + using (SqlCommand cmd = sqlConnection.CreateCommand()) + { + QueryAndValidate(api, 1, delayQuery2s, 1, true, false, sqlConnection, cmd).Wait(); + QueryAndValidate(api, 2, delayQuery2s, 5, false, false, sqlConnection, cmd).Wait(); + QueryAndValidate(api, 3, delayQuery10s, 1, true, false, sqlConnection, cmd).Wait(); + QueryAndValidate(api, 4, delayQuery2s, 10, false, false, sqlConnection, cmd).Wait(); + } + break; + } + } + } + + private static async Task QueryAndValidate(AsyncAPI api, int index, string delayQuery, int timeout, + bool timeoutExExpected = false, bool useTransaction = false, SqlConnection cn = null, SqlCommand cmd = null) + { + SqlTransaction tx = null; + try + { + if (cn != null) + { + if (cn.State != ConnectionState.Open) + { + await cn.OpenAsync(); + } + cmd = cn.CreateCommand(); + if (useTransaction) + { + tx = cn.BeginTransaction(IsolationLevel.ReadCommitted); + cmd.Transaction = tx; + } + } + + cmd.CommandTimeout = timeout; + if (api != AsyncAPI.ExecuteXmlReaderAsync) + { + cmd.CommandText = delayQuery + $";select {index} as Id;"; + } + else + { + cmd.CommandText = delayQuery + $";select {index} as Id FOR XML PATH;"; + } + + var result = -1; + switch (api) + { + case AsyncAPI.ExecuteReaderAsync: + using (SqlDataReader reader = await cmd.ExecuteReaderAsync().ConfigureAwait(false)) + { + while (await reader.ReadAsync().ConfigureAwait(false)) + { + var columnIndex = reader.GetOrdinal("Id"); + result = reader.GetInt32(columnIndex); + break; + } + } + break; + case AsyncAPI.ExecuteScalarAsync: + result = (int)await cmd.ExecuteScalarAsync().ConfigureAwait(false); + break; + case AsyncAPI.ExecuteXmlReaderAsync: + using (XmlReader reader = await cmd.ExecuteXmlReaderAsync().ConfigureAwait(false)) + { + try + { + Assert.True(reader.Settings.Async); + reader.ReadToDescendant("Id"); + result = reader.ReadElementContentAsInt(); + } + catch (Exception ex) + { + Assert.False(true, "Exception occurred: " + ex.Message); + } + } + break; + } + + if (result != index) + { + throw new Exception("High Alert! Wrong data received for index: " + index); + } + else + { + Assert.True(!timeoutExExpected && result == index); + } + } + catch (SqlException e) + { + if (!timeoutExExpected) + throw new Exception("Index " + index + " failed with: " + e.Message); + else + Assert.True(timeoutExExpected && e.Class == 11 && e.Number == -2); + } + finally + { + if (cn != null) + { + if (useTransaction) + tx.Commit(); + cn.Close(); + } + } + } + } +} diff --git a/src/System.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionHelper.cs b/src/System.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionHelper.cs index 28399f6d4a97..2e7e81eff5f4 100644 --- a/src/System.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionHelper.cs +++ b/src/System.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/ConnectionHelper.cs @@ -14,10 +14,17 @@ internal static class ConnectionHelper private static Type s_sqlInternalConnection = s_systemDotData.GetType("System.Data.SqlClient.SqlInternalConnection"); private static Type s_sqlInternalConnectionTds = s_systemDotData.GetType("System.Data.SqlClient.SqlInternalConnectionTds"); private static Type s_dbConnectionInternal = s_systemDotData.GetType("System.Data.ProviderBase.DbConnectionInternal"); + private static Type s_tdsParser = s_systemDotData.GetType("System.Data.SqlClient.TdsParser"); + private static Type s_tdsParserStateObject = s_systemDotData.GetType("System.Data.SqlClient.TdsParserStateObject"); private static PropertyInfo s_sqlConnectionInternalConnection = s_sqlConnection.GetProperty("InnerConnection", BindingFlags.Instance | BindingFlags.NonPublic); private static PropertyInfo s_dbConnectionInternalPool = s_dbConnectionInternal.GetProperty("Pool", BindingFlags.Instance | BindingFlags.NonPublic); private static MethodInfo s_dbConnectionInternalIsConnectionAlive = s_dbConnectionInternal.GetMethod("IsConnectionAlive", BindingFlags.Instance | BindingFlags.NonPublic); private static FieldInfo s_sqlInternalConnectionTdsParser = s_sqlInternalConnectionTds.GetField("_parser", BindingFlags.Instance | BindingFlags.NonPublic); + private static PropertyInfo s_innerConnectionProperty = s_sqlConnection.GetProperty("InnerConnection", BindingFlags.Instance | BindingFlags.NonPublic); + private static PropertyInfo s_tdsParserProperty = s_sqlInternalConnectionTds.GetProperty("Parser", BindingFlags.Instance | BindingFlags.NonPublic); + private static FieldInfo s_tdsParserStateObjectProperty = s_tdsParser.GetField("_physicalStateObj", BindingFlags.Instance | BindingFlags.NonPublic); + private static FieldInfo s_enforceTimeoutDelayProperty = s_tdsParserStateObject.GetField("_enforceTimeoutDelay", BindingFlags.Instance | BindingFlags.NonPublic); + private static FieldInfo s_enforcedTimeoutDelayInMilliSeconds = s_tdsParserStateObject.GetField("_enforcedTimeoutDelayInMilliSeconds", BindingFlags.Instance | BindingFlags.NonPublic); public static object GetConnectionPool(object internalConnection) { @@ -27,6 +34,7 @@ public static object GetConnectionPool(object internalConnection) public static object GetInternalConnection(this SqlConnection connection) { + VerifyObjectIsConnection(connection); object internalConnection = s_sqlConnectionInternalConnection.GetValue(connection, null); Debug.Assert(((internalConnection != null) && (s_dbConnectionInternal.IsInstanceOfType(internalConnection))), "Connection provided has an invalid internal connection"); return internalConnection; @@ -44,7 +52,14 @@ private static void VerifyObjectIsInternalConnection(object internalConnection) if (internalConnection == null) throw new ArgumentNullException(nameof(internalConnection)); if (!s_dbConnectionInternal.IsInstanceOfType(internalConnection)) - throw new ArgumentException("Object provided was not a DbConnectionInternal", "internalConnection"); + throw new ArgumentException("Object provided was not a DbConnectionInternal", nameof(internalConnection)); + } + private static void VerifyObjectIsConnection(object connection) + { + if (connection == null) + throw new ArgumentNullException(nameof(connection)); + if (!s_sqlConnection.IsInstanceOfType(connection)) + throw new ArgumentException("Object provided was not a SqlConnection", nameof(connection)); } public static object GetParser(object internalConnection) @@ -52,5 +67,15 @@ public static object GetParser(object internalConnection) VerifyObjectIsInternalConnection(internalConnection); return s_sqlInternalConnectionTdsParser.GetValue(internalConnection); } + public static void SetEnforcedTimeout(this SqlConnection connection, bool enforce, int timeout) + { + VerifyObjectIsConnection(connection); + var stateObj = s_tdsParserStateObjectProperty.GetValue( + s_tdsParserProperty.GetValue( + s_innerConnectionProperty.GetValue( + connection, null), null)); + s_enforceTimeoutDelayProperty.SetValue(stateObj, enforce); + s_enforcedTimeoutDelayInMilliSeconds.SetValue(stateObj, timeout); + } } } diff --git a/src/System.Data.SqlClient/tests/ManualTests/System.Data.SqlClient.ManualTesting.Tests.csproj b/src/System.Data.SqlClient/tests/ManualTests/System.Data.SqlClient.ManualTesting.Tests.csproj index c2e36b81121b..188a9537a26a 100644 --- a/src/System.Data.SqlClient/tests/ManualTests/System.Data.SqlClient.ManualTesting.Tests.csproj +++ b/src/System.Data.SqlClient/tests/ManualTests/System.Data.SqlClient.ManualTesting.Tests.csproj @@ -9,6 +9,7 @@ + diff --git a/src/packages.builds b/src/packages.builds index 4e5cb8491b59..7436cfa4a99e 100644 --- a/src/packages.builds +++ b/src/packages.builds @@ -31,6 +31,12 @@ $(AdditionalProperties) + + $(AdditionalProperties) + + + $(AdditionalProperties) +