From 22c25f586e1c116bcdeaff60d0aab1d584dbba5d Mon Sep 17 00:00:00 2001 From: panoskj Date: Mon, 21 Feb 2022 11:03:17 +0200 Subject: [PATCH 01/16] Merging common fields, properties and types of TdsParserStateObject. --- .../src/Microsoft.Data.SqlClient.csproj | 3 + .../Data/SqlClient/TdsParserStateObject.cs | 444 +---------------- .../netfx/src/Microsoft.Data.SqlClient.csproj | 3 + .../Data/SqlClient/TdsParserStateObject.cs | 415 +--------------- .../Data/SqlClient/TdsParserStateObject.cs | 454 ++++++++++++++++++ 5 files changed, 465 insertions(+), 854 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj index e1483a26a8..f31370d49a 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj @@ -352,6 +352,9 @@ Microsoft\Data\SqlClient\SQLFallbackDNSCache.cs + + Microsoft\Data\SqlClient\TdsParserStateObject.cs + Microsoft\Data\OperationAbortedException.cs 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 7a095d0933..e89f0fe1c1 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 @@ -15,275 +15,19 @@ namespace Microsoft.Data.SqlClient { - sealed internal class LastIOTimer + internal abstract partial class TdsParserStateObject { - internal long _value; - } - - internal abstract class TdsParserStateObject - { - private static int _objectTypeCount; // EventSource counter - internal readonly int _objectID = Interlocked.Increment(ref _objectTypeCount); - - [Flags] - internal enum SnapshottedStateFlags : byte - { - None = 0, - PendingData = 1 << 1, - OpenResult = 1 << 2, - ErrorTokenReceived = 1 << 3, // Keep track of whether an error was received for the result. This is reset upon each done token - ColMetaDataReceived = 1 << 4, // Used to keep track of when to fire StatementCompleted event. - AttentionReceived = 1 << 5 // NOTE: Received is not volatile as it is only ever accessed\modified by TryRun its callees (i.e. single threaded access) - } - - 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; - private static readonly ContextCallback s_readAdyncCallbackComplete = ReadAsyncCallbackComplete; - // Ticks to consider a connection "good" after a successful I/O (10,000 ticks = 1 ms) - // The resolution of the timer is typically in the range 10 to 16 milliseconds according to msdn. - // We choose a value that is smaller than the likely timer resolution, but - // large enough to ensure that check connection execution will be 0.1% or less - // of very small open, query, close loops. - private const long CheckConnectionWindow = 50000; - - - protected readonly TdsParser _parser; // TdsParser pointer - private readonly WeakReference _owner = new WeakReference(null); // the owner of this session, used to track when it's been orphaned - internal SqlDataReader.SharedState _readerState; // susbset of SqlDataReader state (if it is the owner) necessary for parsing abandoned results in TDS - private int _activateCount; // 0 when we're in the pool, 1 when we're not, all others are an error - - // Two buffers exist in tdsparser, an in buffer and an out buffer. For the out buffer, only - // one bookkeeping variable is needed, the number of bytes used in the buffer. For the in buffer, - // three variables are actually needed. First, we need to record from the netlib how many bytes it - // read from the netlib, this variable is _inBytesRead. Then, we need to also keep track of how many - // bytes we have used as we consume the bytes from the buffer, that variable is _inBytesUsed. Third, - // we need to keep track of how many bytes are left in the packet, so that we know when we have reached - // the end of the packet and so we need to consume the next header. That variable is _inBytesPacket. - - // Header length constants - internal readonly int _inputHeaderLen = TdsEnums.HEADER_LEN; - internal readonly int _outputHeaderLen = TdsEnums.HEADER_LEN; - - // Out buffer variables - internal byte[] _outBuff; // internal write buffer - initialize on login - internal int _outBytesUsed = TdsEnums.HEADER_LEN; // number of bytes used in internal write buffer - - // - initialize past header - // In buffer variables - - /// - /// internal read buffer - initialize on login - /// - protected byte[] _inBuff; - /// - /// number of bytes used in internal read buffer - /// - internal int _inBytesUsed; - /// - /// number of bytes read into internal read buffer - /// - internal int _inBytesRead; - - /// - /// number of bytes left in packet - /// - internal int _inBytesPacket; - - internal int _spid; // SPID of the current connection - - // Packet state variables - internal byte _outputMessageType; // tds header type - internal byte _messageStatus; // tds header status - internal byte _outputPacketNumber = 1; // number of packets sent to server in message - start at 1 per ramas - internal uint _outputPacketCount; - internal volatile bool _fResetEventOwned; // ResetEvent serializing call to sp_reset_connection - internal volatile bool _fResetConnectionSent; // For multiple packet execute - internal bool _bulkCopyOpperationInProgress; // Set to true during bulk copy and used to turn toggle write timeouts. - internal bool _bulkCopyWriteTimeout; // Set to trun when _bulkCopyOperationInProgress is trun and write timeout happens - - // SNI variables - /// - /// Used to synchronize access to _writePacketCache and _pendingWritePackets - /// - protected readonly object _writePacketLockObject = new object(); - - // Async variables - private int _pendingCallbacks; // we increment this before each async read/write call and decrement it in the callback. We use this to determine when to release the GcHandle... // 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; // true if we sent an Attention to the server - internal volatile bool _attentionSending; private readonly TimerCallback _onTimeoutAsync; - // 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; - - // secure password information to be stored - // At maximum number of secure string that need to be stored is two; one for login password and the other for new change password - private SecureString[] _securePasswords = new SecureString[2] { null, null }; - private int[] _securePasswordOffsetsInBuffer = new int[2]; - - // This variable is used to track whether another thread has requested a cancel. The - // synchronization points are - // On the user's execute thread: - // 1) the first packet write - // 2) session close - return this stateObj to the session pool - // On cancel thread we only have the cancel call. - // Currently all access to this variable is inside a lock, The state diagram is: - // 1) pre first packet write, if cancel is requested, set variable so exception is triggered - // on user thread when first packet write is attempted - // 2) post first packet write, but before session return - a call to cancel will send an - // attention to the server - // 3) post session close - no attention is allowed - private bool _cancelled; - private const int _waitForCancellationLockPollTimeout = 100; private WeakReference _cancellationOwner = new WeakReference(null); - // Cache the transaction for which this command was executed so upon completion we can - // decrement the appropriate result count. - internal SqlInternalTransaction _executedUnderTransaction; - - // TDS stream processing variables - internal ulong _longlen; // plp data length indicator - internal ulong _longlenleft; // Length of data left to read (64 bit lengths) - internal int[] _decimalBits; // scratch buffer for decimal/numeric data - internal byte[] _bTmp = new byte[TdsEnums.SQL2005_HEADER_LEN]; // Scratch buffer for misc use - internal int _bTmpRead; // Counter for number of temporary bytes read - internal Decoder _plpdecoder; // Decoder object to process plp character data - internal bool _accumulateInfoEvents; // TRUE - accumulate info messages during TdsParser.Run, FALSE - fire them - internal List _pendingInfoEvents; - internal byte[] _bLongBytes; // scratch buffer to serialize Long values (8 bytes). - internal byte[] _bIntBytes; // scratch buffer to serialize Int values (4 bytes). - internal byte[] _bShortBytes; // scratch buffer to serialize Short values (2 bytes). - internal byte[] _bDecimalBytes; // scratch buffer to serialize decimal values (17 bytes). - - - // - // DO NOT USE THIS BUFFER FOR OTHER THINGS. - // ProcessHeader can be called ANYTIME while doing network reads. - private byte[] _partialHeaderBuffer = new byte[TdsEnums.HEADER_LEN]; // Scratch buffer for ProcessHeader - internal int _partialHeaderBytesRead; - - internal _SqlMetaDataSet _cleanupMetaData; - internal _SqlMetaDataSetCollection _cleanupAltMetaDataSetArray; - - private SniContext _sniContext = SniContext.Undefined; -#if DEBUG - private SniContext _debugOnlyCopyOfSniContext = SniContext.Undefined; -#endif - - private bool _bcpLock = false; - - // Null bitmap compression (NBC) information for the current row - private NullBitmap _nullBitmapInfo; - - // Async - internal TaskCompletionSource _networkPacketTaskSource; - private Timer _networkPacketTimeout; - internal bool _syncOverAsync = true; - private bool _snapshotReplay; - private StateSnapshot _snapshot; + // Async private StateSnapshot _cachedSnapshot; private SnapshottedStateFlags _snapshottedState; - internal ExecutionContext _executionContext; - internal bool _asyncReadWithoutSnapshot; -#if DEBUG - // Used to override the assert than ensures that the stacktraces on subsequent replays are the same - // This is useful is you are purposefully running the replay from a different thread (e.g. during SqlDataReader.Close) - internal bool _permitReplayStackTraceToDiffer; - - // Used to indicate that the higher level object believes that this stateObj has enough data to complete an operation - // If this stateObj has to read, then it will raise an assert - internal bool _shouldHaveEnoughData; -#endif - - // local exceptions to cache warnings and errors - internal SqlErrorCollection _errors; - internal SqlErrorCollection _warnings; - internal object _errorAndWarningsLock = new object(); - private bool _hasErrorOrWarning; - - // local exceptions to cache warnings and errors that occurred prior to sending attention - internal SqlErrorCollection _preAttentionErrors; - internal SqlErrorCollection _preAttentionWarnings; - - private volatile TaskCompletionSource _writeCompletionSource; - protected volatile int _asyncWriteCount; - private volatile Exception _delayedWriteAsyncCallbackException; // set by write async callback if completion source is not yet created - - // _readingcount is incremented when we are about to read. - // We check the parser state afterwards. - // When the read is completed, we decrement it before handling errors - // as the error handling may end up calling Dispose. - private int _readingCount; - - // Test hooks -#if DEBUG - // This is a test hook to enable testing of the retry paths. - // When set to true, almost every possible retry point will be attempted. - // This will drastically impact performance. - // - // Sample code to enable: - // - // Type type = typeof(SqlDataReader).Assembly.GetType("Microsoft.Data.SqlClient.TdsParserStateObject"); - // System.Reflection.FieldInfo field = type.GetField("_forceAllPends", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static); - // if (field != null) { - // field.SetValue(null, true); - // } - // - internal static bool _forceAllPends = false; - - // set this while making a call that should not block. - // instead of blocking it will fail. - internal static bool _failAsyncPends = false; - - // If this is set and an async read is made, then - // we will switch to syncOverAsync mode for the - // remainder of the async operation. - internal static bool _forceSyncOverAsyncAfterFirstPend = false; - - // Requests to send attention will be ignored when _skipSendAttention is true. - // This is useful to simulate circumstances where timeouts do not recover. - internal static bool _skipSendAttention = false; - - // Prevents any pending read from completing until the user signals it using - // CompletePendingReadWithSuccess() or CompletePendingReadWithFailure(int errorCode) in SqlCommand\SqlDataReader - internal static bool _forcePendingReadsToWaitForUser = false; - internal TaskCompletionSource _realNetworkPacketTaskSource; - - // Field is never assigned to, and will always have its default value -#pragma warning disable 0649 - // Set to true to enable checking the call stacks match when packet retry occurs. - internal static bool _checkNetworkPacketRetryStacks = false; -#pragma warning restore 0649 -#endif ////////////////// // Constructors // @@ -341,94 +85,11 @@ internal TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalCon //////////////// // Properties // //////////////// - - // BcpLock - use to lock this object if there is a potential risk of using this object - // between tds packets - internal bool BcpLock - { - get - { - return _bcpLock; - } - set - { - _bcpLock = value; - } - } - -#if DEBUG - internal SniContext DebugOnlyCopyOfSniContext - { - get - { - return _debugOnlyCopyOfSniContext; - } - } - - internal void InvalidateDebugOnlyCopyOfSniContext() - { - _debugOnlyCopyOfSniContext = SniContext.Undefined; - } -#endif - - internal bool IsOrphaned - { - get - { - bool isAlive = _owner.TryGetTarget(out object target); - Debug.Assert((0 == _activateCount && !isAlive) // in pool - || (1 == _activateCount && isAlive && target != null) - || (1 == _activateCount && !isAlive), "Unknown state on TdsParserStateObject.IsOrphaned!"); - return (0 != _activateCount && !isAlive); - } - } - - internal object Owner - { - set - { - Debug.Assert(value == null || !_owner.TryGetTarget(out object target) || value is SqlDataReader reader1 && reader1.Command == target, "Should not be changing the owner of an owned stateObj"); - if (value is SqlDataReader reader) - { - _readerState = reader._sharedState; - } - else - { - _readerState = null; - } - _owner.SetTarget(value); - } - } - internal abstract uint DisableSsl(); - internal bool HasOwner => _owner.TryGetTarget(out object _); - - internal TdsParser Parser - { - get - { - return _parser; - } - } internal abstract uint EnableMars(ref uint info); - internal SniContext SniContext - { - get - { - return _sniContext; - } - set - { - _sniContext = value; -#if DEBUG - _debugOnlyCopyOfSniContext = value; -#endif - } - } - internal abstract uint Status { get; @@ -439,107 +100,6 @@ internal abstract SessionHandle SessionHandle get; } - internal bool TimeoutHasExpired - { - get - { - Debug.Assert(0 == _timeoutMilliseconds || 0 == _timeoutTime, "_timeoutTime hasn't been reset"); - return TdsParserStaticMethods.TimeoutHasExpired(_timeoutTime); - } - } - - internal long TimeoutTime - { - get - { - if (0 != _timeoutMilliseconds) - { - _timeoutTime = TdsParserStaticMethods.GetTimeout(_timeoutMilliseconds); - _timeoutMilliseconds = 0; - } - return _timeoutTime; - } - set - { - _timeoutMilliseconds = 0; - _timeoutTime = value; - } - } - - internal int GetTimeoutRemaining() - { - int remaining; - if (0 != _timeoutMilliseconds) - { - remaining = (int)Math.Min((long)int.MaxValue, _timeoutMilliseconds); - _timeoutTime = TdsParserStaticMethods.GetTimeout(_timeoutMilliseconds); - _timeoutMilliseconds = 0; - } - else - { - remaining = TdsParserStaticMethods.GetTimeoutMilliseconds(_timeoutTime); - } - return remaining; - } - - internal bool TryStartNewRow(bool isNullCompressed, int nullBitmapColumnsCount = 0) - { - Debug.Assert(!isNullCompressed || nullBitmapColumnsCount > 0, "Null-Compressed row requires columns count"); - - if (_snapshot != null) - { - _snapshot.CloneNullBitmapInfo(); - } - - // initialize or unset null bitmap information for the current row - if (isNullCompressed) - { - // assert that NBCROW is not in use by 2005 or before - Debug.Assert(_parser.Is2008OrNewer, "NBCROW is sent by pre-2008 server"); - - if (!_nullBitmapInfo.TryInitialize(this, nullBitmapColumnsCount)) - { - return false; - } - } - else - { - _nullBitmapInfo.Clean(); - } - - return true; - } - - internal bool IsRowTokenReady() - { - // Removing one byte since TryReadByteArray\TryReadByte will aggressively read the next packet if there is no data left - so we need to ensure there is a spare byte - int bytesRemaining = Math.Min(_inBytesPacket, _inBytesRead - _inBytesUsed) - 1; - - if (bytesRemaining > 0) - { - if (_inBuff[_inBytesUsed] == TdsEnums.SQLROW) - { - // At a row token, so we're ready - return true; - } - else if (_inBuff[_inBytesUsed] == TdsEnums.SQLNBCROW) - { - // NBC row token, ensure that we have enough data for the bitmap - // SQLNBCROW + Null Bitmap (copied from NullBitmap.TryInitialize) - int bytesToRead = 1 + (_cleanupMetaData.Length + 7) / 8; - return (bytesToRead <= bytesRemaining); - } - } - - // No data left, or not at a row token - return false; - } - - internal bool IsNullCompressionBitSet(int columnOrdinal) - { - return _nullBitmapInfo.IsGuaranteedNull(columnOrdinal); - } - private struct NullBitmap { private byte[] _nullBitmap; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj index a4ecd51c67..af8bb19629 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj @@ -432,6 +432,9 @@ Microsoft\Data\SqlClient\SQLFallbackDNSCache.cs + + Microsoft\Data\SqlClient\TdsParserStateObject.cs + Microsoft\Data\OperationAbortedException.cs 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 b22faacb58..48c2cf63c5 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 @@ -17,42 +17,8 @@ namespace Microsoft.Data.SqlClient { - sealed internal class LastIOTimer + internal partial class TdsParserStateObject { - internal long _value; - } - - sealed internal class TdsParserStateObject - { - private const int AttentionTimeoutSeconds = 5; - - // Ticks to consider a connection "good" after a successful I/O (10,000 ticks = 1 ms) - // The resolution of the timer is typically in the range 10 to 16 milliseconds according to msdn. - // We choose a value that is smaller than the likely timer resolution, but - // large enough to ensure that check connection execution will be 0.1% or less - // of very small open, query, close loops. - private const long CheckConnectionWindow = 50000; - - 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 static int _objectTypeCount; // EventSource Counter - internal readonly int _objectID = System.Threading.Interlocked.Increment(ref _objectTypeCount); - internal int ObjectID { get @@ -61,100 +27,23 @@ internal int ObjectID } } - private readonly TdsParser _parser; // TdsParser pointer private SNIHandle _sessionHandle = null; // the SNI handle we're to work on - private readonly WeakReference _owner = new WeakReference(null); // the owner of this session, used to track when it's been orphaned - internal SqlDataReader.SharedState _readerState; // susbset of SqlDataReader state (if it is the owner) necessary for parsing abandoned results in TDS - private int _activateCount; // 0 when we're in the pool, 1 when we're not, all others are an error - - // Two buffers exist in tdsparser, an in buffer and an out buffer. For the out buffer, only - // one bookkeeping variable is needed, the number of bytes used in the buffer. For the in buffer, - // three variables are actually needed. First, we need to record from the netlib how many bytes it - // read from the netlib, this variable is _inBytesRead. Then, we need to also keep track of how many - // bytes we have used as we consume the bytes from the buffer, that variable is _inBytesUsed. Third, - // we need to keep track of how many bytes are left in the packet, so that we know when we have reached - // the end of the packet and so we need to consume the next header. That variable is _inBytesPacket. - - // Header length constants - internal readonly int _inputHeaderLen = TdsEnums.HEADER_LEN; - internal readonly int _outputHeaderLen = TdsEnums.HEADER_LEN; - - // Out buffer variables - internal byte[] _outBuff; // internal write buffer - initialize on login - internal int _outBytesUsed = TdsEnums.HEADER_LEN; // number of bytes used in internal write buffer - - // - initialize past header - // In buffer variables - private byte[] _inBuff; // internal read buffer - initialize on login - internal int _inBytesUsed = 0; // number of bytes used in internal read buffer - internal int _inBytesRead = 0; // number of bytes read into internal read buffer - internal int _inBytesPacket = 0; // number of bytes left in packet - - internal int _spid; // SPID of the current connection - // Packet state variables - internal byte _outputMessageType = 0; // tds header type - internal byte _messageStatus; // tds header status - internal byte _outputPacketNumber = 1; // number of packets sent to server in message - start at 1 per ramas - internal uint _outputPacketCount; internal bool _pendingData = false; - internal volatile bool _fResetEventOwned = false; // ResetEvent serializing call to sp_reset_connection - internal volatile bool _fResetConnectionSent = false; // For multiple packet execute - + internal bool _errorTokenReceived = false; // Keep track of whether an error was received for the result. // This is reset upon each done token - there can be - - internal bool _bulkCopyOpperationInProgress = false; // Set to true during bulk copy and used to turn toggle write timeouts. - internal bool _bulkCopyWriteTimeout = false; // Set to trun when _bulkCopyOpeperationInProgress is trun and write timeout happens - // SNI variables // multiple resultsets in one batch. private SNIPacket _sniPacket = null; // Will have to re-vamp this for MARS internal SNIPacket _sniAsyncAttnPacket = null; // Packet to use to send Attn private WritePacketCache _writePacketCache = new WritePacketCache(); // Store write packets that are ready to be re-used 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 object _writePacketLockObject = new object(); // Used to synchronize access to _writePacketCache and _pendingWritePackets // Async variables private GCHandle _gcHandle; // keeps this object alive until we're closed. - private int _pendingCallbacks; // we increment this before each async read/write call and decrement it in the callback. We use this to determine when to release the GcHandle... // 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; - - // 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; - - // secure password information to be stored - // At maximum number of secure string that need to be stored is two; one for login password and the other for new change password - private SecureString[] _securePasswords = new SecureString[2] { null, null }; - private int[] _securePasswordOffsetsInBuffer = new int[2]; - - // This variable is used to track whether another thread has requested a cancel. The - // synchronization points are - // On the user's execute thread: - // 1) the first packet write - // 2) session close - return this stateObj to the session pool - // On cancel thread we only have the cancel call. - // Currently all access to this variable is inside a lock, though I hope to limit that in the - // future. The state diagram is: - // 1) pre first packet write, if cancel is requested, set variable so exception is triggered - // on user thread when first packet write is attempted - // 2) post first packet write, but before session return - a call to cancel will send an - // attention to the server - // 3) post session close - no attention is allowed - private bool _cancelled; - private const int _waitForCancellationLockPollTimeout = 100; // This variable is used to prevent sending an attention by another thread that is not the // current owner of the stateObj. I currently do not know how this can happen. Mark added @@ -163,33 +52,7 @@ internal int ObjectID private volatile int _allowObjectID; internal bool _hasOpenResult = false; - // Cache the transaction for which this command was executed so upon completion we can - // decrement the appropriate result count. - internal SqlInternalTransaction _executedUnderTransaction = null; - - // TDS stream processing variables - internal ulong _longlen; // plp data length indicator - internal ulong _longlenleft; // Length of data left to read (64 bit lengths) - internal int[] _decimalBits = null; // scratch buffer for decimal/numeric data - internal byte[] _bTmp = new byte[TdsEnums.SQL2005_HEADER_LEN]; // Scratch buffer for misc use - internal int _bTmpRead = 0; // Counter for number of temporary bytes read - internal Decoder _plpdecoder = null; // Decoder object to process plp character data - internal bool _accumulateInfoEvents = false; // TRUE - accumulate info messages during TdsParser.Run, FALSE - fire them - internal List _pendingInfoEvents = null; - internal byte[] _bLongBytes = null; // scratch buffer to serialize Long values (8 bytes). - internal byte[] _bIntBytes = null; // scratch buffer to serialize Int values (4 bytes). - internal byte[] _bShortBytes = null; // scratch buffer to serialize Short values (2 bytes). - internal byte[] _bDecimalBytes = null; // scratch buffer to serialize decimal values (17 bytes). - - // DO NOT USE THIS BUFFER FOR OTHER THINGS. - // ProcessHeader can be called ANYTIME while doing network reads. - private byte[] _partialHeaderBuffer = new byte[TdsEnums.HEADER_LEN]; // Scratch buffer for ProcessHeader - internal int _partialHeaderBytesRead = 0; - - // UNDONE - temporary hack for case where pooled connection is returned to pool with data on wire - - // need to cache metadata on parser to read off wire - internal _SqlMetaDataSet _cleanupMetaData = null; - internal _SqlMetaDataSetCollection _cleanupAltMetaDataSetArray = null; + // Used for blanking out password in trace. internal int _tracePasswordOffset = 0; @@ -199,91 +62,6 @@ internal int ObjectID internal bool _receivedColMetaData; // Used to keep track of when to fire StatementCompleted event. - private SniContext _sniContext = SniContext.Undefined; -#if DEBUG - private SniContext _debugOnlyCopyOfSniContext = SniContext.Undefined; -#endif - - private bool _bcpLock = false; - - // Null bitmap compression (NBC) information for the current row - private NullBitmap _nullBitmapInfo; - - // Async - internal TaskCompletionSource _networkPacketTaskSource; - private Timer _networkPacketTimeout; - internal bool _syncOverAsync = true; - private bool _snapshotReplay = false; - private StateSnapshot _snapshot; - internal ExecutionContext _executionContext; - internal bool _asyncReadWithoutSnapshot = false; -#if DEBUG - // Used to override the assert than ensures that the stacktraces on subsequent replays are the same - // This is useful is you are purposefully running the replay from a different thread (e.g. during SqlDataReader.Close) - internal bool _permitReplayStackTraceToDiffer = false; - - // Used to indicate that the higher level object believes that this stateObj has enough data to complete an operation - // If this stateObj has to read, then it will raise an assert - internal bool _shouldHaveEnoughData = false; -#endif - - // local exceptions to cache warnings and errors - internal SqlErrorCollection _errors; - internal SqlErrorCollection _warnings; - internal object _errorAndWarningsLock = new object(); - private bool _hasErrorOrWarning = false; - - // local exceptions to cache warnings and errors that occurred prior to sending attention - internal SqlErrorCollection _preAttentionErrors; - internal SqlErrorCollection _preAttentionWarnings; - - volatile private TaskCompletionSource _writeCompletionSource = null; - volatile private int _asyncWriteCount = 0; - volatile private Exception _delayedWriteAsyncCallbackException = null; // set by write async callback if completion source is not yet created - - // _readingcount is incremented when we are about to read. - // We check the parser state afterwards. - // When the read is completed, we decrement it before handling errors - // as the error handling may end up calling Dispose. - int _readingCount; - - // Test hooks -#if DEBUG - // This is a test hook to enable testing of the retry paths. - // When set to true, almost every possible retry point will be attempted. - // This will drastically impact performance. - // - // Sample code to enable: - // - // Type type = typeof(SqlDataReader).Assembly.GetType("Microsoft.Data.SqlClient.TdsParserStateObject"); - // System.Reflection.FieldInfo field = type.GetField("_forceAllPends", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static); - // if (field != null) { - // field.SetValue(null, true); - // } - // - internal static bool _forceAllPends = false; - - // set this while making a call that should not block. - // instead of blocking it will fail. - internal static bool _failAsyncPends = false; - - // If this is set and an async read is made, then - // we will switch to syncOverAsync mode for the - // remainder of the async operation. - internal static bool _forceSyncOverAsyncAfterFirstPend = false; - - // Requests to send attention will be ignored when _skipSendAttention is true. - // This is useful to simulate circumstances where timeouts do not recover. - internal static bool _skipSendAttention = false; - - // Prevents any pending read from completing until the user signals it using - // CompletePendingReadWithSuccess() or CompletePendingReadWithFailure(int errorCode) in SqlCommand\SqlDataReader - internal static bool _forcePendingReadsToWaitForUser; - internal TaskCompletionSource _realNetworkPacketTaskSource = null; - - // Set to true to enable checking the call stacks match when packet retry occurs. - internal static bool _checkNetworkPacketRetryStacks; -#endif ////////////////// // Constructors // @@ -343,31 +121,6 @@ internal TdsParserStateObject(TdsParser parser, SNIHandle physicalConnection, bo //////////////// // Properties // //////////////// - - // BcpLock - use to lock this object if there is a potential risk of using this object - // between tds packets - internal bool BcpLock - { - get - { - return _bcpLock; - } - set - { - _bcpLock = value; - } - } - -#if DEBUG - internal SniContext DebugOnlyCopyOfSniContext - { - get - { - return _debugOnlyCopyOfSniContext; - } - } -#endif - internal SNIHandle Handle { get @@ -384,67 +137,6 @@ internal bool HasOpenResult } } -#if DEBUG - internal void InvalidateDebugOnlyCopyOfSniContext() - { - _debugOnlyCopyOfSniContext = SniContext.Undefined; - } -#endif - - internal bool IsOrphaned - { - get - { - bool isAlive = _owner.TryGetTarget(out object target); - Debug.Assert((0 == _activateCount && !isAlive) // in pool - || (1 == _activateCount && isAlive && target != null) - || (1 == _activateCount && !isAlive), "Unknown state on TdsParserStateObject.IsOrphaned!"); - return 0 != _activateCount && !isAlive; - } - } - - internal object Owner - { - set - { - Debug.Assert(value == null || !_owner.TryGetTarget(out object target) || value is SqlDataReader reader1 && reader1.Command == target, "Should not be changing the owner of an owned stateObj"); - if (value is SqlDataReader reader) - { - _readerState = reader._sharedState; - } - else - { - _readerState = null; - } - _owner.SetTarget(value); - } - } - - internal bool HasOwner => _owner.TryGetTarget(out object _); - - internal TdsParser Parser - { - get - { - return _parser; - } - } - - internal SniContext SniContext - { - get - { - return _sniContext; - } - set - { - _sniContext = value; -#if DEBUG - _debugOnlyCopyOfSniContext = value; -#endif - } - } - internal UInt32 Status { get @@ -460,107 +152,6 @@ internal UInt32 Status } } - internal bool TimeoutHasExpired - { - get - { - Debug.Assert(0 == _timeoutMilliseconds || 0 == _timeoutTime, "_timeoutTime hasn't been reset"); - return TdsParserStaticMethods.TimeoutHasExpired(_timeoutTime); - } - } - - internal long TimeoutTime - { - get - { - if (0 != _timeoutMilliseconds) - { - _timeoutTime = TdsParserStaticMethods.GetTimeout(_timeoutMilliseconds); - _timeoutMilliseconds = 0; - } - return _timeoutTime; - } - set - { - _timeoutMilliseconds = 0; - _timeoutTime = value; - } - } - - internal int GetTimeoutRemaining() - { - int remaining; - if (0 != _timeoutMilliseconds) - { - remaining = (int)Math.Min((long)Int32.MaxValue, _timeoutMilliseconds); - _timeoutTime = TdsParserStaticMethods.GetTimeout(_timeoutMilliseconds); - _timeoutMilliseconds = 0; - } - else - { - remaining = TdsParserStaticMethods.GetTimeoutMilliseconds(_timeoutTime); - } - return remaining; - } - - internal bool TryStartNewRow(bool isNullCompressed, int nullBitmapColumnsCount = 0) - { - Debug.Assert(!isNullCompressed || nullBitmapColumnsCount > 0, "Null-Compressed row requires columns count"); - - if (_snapshot != null) - { - _snapshot.CloneNullBitmapInfo(); - } - - // initialize or unset null bitmap information for the current row - if (isNullCompressed) - { - // assert that NBCROW is not in use by 2005 or before - Debug.Assert(_parser.Is2008OrNewer, "NBCROW is sent by pre-2008 server"); - - if (!_nullBitmapInfo.TryInitialize(this, nullBitmapColumnsCount)) - { - return false; - } - } - else - { - _nullBitmapInfo.Clean(); - } - - return true; - } - - internal bool IsRowTokenReady() - { - // Removing one byte since TryReadByteArray\TryReadByte will aggressively read the next packet if there is no data left - so we need to ensure there is a spare byte - int bytesRemaining = Math.Min(_inBytesPacket, _inBytesRead - _inBytesUsed) - 1; - - if (bytesRemaining > 0) - { - if (_inBuff[_inBytesUsed] == TdsEnums.SQLROW) - { - // At a row token, so we're ready - return true; - } - else if (_inBuff[_inBytesUsed] == TdsEnums.SQLNBCROW) - { - // NBC row token, ensure that we have enough data for the bitmap - // SQLNBCROW + Null Bitmap (copied from NullBitmap.TryInitialize) - int bytesToRead = 1 + (_cleanupMetaData.Length + 7) / 8; - return (bytesToRead <= bytesRemaining); - } - } - - // No data left, or not at a row token - return false; - } - - internal bool IsNullCompressionBitSet(int columnOrdinal) - { - return _nullBitmapInfo.IsGuaranteedNull(columnOrdinal); - } - private struct NullBitmap { private byte[] _nullBitmap; diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs new file mode 100644 index 0000000000..1b94a28efb --- /dev/null +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -0,0 +1,454 @@ +// 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.Generic; +using System.Diagnostics; +using System.Security; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.Data.SqlClient +{ + sealed internal class LastIOTimer + { + internal long _value; + } + + partial class TdsParserStateObject + { + private static int _objectTypeCount; // EventSource counter + internal readonly int _objectID = Interlocked.Increment(ref _objectTypeCount); + + [Flags] + internal enum SnapshottedStateFlags : byte + { + None = 0, + PendingData = 1 << 1, + OpenResult = 1 << 2, + ErrorTokenReceived = 1 << 3, // Keep track of whether an error was received for the result. This is reset upon each done token + ColMetaDataReceived = 1 << 4, // Used to keep track of when to fire StatementCompleted event. + AttentionReceived = 1 << 5 // NOTE: Received is not volatile as it is only ever accessed\modified by TryRun its callees (i.e. single threaded access) + } + + 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) + // The resolution of the timer is typically in the range 10 to 16 milliseconds according to msdn. + // We choose a value that is smaller than the likely timer resolution, but + // large enough to ensure that check connection execution will be 0.1% or less + // of very small open, query, close loops. + private const long CheckConnectionWindow = 50000; + + + protected readonly TdsParser _parser; // TdsParser pointer + private readonly WeakReference _owner = new(null); // the owner of this session, used to track when it's been orphaned + internal SqlDataReader.SharedState _readerState; // susbset of SqlDataReader state (if it is the owner) necessary for parsing abandoned results in TDS + private int _activateCount; // 0 when we're in the pool, 1 when we're not, all others are an error + + // Two buffers exist in tdsparser, an in buffer and an out buffer. For the out buffer, only + // one bookkeeping variable is needed, the number of bytes used in the buffer. For the in buffer, + // three variables are actually needed. First, we need to record from the netlib how many bytes it + // read from the netlib, this variable is _inBytesRead. Then, we need to also keep track of how many + // bytes we have used as we consume the bytes from the buffer, that variable is _inBytesUsed. Third, + // we need to keep track of how many bytes are left in the packet, so that we know when we have reached + // the end of the packet and so we need to consume the next header. That variable is _inBytesPacket. + + // Header length constants + internal readonly int _inputHeaderLen = TdsEnums.HEADER_LEN; + internal readonly int _outputHeaderLen = TdsEnums.HEADER_LEN; + + // Out buffer variables + internal byte[] _outBuff; // internal write buffer - initialize on login + internal int _outBytesUsed = TdsEnums.HEADER_LEN; // number of bytes used in internal write buffer - initialize past header + + // In buffer variables + + /// + /// internal read buffer - initialize on login + /// + protected byte[] _inBuff; + /// + /// number of bytes used in internal read buffer + /// + internal int _inBytesUsed; + /// + /// number of bytes read into internal read buffer + /// + internal int _inBytesRead; + + /// + /// number of bytes left in packet + /// + internal int _inBytesPacket; + + internal int _spid; // SPID of the current connection + + // Packet state variables + internal byte _outputMessageType; // tds header type + internal byte _messageStatus; // tds header status + internal byte _outputPacketNumber = 1; // number of packets sent to server in message - start at 1 per ramas + internal uint _outputPacketCount; + + internal volatile bool _fResetEventOwned; // ResetEvent serializing call to sp_reset_connection + internal volatile bool _fResetConnectionSent; // For multiple packet execute + internal bool _bulkCopyOpperationInProgress; // Set to true during bulk copy and used to turn toggle write timeouts. + internal bool _bulkCopyWriteTimeout; // Set to trun when _bulkCopyOperationInProgress is trun and write timeout happens + + // SNI variables + /// + /// Used to synchronize access to _writePacketCache and _pendingWritePackets + /// + protected readonly object _writePacketLockObject = new object(); + + // Async variables + private int _pendingCallbacks; // we increment this before each async read/write call and decrement it in the callback. We use this to determine when to release the GcHandle... + + // 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; // true if we sent an Attention to the server + internal volatile bool _attentionSending; + + + // 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; + + // secure password information to be stored + // At maximum number of secure string that need to be stored is two; one for login password and the other for new change password + private SecureString[] _securePasswords = new SecureString[2] { null, null }; + private int[] _securePasswordOffsetsInBuffer = new int[2]; + + // This variable is used to track whether another thread has requested a cancel. The + // synchronization points are + // On the user's execute thread: + // 1) the first packet write + // 2) session close - return this stateObj to the session pool + // On cancel thread we only have the cancel call. + // Currently all access to this variable is inside a lock, The state diagram is: + // 1) pre first packet write, if cancel is requested, set variable so exception is triggered + // on user thread when first packet write is attempted + // 2) post first packet write, but before session return - a call to cancel will send an + // attention to the server + // 3) post session close - no attention is allowed + private bool _cancelled; + private const int _waitForCancellationLockPollTimeout = 100; + + // Cache the transaction for which this command was executed so upon completion we can + // decrement the appropriate result count. + internal SqlInternalTransaction _executedUnderTransaction; + + // TDS stream processing variables + internal ulong _longlen; // plp data length indicator + internal ulong _longlenleft; // Length of data left to read (64 bit lengths) + internal int[] _decimalBits; // scratch buffer for decimal/numeric data + internal byte[] _bTmp = new byte[TdsEnums.SQL2005_HEADER_LEN]; // Scratch buffer for misc use + internal int _bTmpRead; // Counter for number of temporary bytes read + internal Decoder _plpdecoder; // Decoder object to process plp character data + internal bool _accumulateInfoEvents; // TRUE - accumulate info messages during TdsParser.Run, FALSE - fire them + internal List _pendingInfoEvents; + internal byte[] _bLongBytes; // scratch buffer to serialize Long values (8 bytes). + internal byte[] _bIntBytes; // scratch buffer to serialize Int values (4 bytes). + internal byte[] _bShortBytes; // scratch buffer to serialize Short values (2 bytes). + internal byte[] _bDecimalBytes; // scratch buffer to serialize decimal values (17 bytes). + + + // DO NOT USE THIS BUFFER FOR OTHER THINGS. + // ProcessHeader can be called ANYTIME while doing network reads. + private byte[] _partialHeaderBuffer = new byte[TdsEnums.HEADER_LEN]; // Scratch buffer for ProcessHeader + internal int _partialHeaderBytesRead; + + internal _SqlMetaDataSet _cleanupMetaData; + internal _SqlMetaDataSetCollection _cleanupAltMetaDataSetArray; + + + private SniContext _sniContext = SniContext.Undefined; +#if DEBUG + private SniContext _debugOnlyCopyOfSniContext = SniContext.Undefined; +#endif + + private bool _bcpLock; + + // Null bitmap compression (NBC) information for the current row + private NullBitmap _nullBitmapInfo; + + // Async + internal TaskCompletionSource _networkPacketTaskSource; + private Timer _networkPacketTimeout; + internal bool _syncOverAsync = true; + private bool _snapshotReplay; + private StateSnapshot _snapshot; + internal ExecutionContext _executionContext; + internal bool _asyncReadWithoutSnapshot; +#if DEBUG + // Used to override the assert than ensures that the stacktraces on subsequent replays are the same + // This is useful is you are purposefully running the replay from a different thread (e.g. during SqlDataReader.Close) + internal bool _permitReplayStackTraceToDiffer; + + // Used to indicate that the higher level object believes that this stateObj has enough data to complete an operation + // If this stateObj has to read, then it will raise an assert + internal bool _shouldHaveEnoughData; +#endif + + // local exceptions to cache warnings and errors + internal SqlErrorCollection _errors; + internal SqlErrorCollection _warnings; + internal object _errorAndWarningsLock = new object(); + private bool _hasErrorOrWarning; + + // local exceptions to cache warnings and errors that occurred prior to sending attention + internal SqlErrorCollection _preAttentionErrors; + internal SqlErrorCollection _preAttentionWarnings; + + private volatile TaskCompletionSource _writeCompletionSource; + protected volatile int _asyncWriteCount; + private volatile Exception _delayedWriteAsyncCallbackException; // set by write async callback if completion source is not yet created + + // _readingcount is incremented when we are about to read. + // We check the parser state afterwards. + // When the read is completed, we decrement it before handling errors + // as the error handling may end up calling Dispose. + private int _readingCount; + + // Test hooks +#if DEBUG + // This is a test hook to enable testing of the retry paths. + // When set to true, almost every possible retry point will be attempted. + // This will drastically impact performance. + // + // Sample code to enable: + // + // Type type = typeof(SqlDataReader).Assembly.GetType("Microsoft.Data.SqlClient.TdsParserStateObject"); + // System.Reflection.FieldInfo field = type.GetField("_forceAllPends", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static); + // if (field != null) { + // field.SetValue(null, true); + // } + // + internal static bool _forceAllPends = false; + + // set this while making a call that should not block. + // instead of blocking it will fail. + internal static bool _failAsyncPends = false; + + // If this is set and an async read is made, then + // we will switch to syncOverAsync mode for the + // remainder of the async operation. + internal static bool _forceSyncOverAsyncAfterFirstPend = false; + + // Requests to send attention will be ignored when _skipSendAttention is true. + // This is useful to simulate circumstances where timeouts do not recover. + internal static bool _skipSendAttention = false; + + // Prevents any pending read from completing until the user signals it using + // CompletePendingReadWithSuccess() or CompletePendingReadWithFailure(int errorCode) in SqlCommand\SqlDataReader + internal static bool _forcePendingReadsToWaitForUser = false; + internal TaskCompletionSource _realNetworkPacketTaskSource; + + // Field is never assigned to, and will always have its default value +#pragma warning disable 0649 + // Set to true to enable checking the call stacks match when packet retry occurs. + internal static bool _checkNetworkPacketRetryStacks = false; +#pragma warning restore 0649 +#endif + + + + + //////////////// + // Properties // + //////////////// + + // BcpLock - use to lock this object if there is a potential risk of using this object + // between tds packets + internal bool BcpLock + { + get => _bcpLock; + set => _bcpLock = value; + } + +#if DEBUG + internal SniContext DebugOnlyCopyOfSniContext => _debugOnlyCopyOfSniContext; + + internal void InvalidateDebugOnlyCopyOfSniContext() + { + _debugOnlyCopyOfSniContext = SniContext.Undefined; + } +#endif + + internal bool IsOrphaned + { + get + { + bool isAlive = _owner.TryGetTarget(out object target); + Debug.Assert((0 == _activateCount && !isAlive) // in pool + || (1 == _activateCount && isAlive && target != null) + || (1 == _activateCount && !isAlive), "Unknown state on TdsParserStateObject.IsOrphaned!"); + return (0 != _activateCount && !isAlive); + } + } + + internal object Owner + { + set + { + Debug.Assert(value == null || !_owner.TryGetTarget(out object target) || value is SqlDataReader reader1 && reader1.Command == target, "Should not be changing the owner of an owned stateObj"); + if (value is SqlDataReader reader) + { + _readerState = reader._sharedState; + } + else + { + _readerState = null; + } + _owner.SetTarget(value); + } + } + + internal bool HasOwner => _owner.TryGetTarget(out object _); + + internal TdsParser Parser => _parser; + + internal SniContext SniContext + { + get + { + return _sniContext; + } + set + { + _sniContext = value; +#if DEBUG + _debugOnlyCopyOfSniContext = value; +#endif + } + } + + internal bool TimeoutHasExpired + { + get + { + Debug.Assert(0 == _timeoutMilliseconds || 0 == _timeoutTime, "_timeoutTime hasn't been reset"); + return TdsParserStaticMethods.TimeoutHasExpired(_timeoutTime); + } + } + + internal long TimeoutTime + { + get + { + if (0 != _timeoutMilliseconds) + { + _timeoutTime = TdsParserStaticMethods.GetTimeout(_timeoutMilliseconds); + _timeoutMilliseconds = 0; + } + return _timeoutTime; + } + set + { + _timeoutMilliseconds = 0; + _timeoutTime = value; + } + } + + internal int GetTimeoutRemaining() + { + int remaining; + if (0 != _timeoutMilliseconds) + { + remaining = (int)Math.Min((long)int.MaxValue, _timeoutMilliseconds); + _timeoutTime = TdsParserStaticMethods.GetTimeout(_timeoutMilliseconds); + _timeoutMilliseconds = 0; + } + else + { + remaining = TdsParserStaticMethods.GetTimeoutMilliseconds(_timeoutTime); + } + return remaining; + } + + internal bool TryStartNewRow(bool isNullCompressed, int nullBitmapColumnsCount = 0) + { + Debug.Assert(!isNullCompressed || nullBitmapColumnsCount > 0, "Null-Compressed row requires columns count"); + + if (_snapshot != null) + { + _snapshot.CloneNullBitmapInfo(); + } + + // initialize or unset null bitmap information for the current row + if (isNullCompressed) + { + // assert that NBCROW is not in use by 2005 or before + Debug.Assert(_parser.Is2008OrNewer, "NBCROW is sent by pre-2008 server"); + + if (!_nullBitmapInfo.TryInitialize(this, nullBitmapColumnsCount)) + { + return false; + } + } + else + { + _nullBitmapInfo.Clean(); + } + + return true; + } + + internal bool IsRowTokenReady() + { + // Removing one byte since TryReadByteArray\TryReadByte will aggressively read the next packet if there is no data left - so we need to ensure there is a spare byte + int bytesRemaining = Math.Min(_inBytesPacket, _inBytesRead - _inBytesUsed) - 1; + + if (bytesRemaining > 0) + { + if (_inBuff[_inBytesUsed] == TdsEnums.SQLROW) + { + // At a row token, so we're ready + return true; + } + else if (_inBuff[_inBytesUsed] == TdsEnums.SQLNBCROW) + { + // NBC row token, ensure that we have enough data for the bitmap + // SQLNBCROW + Null Bitmap (copied from NullBitmap.TryInitialize) + int bytesToRead = 1 + (_cleanupMetaData.Length + 7) / 8; + return (bytesToRead <= bytesRemaining); + } + } + + // No data left, or not at a row token + return false; + } + + internal bool IsNullCompressionBitSet(int columnOrdinal) + { + return _nullBitmapInfo.IsGuaranteedNull(columnOrdinal); + } + } +} From 4ec17f6f2b2d7e2a7fa5bf6b5fda375f6909d256 Mon Sep 17 00:00:00 2001 From: panoskj Date: Mon, 21 Feb 2022 12:20:40 +0200 Subject: [PATCH 02/16] Merging common type NullBitmap of TdsParserStateObject. --- .../Data/SqlClient/TdsParserStateObject.cs | 45 +----------------- .../Data/SqlClient/TdsParserStateObject.cs | 45 +----------------- .../Data/SqlClient/TdsParserStateObject.cs | 46 +++++++++++++++++++ 3 files changed, 48 insertions(+), 88 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 e89f0fe1c1..82e8ac47c4 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 @@ -100,11 +100,8 @@ internal abstract SessionHandle SessionHandle get; } - private struct NullBitmap + private partial struct NullBitmap { - private byte[] _nullBitmap; - private int _columnsCount; // set to 0 if not used or > 0 for NBC rows - internal bool TryInitialize(TdsParserStateObject stateObj, int columnsCount) { _columnsCount = columnsCount; @@ -128,46 +125,6 @@ internal bool TryInitialize(TdsParserStateObject stateObj, int columnsCount) SqlClientEventSource.Log.TryAdvancedTraceBinEvent("TdsParserStateObject.NullBitmap.Initialize | INFO | ADV | State Object Id {0}, Null Bitmap length {1}, NBCROW bitmap data: {2}", stateObj._objectID, (ushort)_nullBitmap.Length, _nullBitmap); return true; } - - internal bool ReferenceEquals(NullBitmap obj) - { - return object.ReferenceEquals(_nullBitmap, obj._nullBitmap); - } - - internal NullBitmap Clone() - { - NullBitmap newBitmap = new NullBitmap(); - newBitmap._nullBitmap = _nullBitmap == null ? null : (byte[])_nullBitmap.Clone(); - newBitmap._columnsCount = _columnsCount; - return newBitmap; - } - - internal void Clean() - { - _columnsCount = 0; - // no need to free _nullBitmap array - it is cached for the next row - } - - /// - /// If this method returns true, the value is guaranteed to be null. This is not true vice versa: - /// if the bitmap value is false (if this method returns false), the value can be either null or non-null - no guarantee in this case. - /// To determine whether it is null or not, read it from the TDS (per NBCROW design spec, for IMAGE/TEXT/NTEXT columns server might send - /// bitmap = 0, when the actual value is null). - /// - internal bool IsGuaranteedNull(int columnOrdinal) - { - if (_columnsCount == 0) - { - // not an NBC row - return false; - } - - Debug.Assert(columnOrdinal >= 0 && columnOrdinal < _columnsCount, "Invalid column ordinal"); - - byte testBit = (byte)(1 << (columnOrdinal & 0x7)); // columnOrdinal & 0x7 == columnOrdinal MOD 0x7 - byte testByte = _nullBitmap[columnOrdinal >> 3]; - return (testBit & testByte) != 0; - } } ///////////////////// 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 48c2cf63c5..8cb79ce187 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 @@ -152,11 +152,8 @@ internal UInt32 Status } } - private struct NullBitmap + private partial struct NullBitmap { - private byte[] _nullBitmap; - private int _columnsCount; // set to 0 if not used or > 0 for NBC rows - internal bool TryInitialize(TdsParserStateObject stateObj, int columnsCount) { _columnsCount = columnsCount; @@ -180,46 +177,6 @@ internal bool TryInitialize(TdsParserStateObject stateObj, int columnsCount) return true; } - - internal bool ReferenceEquals(NullBitmap obj) - { - return object.ReferenceEquals(_nullBitmap, obj._nullBitmap); - } - - internal NullBitmap Clone() - { - NullBitmap newBitmap = new NullBitmap(); - newBitmap._nullBitmap = _nullBitmap == null ? null : (byte[])_nullBitmap.Clone(); - newBitmap._columnsCount = _columnsCount; - return newBitmap; - } - - internal void Clean() - { - _columnsCount = 0; - // no need to free _nullBitmap array - it is cached for the next row - } - - /// - /// If this method returns true, the value is guaranteed to be null. This is not true vice versa: - /// if the bitmat value is false (if this method returns false), the value can be either null or non-null - no guarantee in this case. - /// To determine whether it is null or not, read it from the TDS (per NBCROW design spec, for IMAGE/TEXT/NTEXT columns server might send - /// bitmap = 0, when the actual value is null). - /// - internal bool IsGuaranteedNull(int columnOrdinal) - { - if (_columnsCount == 0) - { - // not an NBC row - return false; - } - - Debug.Assert(columnOrdinal >= 0 && columnOrdinal < _columnsCount, "Invalid column ordinal"); - - byte testBit = (byte)(1 << (columnOrdinal & 0x7)); // columnOrdinal & 0x7 == columnOrdinal MOD 0x7 - byte testByte = _nullBitmap[columnOrdinal >> 3]; - return (testBit & testByte) != 0; - } } ///////////////////// 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 1b94a28efb..934483e891 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -450,5 +450,51 @@ internal bool IsNullCompressionBitSet(int columnOrdinal) { return _nullBitmapInfo.IsGuaranteedNull(columnOrdinal); } + + private partial struct NullBitmap + { + private byte[] _nullBitmap; + private int _columnsCount; // set to 0 if not used or > 0 for NBC rows + + internal bool ReferenceEquals(NullBitmap obj) + { + return object.ReferenceEquals(_nullBitmap, obj._nullBitmap); + } + + internal NullBitmap Clone() + { + NullBitmap newBitmap = new NullBitmap(); + newBitmap._nullBitmap = _nullBitmap == null ? null : (byte[])_nullBitmap.Clone(); + newBitmap._columnsCount = _columnsCount; + return newBitmap; + } + + internal void Clean() + { + _columnsCount = 0; + // no need to free _nullBitmap array - it is cached for the next row + } + + /// + /// If this method returns true, the value is guaranteed to be null. This is not true vice versa: + /// if the bitmap value is false (if this method returns false), the value can be either null or non-null - no guarantee in this case. + /// To determine whether it is null or not, read it from the TDS (per NBCROW design spec, for IMAGE/TEXT/NTEXT columns server might send + /// bitmap = 0, when the actual value is null). + /// + internal bool IsGuaranteedNull(int columnOrdinal) + { + if (_columnsCount == 0) + { + // not an NBC row + return false; + } + + Debug.Assert(columnOrdinal >= 0 && columnOrdinal < _columnsCount, "Invalid column ordinal"); + + byte testBit = (byte)(1 << (columnOrdinal & 0x7)); // columnOrdinal & 0x7 == columnOrdinal MOD 0x7 + byte testByte = _nullBitmap[columnOrdinal >> 3]; + return (testBit & testByte) != 0; + } + } } } From de83b8d835d7183b73ed5a88161f58be2d715ab5 Mon Sep 17 00:00:00 2001 From: panoskj Date: Mon, 21 Feb 2022 12:30:46 +0200 Subject: [PATCH 03/16] Merging a few methods of TdsParserStateObject. --- .../Data/SqlClient/TdsParserStateObject.cs | 85 ----------------- .../Data/SqlClient/TdsParserStateObject.cs | 91 ------------------- .../Data/SqlClient/TdsParserStateObject.cs | 91 +++++++++++++++++++ 3 files changed, 91 insertions(+), 176 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 82e8ac47c4..1a7c558c03 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 @@ -131,15 +131,6 @@ internal bool TryInitialize(TdsParserStateObject stateObj, int columnsCount) // General methods // ///////////////////// - // If this object is part of a TdsParserSessionPool, then this *must* be called inside the pool's lock - internal void Activate(object owner) - { - Debug.Assert(_parser.MARSOn, "Can not activate a non-MARS connection"); - Owner = owner; // must assign an owner for reclamation to work - int result = Interlocked.Increment(ref _activateCount); // must have non-zero activation count for reclamation to work too. - Debug.Assert(result == 1, "invalid deactivate count"); - } - // This method is only called by the command or datareader as a result of a user initiated // cancel request. internal void Cancel(object caller) @@ -204,82 +195,6 @@ internal void Cancel(object caller) } } - // CancelRequest - use to cancel while writing a request to the server - // - // o none of the request might have been sent to the server, simply reset the buffer, - // sending attention does not hurt - // o the request was partially written. Send an ignore header to the server. attention is - // required if the server was waiting for data (e.g. insert bulk rows) - // o the request was completely written out and the server started to process the request. - // attention is required to have the server stop processing. - // - internal void CancelRequest() - { - ResetBuffer(); // clear out unsent buffer - // If the first sqlbulkcopy timeout, _outputPacketNumber may not be 1, - // the next sqlbulkcopy (same connection string) requires this to be 1, hence reset - // it here when exception happens in the first sqlbulkcopy - ResetPacketCounters(); - - // VSDD#907507, if bulkcopy write timeout happens, it already sent the attention, - // so no need to send it again - if (!_bulkCopyWriteTimeout) - { - SendAttention(); - Parser.ProcessPendingAck(this); - } - } - - public void CheckSetResetConnectionState(uint error, CallbackType callbackType) - { - // Should only be called for MARS - that is the only time we need to take - // the ResetConnection lock! - - // It was raised in a security review by Microsoft questioning whether - // we need to actually process the resulting packet (sp_reset ack or error) to know if the - // reset actually succeeded. There was a concern that if the reset failed and we proceeded - // there might be a security issue present. We have been assured by the server that if - // sp_reset fails, they guarantee they will kill the resulting connection. So - it is - // safe for us to simply receive the packet and then consume the pre-login later. - - Debug.Assert(_parser.MARSOn, "Should not be calling CheckSetResetConnectionState on non MARS connection"); - - if (_fResetEventOwned) - { - if (callbackType == CallbackType.Read && TdsEnums.SNI_SUCCESS == error) - { - // RESET SUCCEEDED! - // If we are on read callback and no error occurred (and we own reset event) - - // then we sent the sp_reset_connection and so we need to reset sp_reset_connection - // flag to false, and then release the ResetEvent. - _parser._fResetConnection = false; - _fResetConnectionSent = false; - _fResetEventOwned = !_parser._resetConnectionEvent.Set(); - Debug.Assert(!_fResetEventOwned, "Invalid AutoResetEvent state!"); - } - - if (TdsEnums.SNI_SUCCESS != error) - { - // RESET FAILED! - - // If write or read failed with reset, we need to clear event but not mark connection - // as reset. - _fResetConnectionSent = false; - _fResetEventOwned = !_parser._resetConnectionEvent.Set(); - Debug.Assert(!_fResetEventOwned, "Invalid AutoResetEvent state!"); - } - } - } - - internal void CloseSession() - { - ResetCancelAndProcessAttention(); -#if DEBUG - InvalidateDebugOnlyCopyOfSniContext(); -#endif - Parser.PutSession(this); - } - private void ResetCancelAndProcessAttention() { // This method is shared by CloseSession initiated by DataReader.Close or completed 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 8cb79ce187..5b88a55511 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 @@ -183,15 +183,6 @@ internal bool TryInitialize(TdsParserStateObject stateObj, int columnsCount) // General methods // ///////////////////// - // If this object is part of a TdsParserSessionPool, then this *must* be called inside the pool's lock - internal void Activate(object owner) - { - Debug.Assert(_parser.MARSOn, "Can not activate a non-MARS connection"); - Owner = owner; // must assign an owner for reclaimation to work - int result = Interlocked.Increment(ref _activateCount); // must have non-zero activation count for reclaimation to work too. - Debug.Assert(result == 1, "invalid deactivate count"); - } - // This method is only called by the command or datareader as a result of a user initiated // cancel request. internal void Cancel(int objectID) @@ -256,88 +247,6 @@ internal void Cancel(int objectID) } } - // CancelRequest - use to cancel while writing a request to the server - // - // o none of the request might have been sent to the server, simply reset the buffer, - // sending attention does not hurt - // o the request was partially written. Send an ignore header to the server. attention is - // required if the server was waiting for data (e.g. insert bulk rows) - // o the request was completely written out and the server started to process the request. - // attention is required to have the server stop processing. - // - internal void CancelRequest() - { - ResetBuffer(); // clear out unsent buffer - // VSDD#903514, if the first sqlbulkcopy timeout, _outputPacketNumber may not be 1, - // the next sqlbulkcopy (same connection string) requires this to be 1, hence reset - // it here when exception happens in the first sqlbulkcopy - ResetPacketCounters(); - - // VSDD#907507, if bulkcopy write timeout happens, it already sent the attention, - // so no need to send it again - if (!_bulkCopyWriteTimeout) - { - SendAttention(); - Parser.ProcessPendingAck(this); - } - } - - public void CheckSetResetConnectionState(UInt32 error, CallbackType callbackType) - { - // Should only be called for MARS - that is the only time we need to take - // the ResetConnection lock! - - // SQL BU DT 333026 - it was raised in a security review questioning whether - // we need to actually process the resulting packet (sp_reset ack or error) to know if the - // reset actually succeeded. There was a concern that if the reset failed and we proceeded - // there might be a security issue present. We have been assured by the server that if - // sp_reset fails, they guarantee they will kill the resulting connection. So - it is - // safe for us to simply receive the packet and then consume the pre-login later. - - Debug.Assert(_parser.MARSOn, "Should not be calling CheckSetResetConnectionState on non MARS connection"); - - if (_fResetEventOwned) - { - if (callbackType == CallbackType.Read && TdsEnums.SNI_SUCCESS == error) - { - // RESET SUCCEEDED! - // If we are on read callback and no error occurred (and we own reset event) - - // then we sent the sp_reset_connection and so we need to reset sp_reset_connection - // flag to false, and then release the ResetEvent. - _parser._fResetConnection = false; - _fResetConnectionSent = false; - _fResetEventOwned = !_parser._resetConnectionEvent.Set(); - Debug.Assert(!_fResetEventOwned, "Invalid AutoResetEvent state!"); - } - - if (TdsEnums.SNI_SUCCESS != error) - { - // RESET FAILED! - - // UNDONE - is there a bug here? - // 1) Are there any cases where netlib write error does not mean reset was not executed? - // 2) Can we receive error on 2nd or beyond packet write but the server has already - // processed the reset??? I don't believe it is the case, but I have inserted a comment - // here as a thought workitem. - - // If write or read failed with reset, we need to clear event but not mark connection - // as reset. - _fResetConnectionSent = false; - _fResetEventOwned = !_parser._resetConnectionEvent.Set(); - Debug.Assert(!_fResetEventOwned, "Invalid AutoResetEvent state!"); - } - } - } - - internal void CloseSession() - { - ResetCancelAndProcessAttention(); -#if DEBUG - InvalidateDebugOnlyCopyOfSniContext(); -#endif - Parser.PutSession(this); - } - private void ResetCancelAndProcessAttention() { // This method is shared by CloseSession initiated by DataReader.Close or completed 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 934483e891..f5e65c0e5f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -496,5 +496,96 @@ internal bool IsGuaranteedNull(int columnOrdinal) return (testBit & testByte) != 0; } } + + + ///////////////////// + // General methods // + ///////////////////// + + // If this object is part of a TdsParserSessionPool, then this *must* be called inside the pool's lock + internal void Activate(object owner) + { + Debug.Assert(_parser.MARSOn, "Can not activate a non-MARS connection"); + Owner = owner; // must assign an owner for reclamation to work + int result = Interlocked.Increment(ref _activateCount); // must have non-zero activation count for reclamation to work too. + Debug.Assert(result == 1, "invalid deactivate count"); + } + + // CancelRequest - use to cancel while writing a request to the server + // + // o none of the request might have been sent to the server, simply reset the buffer, + // sending attention does not hurt + // o the request was partially written. Send an ignore header to the server. attention is + // required if the server was waiting for data (e.g. insert bulk rows) + // o the request was completely written out and the server started to process the request. + // attention is required to have the server stop processing. + // + internal void CancelRequest() + { + ResetBuffer(); // clear out unsent buffer + // If the first sqlbulkcopy timeout, _outputPacketNumber may not be 1, + // the next sqlbulkcopy (same connection string) requires this to be 1, hence reset + // it here when exception happens in the first sqlbulkcopy + ResetPacketCounters(); + + // VSDD#907507, if bulkcopy write timeout happens, it already sent the attention, + // so no need to send it again + if (!_bulkCopyWriteTimeout) + { + SendAttention(); + Parser.ProcessPendingAck(this); + } + } + + public void CheckSetResetConnectionState(uint error, CallbackType callbackType) + { + // Should only be called for MARS - that is the only time we need to take + // the ResetConnection lock! + + // It was raised in a security review by Microsoft questioning whether + // we need to actually process the resulting packet (sp_reset ack or error) to know if the + // reset actually succeeded. There was a concern that if the reset failed and we proceeded + // there might be a security issue present. We have been assured by the server that if + // sp_reset fails, they guarantee they will kill the resulting connection. So - it is + // safe for us to simply receive the packet and then consume the pre-login later. + + Debug.Assert(_parser.MARSOn, "Should not be calling CheckSetResetConnectionState on non MARS connection"); + + if (_fResetEventOwned) + { + if (callbackType == CallbackType.Read && TdsEnums.SNI_SUCCESS == error) + { + // RESET SUCCEEDED! + // If we are on read callback and no error occurred (and we own reset event) - + // then we sent the sp_reset_connection and so we need to reset sp_reset_connection + // flag to false, and then release the ResetEvent. + _parser._fResetConnection = false; + _fResetConnectionSent = false; + _fResetEventOwned = !_parser._resetConnectionEvent.Set(); + Debug.Assert(!_fResetEventOwned, "Invalid AutoResetEvent state!"); + } + + if (TdsEnums.SNI_SUCCESS != error) + { + // RESET FAILED! + + // If write or read failed with reset, we need to clear event but not mark connection + // as reset. + _fResetConnectionSent = false; + _fResetEventOwned = !_parser._resetConnectionEvent.Set(); + Debug.Assert(!_fResetEventOwned, "Invalid AutoResetEvent state!"); + } + } + } + + internal void CloseSession() + { + ResetCancelAndProcessAttention(); +#if DEBUG + InvalidateDebugOnlyCopyOfSniContext(); +#endif + Parser.PutSession(this); + } + } } From d15a7d369e60922a4080d1dcdc49dca4faea6bfc Mon Sep 17 00:00:00 2001 From: panoskj Date: Mon, 21 Feb 2022 13:09:12 +0200 Subject: [PATCH 04/16] Merging constructor of TdsParserStateObject. --- .../Data/SqlClient/TdsParserStateObject.cs | 20 ------------------ .../Data/SqlClient/TdsParserStateObject.cs | 16 -------------- .../Data/SqlClient/TdsParserStateObject.cs | 21 +++++++++++++++++++ 3 files changed, 21 insertions(+), 36 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 1a7c558c03..62fb805ea4 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,10 +19,7 @@ internal abstract partial class TdsParserStateObject { private static readonly ContextCallback s_readAdyncCallbackComplete = ReadAsyncCallbackComplete; - // Timeout variables - private readonly TimerCallback _onTimeoutAsync; - private WeakReference _cancellationOwner = new WeakReference(null); // Async @@ -33,23 +30,6 @@ internal abstract partial class TdsParserStateObject // Constructors // ////////////////// - internal TdsParserStateObject(TdsParser parser) - { - // Construct a physical connection - Debug.Assert(null != parser, "no parser?"); - _parser = parser; - _onTimeoutAsync = OnTimeoutAsync; - - // For physical connection, initialize to default login packet size. - SetPacketSize(TdsEnums.DEFAULT_LOGIN_PACKET_SIZE); - - // we post a callback that represents the call to dispose; once the - // object is disposed, the next callback will cause the GC Handle to - // be released. - IncrementPendingCallbacks(); - _lastSuccessfulIOTimer = new LastIOTimer(); - } - internal TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalConnection, bool async) { // Construct a MARS session 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 5b88a55511..76d2004ea5 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 @@ -67,22 +67,6 @@ internal int ObjectID // Constructors // ////////////////// - internal TdsParserStateObject(TdsParser parser) - { - // Construct a physical connection - Debug.Assert(null != parser, "no parser?"); - _parser = parser; - - // For physical connection, initialize to default login packet size. - SetPacketSize(TdsEnums.DEFAULT_LOGIN_PACKET_SIZE); - - // we post a callback that represents the call to dispose; once the - // object is disposed, the next callback will cause the GC Handle to - // be released. - IncrementPendingCallbacks(); - _lastSuccessfulIOTimer = new LastIOTimer(); - } - internal TdsParserStateObject(TdsParser parser, SNIHandle physicalConnection, bool async) { // Construct a MARS session 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 f5e65c0e5f..e45e88423e 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -131,6 +131,7 @@ public TimeoutState(int value) private volatile int _timeoutIdentityValue; internal volatile bool _attentionSent; // true if we sent an Attention to the server internal volatile bool _attentionSending; + private readonly TimerCallback _onTimeoutAsync; // Below 2 properties are used to enforce timeout delays in code to @@ -279,6 +280,26 @@ public TimeoutState(int value) #endif + ////////////////// + // Constructors // + ////////////////// + + internal TdsParserStateObject(TdsParser parser) + { + // Construct a physical connection + Debug.Assert(null != parser, "no parser?"); + _parser = parser; + _onTimeoutAsync = OnTimeoutAsync; + + // For physical connection, initialize to default login packet size. + SetPacketSize(TdsEnums.DEFAULT_LOGIN_PACKET_SIZE); + + // we post a callback that represents the call to dispose; once the + // object is disposed, the next callback will cause the GC Handle to + // be released. + IncrementPendingCallbacks(); + _lastSuccessfulIOTimer = new LastIOTimer(); + } //////////////// From 927e4a6cd39ccfe6343488d8289c43012626d715 Mon Sep 17 00:00:00 2001 From: panoskj Date: Mon, 21 Feb 2022 13:37:22 +0200 Subject: [PATCH 05/16] Merging some methods of TdsParserStateObject. --- .../Data/SqlClient/TdsParserStateObject.cs | 92 ------------------ .../Data/SqlClient/TdsParserStateObject.cs | 96 +------------------ .../Data/SqlClient/TdsParserStateObject.cs | 91 ++++++++++++++++++ 3 files changed, 95 insertions(+), 184 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 62fb805ea4..36c187bf61 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 @@ -276,36 +276,6 @@ internal bool Deactivate() return goodForReuse; } - // If this object is part of a TdsParserSessionPool, then this *must* be called inside the pool's lock - internal void RemoveOwner() - { - if (_parser.MARSOn) - { - // We only care about the activation count for MARS connections - int result = Interlocked.Decrement(ref _activateCount); // must have non-zero activation count for reclamation to work too. - Debug.Assert(result == 0, "invalid deactivate count"); - } - Owner = null; - } - - internal void DecrementOpenResultCount() - { - if (_executedUnderTransaction == null) - { - // If we were not executed under a transaction - decrement the global count - // on the parser. - SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObject.DecrementOpenResultCount | INFO | State Object Id {0}, Processing Attention.", _objectID); - _parser.DecrementNonTransactedOpenResultCount(); - } - else - { - // If we were executed under a transaction - decrement the count on the transaction. - _executedUnderTransaction.DecrementAndObtainOpenResultCount(); - _executedUnderTransaction = null; - } - HasOpenResult = false; - } - internal int DecrementPendingCallbacks(bool release) { int remaining = Interlocked.Decrement(ref _pendingCallbacks); @@ -317,48 +287,6 @@ internal int DecrementPendingCallbacks(bool release) return remaining; } - internal void DisposeCounters() - { - Timer networkPacketTimeout = _networkPacketTimeout; - if (networkPacketTimeout != null) - { - _networkPacketTimeout = null; - networkPacketTimeout.Dispose(); - } - - Debug.Assert(Volatile.Read(ref _readingCount) >= 0, "_readingCount is negative"); - if (Volatile.Read(ref _readingCount) > 0) - { - // if _reading is true, we need to wait for it to complete - // if _reading is false, then future read attempts will - // already see the null _sessionHandle and abort. - - // We block after nulling _sessionHandle but before disposing it - // to give a chance for a read that has already grabbed the - // handle to complete. - SpinWait.SpinUntil(() => Volatile.Read(ref _readingCount) == 0); - } - } - - internal int IncrementAndObtainOpenResultCount(SqlInternalTransaction transaction) - { - HasOpenResult = true; - - if (transaction == null) - { - // If we are not passed a transaction, we are not executing under a transaction - // and thus we should increment the global connection result count. - return _parser.IncrementNonTransactedOpenResultCount(); - } - else - { - // If we are passed a transaction, we are executing under a transaction - // and thus we should increment the transaction's result count. - _executedUnderTransaction = transaction; - return transaction.IncrementAndObtainOpenResultCount(); - } - } - internal int IncrementPendingCallbacks() { int remaining = Interlocked.Increment(ref _pendingCallbacks); @@ -367,26 +295,6 @@ internal int IncrementPendingCallbacks() return remaining; } - internal void SetTimeoutSeconds(int timeout) - { - SetTimeoutMilliseconds((long)timeout * 1000L); - } - - internal void SetTimeoutMilliseconds(long timeout) - { - if (timeout <= 0) - { - // 0 or less (i.e. Timespan.Infinite) == infinite (which is represented by Int64.MaxValue) - _timeoutMilliseconds = 0; - _timeoutTime = long.MaxValue; - } - else - { - _timeoutMilliseconds = timeout; - _timeoutTime = 0; - } - } - internal void StartSession(object cancellationOwner) { _cancellationOwner.Target = cancellationOwner; 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 76d2004ea5..38ec15d836 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 @@ -115,12 +115,10 @@ internal SNIHandle Handle internal bool HasOpenResult { - get - { - return _hasOpenResult; - } + get => _hasOpenResult; + set => _hasOpenResult = value; } - + internal UInt32 Status { get @@ -354,35 +352,6 @@ internal bool Deactivate() return goodForReuse; } - // If this object is part of a TdsParserSessionPool, then this *must* be called inside the pool's lock - internal void RemoveOwner() - { - if (_parser.MARSOn) - { - // We only care about the activation count for MARS connections - int result = Interlocked.Decrement(ref _activateCount); // must have non-zero activation count for reclaimation to work too. - Debug.Assert(result == 0, "invalid deactivate count"); - } - Owner = null; - } - - internal void DecrementOpenResultCount() - { - if (_executedUnderTransaction == null) - { - // If we were not executed under a transaction - decrement the global count - // on the parser. - _parser.DecrementNonTransactedOpenResultCount(); - } - else - { - // If we were executed under a transaction - decrement the count on the transaction. - _executedUnderTransaction.DecrementAndObtainOpenResultCount(); - _executedUnderTransaction = null; - } - _hasOpenResult = false; - } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] internal int DecrementPendingCallbacks(bool release) { @@ -411,25 +380,7 @@ internal void Dispose() _sessionHandle = null; _sniAsyncAttnPacket = null; - Timer networkPacketTimeout = _networkPacketTimeout; - if (networkPacketTimeout != null) - { - _networkPacketTimeout = null; - networkPacketTimeout.Dispose(); - } - - Debug.Assert(Volatile.Read(ref _readingCount) >= 0, "_readingCount is negative"); - if (Volatile.Read(ref _readingCount) > 0) - { - // if _reading is true, we need to wait for it to complete - // if _reading is false, then future read attempts will - // already see the null _sessionHandle and abort. - - // We block after nulling _sessionHandle but before disposing it - // to give a chance for a read that has already grabbed the - // handle to complete. - SpinWait.SpinUntil(() => Volatile.Read(ref _readingCount) == 0); - } + DisposeCounters(); if (null != sessionHandle || null != packetHandle) { @@ -476,25 +427,6 @@ internal void Dispose() } } - internal Int32 IncrementAndObtainOpenResultCount(SqlInternalTransaction transaction) - { - _hasOpenResult = true; - - if (transaction == null) - { - // If we are not passed a transaction, we are not executing under a transaction - // and thus we should increment the global connection result count. - return _parser.IncrementNonTransactedOpenResultCount(); - } - else - { - // If we are passed a transaction, we are executing under a transaction - // and thus we should increment the transaction's result count. - _executedUnderTransaction = transaction; - return transaction.IncrementAndObtainOpenResultCount(); - } - } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] internal int IncrementPendingCallbacks() { @@ -505,26 +437,6 @@ internal int IncrementPendingCallbacks() return remaining; } - internal void SetTimeoutSeconds(int timeout) - { - SetTimeoutMilliseconds((long)timeout * 1000L); - } - - internal void SetTimeoutMilliseconds(long timeout) - { - if (timeout <= 0) - { - // 0 or less (i.e. Timespan.Infinite) == infinite (which is represented by Int64.MaxValue) - _timeoutMilliseconds = 0; - _timeoutTime = Int64.MaxValue; - } - else - { - _timeoutMilliseconds = timeout; - _timeoutTime = 0; - } - } - internal void StartSession(int objectID) { _allowObjectID = objectID; 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 e45e88423e..4e2891ae35 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -608,5 +608,96 @@ internal void CloseSession() Parser.PutSession(this); } + // If this object is part of a TdsParserSessionPool, then this *must* be called inside the pool's lock + internal void RemoveOwner() + { + if (_parser.MARSOn) + { + // We only care about the activation count for MARS connections + int result = Interlocked.Decrement(ref _activateCount); // must have non-zero activation count for reclamation to work too. + Debug.Assert(result == 0, "invalid deactivate count"); + } + Owner = null; + } + + internal void DecrementOpenResultCount() + { + if (_executedUnderTransaction == null) + { + // If we were not executed under a transaction - decrement the global count + // on the parser. + SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObject.DecrementOpenResultCount | INFO | State Object Id {0}, Processing Attention.", _objectID); + _parser.DecrementNonTransactedOpenResultCount(); + } + else + { + // If we were executed under a transaction - decrement the count on the transaction. + _executedUnderTransaction.DecrementAndObtainOpenResultCount(); + _executedUnderTransaction = null; + } + HasOpenResult = false; + } + + internal void DisposeCounters() + { + Timer networkPacketTimeout = _networkPacketTimeout; + if (networkPacketTimeout != null) + { + _networkPacketTimeout = null; + networkPacketTimeout.Dispose(); + } + + Debug.Assert(Volatile.Read(ref _readingCount) >= 0, "_readingCount is negative"); + if (Volatile.Read(ref _readingCount) > 0) + { + // if _reading is true, we need to wait for it to complete + // if _reading is false, then future read attempts will + // already see the null _sessionHandle and abort. + + // We block after nulling _sessionHandle but before disposing it + // to give a chance for a read that has already grabbed the + // handle to complete. + SpinWait.SpinUntil(() => Volatile.Read(ref _readingCount) == 0); + } + } + + internal int IncrementAndObtainOpenResultCount(SqlInternalTransaction transaction) + { + HasOpenResult = true; + + if (transaction == null) + { + // If we are not passed a transaction, we are not executing under a transaction + // and thus we should increment the global connection result count. + return _parser.IncrementNonTransactedOpenResultCount(); + } + else + { + // If we are passed a transaction, we are executing under a transaction + // and thus we should increment the transaction's result count. + _executedUnderTransaction = transaction; + return transaction.IncrementAndObtainOpenResultCount(); + } + } + + internal void SetTimeoutSeconds(int timeout) + { + SetTimeoutMilliseconds((long)timeout * 1000L); + } + + internal void SetTimeoutMilliseconds(long timeout) + { + if (timeout <= 0) + { + // 0 or less (i.e. Timespan.Infinite) == infinite (which is represented by Int64.MaxValue) + _timeoutMilliseconds = 0; + _timeoutTime = long.MaxValue; + } + else + { + _timeoutMilliseconds = timeout; + _timeoutTime = 0; + } + } } } From 2c81d54b8ac737622b7f0f5b5c5cf0f951a03d0b Mon Sep 17 00:00:00 2001 From: panoskj Date: Mon, 21 Feb 2022 13:55:02 +0200 Subject: [PATCH 06/16] Merging ObjectID property of TdsParserStateObject. --- .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 1 - .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 8 -------- .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 ++ 3 files changed, 2 insertions(+), 9 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 36c187bf61..5df39c1787 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 @@ -3166,7 +3166,6 @@ internal int WarningCount } protected abstract PacketHandle EmptyReadPacket { get; } - internal int ObjectID => _objectID; internal int PreAttentionErrorCount { 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 38ec15d836..d9d1a36522 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 @@ -19,14 +19,6 @@ namespace Microsoft.Data.SqlClient { internal partial class TdsParserStateObject { - internal int ObjectID - { - get - { - return _objectID; - } - } - private SNIHandle _sessionHandle = null; // the SNI handle we're to work on internal bool _pendingData = false; 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 4e2891ae35..72356e6fc6 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -306,6 +306,8 @@ internal TdsParserStateObject(TdsParser parser) // Properties // //////////////// + internal int ObjectID => _objectID; + // BcpLock - use to lock this object if there is a potential risk of using this object // between tds packets internal bool BcpLock From 417a6daadd675e7ff4072a85304f34e0816c7a74 Mon Sep 17 00:00:00 2001 From: panoskj Date: Fri, 18 Mar 2022 16:51:19 +0200 Subject: [PATCH 07/16] Merging some methods of TdsParserStateObject (note: ports part of #1060 from netcore to common file). --- .../Data/SqlClient/TdsParserStateObject.cs | 45 ------------------- .../src/Microsoft/Data/SqlClient/SqlUtil.cs | 22 +++++++++ .../Data/SqlClient/TdsParserStateObject.cs | 42 +++-------------- .../Data/SqlClient/TdsParserStateObject.cs | 45 +++++++++++++++++++ 4 files changed, 73 insertions(+), 81 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 5df39c1787..f534234781 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 @@ -300,51 +300,6 @@ internal void StartSession(object cancellationOwner) _cancellationOwner.Target = cancellationOwner; } - internal void ThrowExceptionAndWarning(bool callerHasConnectionLock = false, bool asyncClose = false) - { - _parser.ThrowExceptionAndWarning(this, callerHasConnectionLock, asyncClose); - } - - //////////////////////////////////////////// - // TDS Packet/buffer manipulation methods // - //////////////////////////////////////////// - - internal Task ExecuteFlush() - { - lock (this) - { - if (_cancelled && 1 == _outputPacketNumber) - { - ResetBuffer(); - _cancelled = false; - throw SQL.OperationCancelled(); - } - else - { - Task writePacketTask = WritePacket(TdsEnums.HARDFLUSH); - if (writePacketTask == null) - { - HasPendingData = true; - _messageStatus = 0; - return null; - } - else - { - return AsyncHelper.CreateContinuationTaskWithState( - task: writePacketTask, - state: this, - onSuccess: static (object state) => - { - TdsParserStateObject stateObject = (TdsParserStateObject)state; - stateObject.HasPendingData = true; - stateObject._messageStatus = 0; - } - ); - } - } - } - } - // Processes the tds header that is present in the buffer internal bool TryProcessHeader() { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs index 6c60e9e009..bae37ce6bf 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs @@ -46,6 +46,28 @@ 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: (object 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); 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 d9d1a36522..f078ba6b5a 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 @@ -111,6 +111,12 @@ internal bool HasOpenResult set => _hasOpenResult = value; } + internal bool HasPendingData + { + get => _pendingData; + set => _pendingData = value; + } + internal UInt32 Status { get @@ -434,42 +440,6 @@ internal void StartSession(int objectID) _allowObjectID = objectID; } - internal void ThrowExceptionAndWarning(bool callerHasConnectionLock = false, bool asyncClose = false) - { - _parser.ThrowExceptionAndWarning(this, callerHasConnectionLock, asyncClose); - } - - //////////////////////////////////////////// - // TDS Packet/buffer manipulation methods // - //////////////////////////////////////////// - - internal Task ExecuteFlush() - { - lock (this) - { - if (_cancelled && 1 == _outputPacketNumber) - { - ResetBuffer(); - _cancelled = false; - throw SQL.OperationCancelled(); - } - else - { - Task writePacketTask = WritePacket(TdsEnums.HARDFLUSH); - if (writePacketTask == null) - { - _pendingData = true; - _messageStatus = 0; - return null; - } - else - { - return AsyncHelper.CreateContinuationTask(writePacketTask, () => { _pendingData = true; _messageStatus = 0; }); - } - } - } - } - // Processes the tds header that is present in the buffer internal bool TryProcessHeader() { 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 72356e6fc6..e245a969be 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -701,5 +701,50 @@ internal void SetTimeoutMilliseconds(long timeout) _timeoutTime = 0; } } + + internal void ThrowExceptionAndWarning(bool callerHasConnectionLock = false, bool asyncClose = false) + { + _parser.ThrowExceptionAndWarning(this, callerHasConnectionLock, asyncClose); + } + + //////////////////////////////////////////// + // TDS Packet/buffer manipulation methods // + //////////////////////////////////////////// + + internal Task ExecuteFlush() + { + lock (this) + { + if (_cancelled && 1 == _outputPacketNumber) + { + ResetBuffer(); + _cancelled = false; + throw SQL.OperationCancelled(); + } + else + { + Task writePacketTask = WritePacket(TdsEnums.HARDFLUSH); + if (writePacketTask == null) + { + HasPendingData = true; + _messageStatus = 0; + return null; + } + else + { + return AsyncHelper.CreateContinuationTaskWithState( + task: writePacketTask, + state: this, + onSuccess: static (object state) => + { + TdsParserStateObject stateObject = (TdsParserStateObject)state; + stateObject.HasPendingData = true; + stateObject._messageStatus = 0; + } + ); + } + } + } + } } } From b8eb31dbd73d348a15f61c5a0a9c89dfff10fdb2 Mon Sep 17 00:00:00 2001 From: panoskj Date: Fri, 18 Mar 2022 18:25:49 +0200 Subject: [PATCH 08/16] Merging some methods of TdsParserStateObject (note: uses some #if directives). --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 2 + .../Data/SqlClient/TdsParserStateObject.cs | 268 ---------------- .../Data/SqlClient/TdsParserStateObject.cs | 275 ----------------- .../Data/SqlClient/TdsParserStateObject.cs | 286 ++++++++++++++++++ 4 files changed, 288 insertions(+), 543 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index 5986bd3e59..9521e3b154 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -257,6 +257,8 @@ internal EncryptionOptions EncryptionOptions } } + internal bool Is2005OrNewer => true; + internal bool Is2008OrNewer { get 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 f534234781..c29b806793 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 @@ -243,39 +243,6 @@ internal abstract void CreatePhysicalSNIHandle(string serverName, bool ignoreSni internal abstract uint GenerateSspiClientContext(byte[] receivedBuff, uint receivedLength, ref byte[] sendBuff, ref uint sendLength, byte[][] _sniSpnBuffer); - internal bool Deactivate() - { - bool goodForReuse = false; - - try - { - TdsParserState state = Parser.State; - if (state != TdsParserState.Broken && state != TdsParserState.Closed) - { - if (HasPendingData) - { - Parser.DrainData(this); // This may throw - taking us to catch block.c - } - - if (HasOpenResult) - { - DecrementOpenResultCount(); - } - - ResetCancelAndProcessAttention(); - goodForReuse = true; - } - } - catch (Exception e) - { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - } - return goodForReuse; - } - internal int DecrementPendingCallbacks(bool release) { int remaining = Interlocked.Decrement(ref _pendingCallbacks); @@ -300,166 +267,6 @@ internal void StartSession(object cancellationOwner) _cancellationOwner.Target = cancellationOwner; } - // Processes the tds header that is present in the buffer - internal bool TryProcessHeader() - { - Debug.Assert(_inBytesPacket == 0, "there should not be any bytes left in packet when ReadHeader is called"); - - // if the header splits buffer reads - special case! - if ((_partialHeaderBytesRead > 0) || (_inBytesUsed + _inputHeaderLen > _inBytesRead)) - { - // VSTS 219884: when some kind of MITM (man-in-the-middle) tool splits the network packets, the message header can be split over - // several network packets. - // Note: cannot use ReadByteArray here since it uses _inBytesPacket which is not set yet. - do - { - int copy = Math.Min(_inBytesRead - _inBytesUsed, _inputHeaderLen - _partialHeaderBytesRead); - Debug.Assert(copy > 0, "ReadNetworkPacket read empty buffer"); - - Buffer.BlockCopy(_inBuff, _inBytesUsed, _partialHeaderBuffer, _partialHeaderBytesRead, copy); - _partialHeaderBytesRead += copy; - _inBytesUsed += copy; - - Debug.Assert(_partialHeaderBytesRead <= _inputHeaderLen, "Read more bytes for header than required"); - if (_partialHeaderBytesRead == _inputHeaderLen) - { - // All read - _partialHeaderBytesRead = 0; - _inBytesPacket = ((int)_partialHeaderBuffer[TdsEnums.HEADER_LEN_FIELD_OFFSET] << 8 | - (int)_partialHeaderBuffer[TdsEnums.HEADER_LEN_FIELD_OFFSET + 1]) - _inputHeaderLen; - - _messageStatus = _partialHeaderBuffer[1]; - _spid = _partialHeaderBuffer[TdsEnums.SPID_OFFSET] << 8 | - _partialHeaderBuffer[TdsEnums.SPID_OFFSET + 1]; - SqlClientEventSource.Log.TryAdvancedTraceEvent("TdsParserStateObject.TryProcessHeader | ADV | State Object Id {0}, Client Connection Id {1}, Server process Id (SPID) {2}", _objectID, _parser?.Connection?.ClientConnectionId, _spid); - } - else - { - Debug.Assert(_inBytesUsed == _inBytesRead, "Did not use all data while reading partial header"); - - // Require more data - if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed) - { - // NOTE: ReadNetworkPacket does nothing if the parser state is closed or broken - // to avoid infinite loop, we raise an exception - ThrowExceptionAndWarning(); - return true; - } - - if (!TryReadNetworkPacket()) - { - return false; - } - - if (IsTimeoutStateExpired) - { - ThrowExceptionAndWarning(); - return true; - } - } - } while (_partialHeaderBytesRead != 0); // This is reset to 0 once we have read everything that we need - - AssertValidState(); - } - else - { - // normal header processing... - _messageStatus = _inBuff[_inBytesUsed + 1]; - _inBytesPacket = (_inBuff[_inBytesUsed + TdsEnums.HEADER_LEN_FIELD_OFFSET] << 8 | - _inBuff[_inBytesUsed + TdsEnums.HEADER_LEN_FIELD_OFFSET + 1]) - _inputHeaderLen; - _spid = _inBuff[_inBytesUsed + TdsEnums.SPID_OFFSET] << 8 | - _inBuff[_inBytesUsed + TdsEnums.SPID_OFFSET + 1]; - SqlClientEventSource.Log.TryAdvancedTraceEvent("TdsParserStateObject.TryProcessHeader | ADV | State Object Id {0}, Client Connection Id {1}, Server process Id (SPID) {2}", _objectID, _parser?.Connection?.ClientConnectionId, _spid); - _inBytesUsed += _inputHeaderLen; - - AssertValidState(); - } - - if (_inBytesPacket < 0) - { - // either TDS stream is corrupted or there is multithreaded misuse of connection - throw SQL.ParsingError(); - } - - return true; - } - - // This ensure that there is data available to be read in the buffer and that the header has been processed - // NOTE: This method (and all it calls) should be retryable without replaying a snapshot - internal bool TryPrepareBuffer() - { - Debug.Assert(_inBuff != null, "packet buffer should not be null!"); - - // Header spans packets, or we haven't read the header yet - process header - if ((_inBytesPacket == 0) && (_inBytesUsed < _inBytesRead)) - { - if (!TryProcessHeader()) - { - return false; - } - Debug.Assert(_inBytesPacket != 0, "_inBytesPacket cannot be 0 after processing header!"); - AssertValidState(); - } - - // If we're out of data, need to read more - if (_inBytesUsed == _inBytesRead) - { - // If the _inBytesPacket is not zero, then we have data left in the packet, but the data in the packet - // spans the buffer, so we can read any amount of data possible, and we do not need to call ProcessHeader - // because there isn't a header at the beginning of the data that we are reading. - if (_inBytesPacket > 0) - { - if (!TryReadNetworkPacket()) - { - return false; - } - } - else if (_inBytesPacket == 0) - { - // Else we have finished the packet and so we must read as much data as possible - if (!TryReadNetworkPacket()) - { - return false; - } - - if (!TryProcessHeader()) - { - return false; - } - - Debug.Assert(_inBytesPacket != 0, "_inBytesPacket cannot be 0 after processing header!"); - if (_inBytesUsed == _inBytesRead) - { - // we read a header but didn't get anything else except it - // VSTS 219884: it can happen that the TDS packet header and its data are split across two network packets. - // Read at least one more byte to get/cache the first data portion of this TDS packet - if (!TryReadNetworkPacket()) - { - return false; - } - } - } - else - { - Debug.Fail("entered negative _inBytesPacket loop"); - } - AssertValidState(); - } - - return true; - } - - internal void ResetBuffer() - { - _outBytesUsed = _outputHeaderLen; - } - - internal void ResetPacketCounters() - { - _outputPacketNumber = 1; - _outputPacketCount = 0; - } - private void SetSnapshottedState(SnapshottedStateFlags flag, bool value) { if (value) @@ -507,81 +314,6 @@ internal bool HasReceivedColumnMetadata set => SetSnapshottedState(SnapshottedStateFlags.ColMetaDataReceived, value); } - - internal bool SetPacketSize(int size) - { - if (size > TdsEnums.MAX_PACKET_SIZE) - { - throw SQL.InvalidPacketSize(); - } - Debug.Assert(size >= 1, "Cannot set packet size to less than 1."); - Debug.Assert((_outBuff == null && _inBuff == null) || - (_outBuff.Length == _inBuff.Length), - "Buffers are not in consistent state"); - Debug.Assert((_outBuff == null && _inBuff == null) || - this == _parser._physicalStateObj, - "SetPacketSize should only be called on a stateObj with null buffers on the physicalStateObj!"); - Debug.Assert(_inBuff == null - || ( - _outBytesUsed == (_outputHeaderLen + BitConverter.ToInt32(_outBuff, _outputHeaderLen)) && - _outputPacketNumber == 1) - || - (_outBytesUsed == _outputHeaderLen && _outputPacketNumber == 1), - "SetPacketSize called with data in the buffer!"); - - if (_inBuff == null || _inBuff.Length != size) - { // We only check _inBuff, since two buffers should be consistent. - // Allocate or re-allocate _inBuff. - if (_inBuff == null) - { - _inBuff = new byte[size]; - _inBytesRead = 0; - _inBytesUsed = 0; - } - else if (size != _inBuff.Length) - { - // If new size is other than existing... - if (_inBytesRead > _inBytesUsed) - { - // if we still have data left in the buffer we must keep that array reference and then copy into new one - byte[] temp = _inBuff; - - _inBuff = new byte[size]; - - // copy remainder of unused data - int remainingData = _inBytesRead - _inBytesUsed; - if ((temp.Length < _inBytesUsed + remainingData) || (_inBuff.Length < remainingData)) - { - string errormessage = StringsHelper.GetString(Strings.SQL_InvalidInternalPacketSize) + ' ' + temp.Length + ", " + _inBytesUsed + ", " + remainingData + ", " + _inBuff.Length; - throw SQL.InvalidInternalPacketSize(errormessage); - } - Buffer.BlockCopy(temp, _inBytesUsed, _inBuff, 0, remainingData); - - _inBytesRead = _inBytesRead - _inBytesUsed; - _inBytesUsed = 0; - - AssertValidState(); - } - else - { - // buffer is empty - just create the new one that is double the size of the old one - _inBuff = new byte[size]; - _inBytesRead = 0; - _inBytesUsed = 0; - } - } - - // Always re-allocate _outBuff - assert is above to verify state. - _outBuff = new byte[size]; - _outBytesUsed = _outputHeaderLen; - - AssertValidState(); - return true; - } - - return false; - } - /////////////////////////////////////// // Buffer read methods - data values // /////////////////////////////////////// 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 f078ba6b5a..a4290af722 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 @@ -316,40 +316,6 @@ internal void CreatePhysicalSNIHandle(string serverName, bool ignoreSniOpenTimeo out instanceName, flushCache, !async, fParallel, transparentNetworkResolutionState, totalTimeout, ipPreference, cachedDNSInfo); } - internal bool Deactivate() - { - bool goodForReuse = false; - - try - { - TdsParserState state = Parser.State; - if (state != TdsParserState.Broken && state != TdsParserState.Closed) - { - if (_pendingData) - { - Parser.DrainData(this); // This may throw - taking us to catch block. - } - - if (HasOpenResult) - { // SQL BU DT 383773 - need to decrement openResultCount for all pending operations. - DecrementOpenResultCount(); - } - - ResetCancelAndProcessAttention(); - goodForReuse = true; - } - } - catch (Exception e) - { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - ADP.TraceExceptionWithoutRethrow(e); - } - return goodForReuse; - } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] internal int DecrementPendingCallbacks(bool release) { @@ -440,247 +406,6 @@ internal void StartSession(int objectID) _allowObjectID = objectID; } - // Processes the tds header that is present in the buffer - internal bool TryProcessHeader() - { - - Debug.Assert(_inBytesPacket == 0, "there should not be any bytes left in packet when ReadHeader is called"); - - // if the header splits buffer reads - special case! - if ((_partialHeaderBytesRead > 0) || (_inBytesUsed + _inputHeaderLen > _inBytesRead)) - { - // VSTS 219884: when some kind of MITM (man-in-the-middle) tool splits the network packets, the message header can be split over - // several network packets. - // Note: cannot use ReadByteArray here since it uses _inBytesPacket which is not set yet. - do - { - int copy = Math.Min(_inBytesRead - _inBytesUsed, _inputHeaderLen - _partialHeaderBytesRead); - Debug.Assert(copy > 0, "ReadNetworkPacket read empty buffer"); - - Buffer.BlockCopy(_inBuff, _inBytesUsed, _partialHeaderBuffer, _partialHeaderBytesRead, copy); - _partialHeaderBytesRead += copy; - _inBytesUsed += copy; - - Debug.Assert(_partialHeaderBytesRead <= _inputHeaderLen, "Read more bytes for header than required"); - if (_partialHeaderBytesRead == _inputHeaderLen) - { - // All read - _partialHeaderBytesRead = 0; - _inBytesPacket = ((int)_partialHeaderBuffer[TdsEnums.HEADER_LEN_FIELD_OFFSET] << 8 | - (int)_partialHeaderBuffer[TdsEnums.HEADER_LEN_FIELD_OFFSET + 1]) - _inputHeaderLen; - - _messageStatus = _partialHeaderBuffer[1]; - _spid = _partialHeaderBuffer[TdsEnums.SPID_OFFSET] << 8 | - _partialHeaderBuffer[TdsEnums.SPID_OFFSET + 1]; - } - else - { - Debug.Assert(_inBytesUsed == _inBytesRead, "Did not use all data while reading partial header"); - - // Require more data - if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed) - { - // NOTE: ReadNetworkPacket does nothing if the parser state is closed or broken - // to avoid infinite loop, we raise an exception - ThrowExceptionAndWarning(); - // TODO: ThrowExceptionAndWarning will not raise exception if user asked to report errors as warnings - return true; - } - - if (!TryReadNetworkPacket()) - { - return false; - } - - if (IsTimeoutStateExpired) - { - ThrowExceptionAndWarning(); - // TODO: see the comment above - return true; - } - } - } while (_partialHeaderBytesRead != 0); // This is reset to 0 once we have read everything that we need - - AssertValidState(); - } - else - { - // normal header processing... - _messageStatus = _inBuff[_inBytesUsed + 1]; - _inBytesPacket = (_inBuff[_inBytesUsed + TdsEnums.HEADER_LEN_FIELD_OFFSET] << 8 | - _inBuff[_inBytesUsed + TdsEnums.HEADER_LEN_FIELD_OFFSET + 1]) - _inputHeaderLen; - _spid = _inBuff[_inBytesUsed + TdsEnums.SPID_OFFSET] << 8 | - _inBuff[_inBytesUsed + TdsEnums.SPID_OFFSET + 1]; - _inBytesUsed += _inputHeaderLen; - - AssertValidState(); - } - - if (_inBytesPacket < 0) - { - // either TDS stream is corrupted or there is multithreaded misuse of connection - // NOTE: usually we do not proactively apply checks to TDS data, but this situation happened several times - // and caused infinite loop in CleanWire (VSTFDEVDIV\DEVDIV2:149937) - throw SQL.ParsingError(ParsingErrorState.CorruptedTdsStream); - } - - return true; - } - - // This ensure that there is data available to be read in the buffer and that the header has been processed - // NOTE: This method (and all it calls) should be retryable without replaying a snapshot - internal bool TryPrepareBuffer() - { - TdsParser.ReliabilitySection.Assert("unreliable call to ReadBuffer"); // you need to setup for a thread abort somewhere before you call this method - Debug.Assert(_inBuff != null, "packet buffer should not be null!"); - - // Header spans packets, or we haven't read the header yet - process header - if ((_inBytesPacket == 0) && (_inBytesUsed < _inBytesRead)) - { - if (!TryProcessHeader()) - { - return false; - } - Debug.Assert(_inBytesPacket != 0, "_inBytesPacket cannot be 0 after processing header!"); - AssertValidState(); - } - - // If we're out of data, need to read more - if (_inBytesUsed == _inBytesRead) - { - // If the _inBytesPacket is not zero, then we have data left in the packet, but the data in the packet - // spans the buffer, so we can read any amount of data possible, and we do not need to call ProcessHeader - // because there isn't a header at the beginning of the data that we are reading. - if (_inBytesPacket > 0) - { - if (!TryReadNetworkPacket()) - { - return false; - } - - } - else if (_inBytesPacket == 0) - { - // Else we have finished the packet and so we must read as much data as possible - if (!TryReadNetworkPacket()) - { - return false; - } - - if (!TryProcessHeader()) - { - return false; - } - - Debug.Assert(_inBytesPacket != 0, "_inBytesPacket cannot be 0 after processing header!"); - if (_inBytesUsed == _inBytesRead) - { - // we read a header but didn't get anything else except it - // VSTS 219884: it can happen that the TDS packet header and its data are split across two network packets. - // Read at least one more byte to get/cache the first data portion of this TDS packet - if (!TryReadNetworkPacket()) - { - return false; - } - } - - } - else - { - Debug.Fail("entered negative _inBytesPacket loop"); - } - AssertValidState(); - } - - return true; - } - - internal void ResetBuffer() - { - _outBytesUsed = _outputHeaderLen; - } - - internal void ResetPacketCounters() - { - _outputPacketNumber = 1; - _outputPacketCount = 0; - } - - internal bool SetPacketSize(int size) - { - if (size > TdsEnums.MAX_PACKET_SIZE) - { - throw SQL.InvalidPacketSize(); - } - Debug.Assert(size >= 1, "Cannot set packet size to less than 1."); - Debug.Assert((_outBuff == null && _inBuff == null) || - (_outBuff.Length == _inBuff.Length), - "Buffers are not in consistent state"); - Debug.Assert((_outBuff == null && _inBuff == null) || - this == _parser._physicalStateObj, - "SetPacketSize should only be called on a stateObj with null buffers on the physicalStateObj!"); - Debug.Assert(_inBuff == null - || - (_parser.Is2005OrNewer && - _outBytesUsed == (_outputHeaderLen + BitConverter.ToInt32(_outBuff, _outputHeaderLen)) && - _outputPacketNumber == 1) - || - (_outBytesUsed == _outputHeaderLen && _outputPacketNumber == 1), - "SetPacketSize called with data in the buffer!"); - - if (_inBuff == null || _inBuff.Length != size) - { // We only check _inBuff, since two buffers should be consistent. - // Allocate or re-allocate _inBuff. - if (_inBuff == null) - { - _inBuff = new byte[size]; - _inBytesRead = 0; - _inBytesUsed = 0; - } - else if (size != _inBuff.Length) - { - // If new size is other than existing... - if (_inBytesRead > _inBytesUsed) - { - // if we still have data left in the buffer we must keep that array reference and then copy into new one - byte[] temp = _inBuff; - - _inBuff = new byte[size]; - - // copy remainder of unused data - int remainingData = _inBytesRead - _inBytesUsed; - if ((temp.Length < _inBytesUsed + remainingData) || (_inBuff.Length < remainingData)) - { - string errormessage = StringsHelper.GetString(Strings.SQL_InvalidInternalPacketSize) + ' ' + temp.Length + ", " + _inBytesUsed + ", " + remainingData + ", " + _inBuff.Length; - throw SQL.InvalidInternalPacketSize(errormessage); - } - Buffer.BlockCopy(temp, _inBytesUsed, _inBuff, 0, remainingData); - - _inBytesRead = _inBytesRead - _inBytesUsed; - _inBytesUsed = 0; - - AssertValidState(); - } - else - { - // buffer is empty - just create the new one that is double the size of the old one - _inBuff = new byte[size]; - _inBytesRead = 0; - _inBytesUsed = 0; - } - } - - // Always re-allocate _outBuff - assert is above to verify state. - _outBuff = new byte[size]; - _outBytesUsed = _outputHeaderLen; - - AssertValidState(); - return true; - } - - return false; - } - /////////////////////////////////////// // Buffer read methods - data values // /////////////////////////////////////// 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 e245a969be..d2f67f301f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -9,6 +9,7 @@ using System.Text; using System.Threading; using System.Threading.Tasks; +using Microsoft.Data.Common; namespace Microsoft.Data.SqlClient { @@ -610,6 +611,42 @@ internal void CloseSession() Parser.PutSession(this); } + internal bool Deactivate() + { + bool goodForReuse = false; + + try + { + TdsParserState state = Parser.State; + if (state != TdsParserState.Broken && state != TdsParserState.Closed) + { + if (HasPendingData) + { + Parser.DrainData(this); // This may throw - taking us to catch block.c + } + + if (HasOpenResult) + { + DecrementOpenResultCount(); + } + + ResetCancelAndProcessAttention(); + goodForReuse = true; + } + } + catch (Exception e) + { + if (!ADP.IsCatchableExceptionType(e)) + { + throw; + } +#if NETFRAMEWORK + ADP.TraceExceptionWithoutRethrow(e); +#endif + } + return goodForReuse; + } + // If this object is part of a TdsParserSessionPool, then this *must* be called inside the pool's lock internal void RemoveOwner() { @@ -746,5 +783,254 @@ internal Task ExecuteFlush() } } } + + // Processes the tds header that is present in the buffer + internal bool TryProcessHeader() + { + Debug.Assert(_inBytesPacket == 0, "there should not be any bytes left in packet when ReadHeader is called"); + + // if the header splits buffer reads - special case! + if ((_partialHeaderBytesRead > 0) || (_inBytesUsed + _inputHeaderLen > _inBytesRead)) + { + // VSTS 219884: when some kind of MITM (man-in-the-middle) tool splits the network packets, the message header can be split over + // several network packets. + // Note: cannot use ReadByteArray here since it uses _inBytesPacket which is not set yet. + do + { + int copy = Math.Min(_inBytesRead - _inBytesUsed, _inputHeaderLen - _partialHeaderBytesRead); + Debug.Assert(copy > 0, "ReadNetworkPacket read empty buffer"); + + Buffer.BlockCopy(_inBuff, _inBytesUsed, _partialHeaderBuffer, _partialHeaderBytesRead, copy); + _partialHeaderBytesRead += copy; + _inBytesUsed += copy; + + Debug.Assert(_partialHeaderBytesRead <= _inputHeaderLen, "Read more bytes for header than required"); + if (_partialHeaderBytesRead == _inputHeaderLen) + { + // All read + _partialHeaderBytesRead = 0; + _inBytesPacket = ((int)_partialHeaderBuffer[TdsEnums.HEADER_LEN_FIELD_OFFSET] << 8 | + (int)_partialHeaderBuffer[TdsEnums.HEADER_LEN_FIELD_OFFSET + 1]) - _inputHeaderLen; + + _messageStatus = _partialHeaderBuffer[1]; + _spid = _partialHeaderBuffer[TdsEnums.SPID_OFFSET] << 8 | + _partialHeaderBuffer[TdsEnums.SPID_OFFSET + 1]; +#if !NETFRAMEWORK + SqlClientEventSource.Log.TryAdvancedTraceEvent("TdsParserStateObject.TryProcessHeader | ADV | State Object Id {0}, Client Connection Id {1}, Server process Id (SPID) {2}", _objectID, _parser?.Connection?.ClientConnectionId, _spid); +#endif + } + else + { + Debug.Assert(_inBytesUsed == _inBytesRead, "Did not use all data while reading partial header"); + + // Require more data + if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed) + { + // NOTE: ReadNetworkPacket does nothing if the parser state is closed or broken + // to avoid infinite loop, we raise an exception + ThrowExceptionAndWarning(); + return true; + } + + if (!TryReadNetworkPacket()) + { + return false; + } + + if (IsTimeoutStateExpired) + { + ThrowExceptionAndWarning(); + return true; + } + } + } while (_partialHeaderBytesRead != 0); // This is reset to 0 once we have read everything that we need + + AssertValidState(); + } + else + { + // normal header processing... + _messageStatus = _inBuff[_inBytesUsed + 1]; + _inBytesPacket = (_inBuff[_inBytesUsed + TdsEnums.HEADER_LEN_FIELD_OFFSET] << 8 | + _inBuff[_inBytesUsed + TdsEnums.HEADER_LEN_FIELD_OFFSET + 1]) - _inputHeaderLen; + _spid = _inBuff[_inBytesUsed + TdsEnums.SPID_OFFSET] << 8 | + _inBuff[_inBytesUsed + TdsEnums.SPID_OFFSET + 1]; +#if !NETFRAMEWORK + SqlClientEventSource.Log.TryAdvancedTraceEvent("TdsParserStateObject.TryProcessHeader | ADV | State Object Id {0}, Client Connection Id {1}, Server process Id (SPID) {2}", _objectID, _parser?.Connection?.ClientConnectionId, _spid); +#endif + _inBytesUsed += _inputHeaderLen; + + AssertValidState(); + } + + if (_inBytesPacket < 0) + { +#if NETFRAMEWORK + throw SQL.ParsingError(ParsingErrorState.CorruptedTdsStream); +#else + // either TDS stream is corrupted or there is multithreaded misuse of connection + throw SQL.ParsingError(); +#endif + } + + return true; + } + + // This ensure that there is data available to be read in the buffer and that the header has been processed + // NOTE: This method (and all it calls) should be retryable without replaying a snapshot + internal bool TryPrepareBuffer() + { +#if NETFRAMEWORK + + TdsParser.ReliabilitySection.Assert("unreliable call to ReadBuffer"); // you need to setup for a thread abort somewhere before you call this method +#endif + Debug.Assert(_inBuff != null, "packet buffer should not be null!"); + + // Header spans packets, or we haven't read the header yet - process header + if ((_inBytesPacket == 0) && (_inBytesUsed < _inBytesRead)) + { + if (!TryProcessHeader()) + { + return false; + } + Debug.Assert(_inBytesPacket != 0, "_inBytesPacket cannot be 0 after processing header!"); + AssertValidState(); + } + + // If we're out of data, need to read more + if (_inBytesUsed == _inBytesRead) + { + // If the _inBytesPacket is not zero, then we have data left in the packet, but the data in the packet + // spans the buffer, so we can read any amount of data possible, and we do not need to call ProcessHeader + // because there isn't a header at the beginning of the data that we are reading. + if (_inBytesPacket > 0) + { + if (!TryReadNetworkPacket()) + { + return false; + } + } + else if (_inBytesPacket == 0) + { + // Else we have finished the packet and so we must read as much data as possible + if (!TryReadNetworkPacket()) + { + return false; + } + + if (!TryProcessHeader()) + { + return false; + } + + Debug.Assert(_inBytesPacket != 0, "_inBytesPacket cannot be 0 after processing header!"); + if (_inBytesUsed == _inBytesRead) + { + // we read a header but didn't get anything else except it + // VSTS 219884: it can happen that the TDS packet header and its data are split across two network packets. + // Read at least one more byte to get/cache the first data portion of this TDS packet + if (!TryReadNetworkPacket()) + { + return false; + } + } + } + else + { + Debug.Fail("entered negative _inBytesPacket loop"); + } + AssertValidState(); + } + + return true; + } + + internal void ResetBuffer() + { + _outBytesUsed = _outputHeaderLen; + } + + internal void ResetPacketCounters() + { + _outputPacketNumber = 1; + _outputPacketCount = 0; + } + + internal bool SetPacketSize(int size) + { + if (size > TdsEnums.MAX_PACKET_SIZE) + { + throw SQL.InvalidPacketSize(); + } + Debug.Assert(size >= 1, "Cannot set packet size to less than 1."); + Debug.Assert((_outBuff == null && _inBuff == null) || + (_outBuff.Length == _inBuff.Length), + "Buffers are not in consistent state"); + Debug.Assert((_outBuff == null && _inBuff == null) || + this == _parser._physicalStateObj, + "SetPacketSize should only be called on a stateObj with null buffers on the physicalStateObj!"); + Debug.Assert(_inBuff == null + || + ( + _parser.Is2005OrNewer && + _outBytesUsed == (_outputHeaderLen + BitConverter.ToInt32(_outBuff, _outputHeaderLen)) && + _outputPacketNumber == 1) + || + (_outBytesUsed == _outputHeaderLen && _outputPacketNumber == 1), + "SetPacketSize called with data in the buffer!"); + + if (_inBuff == null || _inBuff.Length != size) + { // We only check _inBuff, since two buffers should be consistent. + // Allocate or re-allocate _inBuff. + if (_inBuff == null) + { + _inBuff = new byte[size]; + _inBytesRead = 0; + _inBytesUsed = 0; + } + else if (size != _inBuff.Length) + { + // If new size is other than existing... + if (_inBytesRead > _inBytesUsed) + { + // if we still have data left in the buffer we must keep that array reference and then copy into new one + byte[] temp = _inBuff; + + _inBuff = new byte[size]; + + // copy remainder of unused data + int remainingData = _inBytesRead - _inBytesUsed; + if ((temp.Length < _inBytesUsed + remainingData) || (_inBuff.Length < remainingData)) + { + string errormessage = StringsHelper.GetString(Strings.SQL_InvalidInternalPacketSize) + ' ' + temp.Length + ", " + _inBytesUsed + ", " + remainingData + ", " + _inBuff.Length; + throw SQL.InvalidInternalPacketSize(errormessage); + } + Buffer.BlockCopy(temp, _inBytesUsed, _inBuff, 0, remainingData); + + _inBytesRead = _inBytesRead - _inBytesUsed; + _inBytesUsed = 0; + + AssertValidState(); + } + else + { + // buffer is empty - just create the new one that is double the size of the old one + _inBuff = new byte[size]; + _inBytesRead = 0; + _inBytesUsed = 0; + } + } + + // Always re-allocate _outBuff - assert is above to verify state. + _outBuff = new byte[size]; + _outBytesUsed = _outputHeaderLen; + + AssertValidState(); + return true; + } + + return false; + } + } } From 4e6ed9ae2fa767b88f0e2a3692daa959cf7ca6ba Mon Sep 17 00:00:00 2001 From: panoskj Date: Wed, 5 Oct 2022 11:24:16 +0300 Subject: [PATCH 09/16] TdsParserStateObject: Fixed naming rules violations. --- .../Data/SqlClient/TdsParserStateObject.cs | 36 +++++++++---------- .../Data/SqlClient/TdsParserStateObject.cs | 36 +++++++++---------- .../Data/SqlClient/TdsParserStateObject.cs | 18 +++++----- 3 files changed, 45 insertions(+), 45 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 60a7db89f4..46fa274787 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 @@ -124,7 +124,7 @@ internal void Cancel(object caller) // Keep looping until we either grabbed the lock (and therefore sent attention) or the connection closes\breaks while ((!hasLock) && (_parser.State != TdsParserState.Closed) && (_parser.State != TdsParserState.Broken)) { - Monitor.TryEnter(this, _waitForCancellationLockPollTimeout, ref hasLock); + Monitor.TryEnter(this, WaitForCancellationLockPollTimeout, ref hasLock); if (hasLock) { // Lock for the time being - since we need to synchronize the attention send. // This lock is also protecting against concurrent close and async continuations @@ -142,7 +142,7 @@ internal void Cancel(object caller) { try { - _parser.Connection._parserLock.Wait(canReleaseFromAnyThread: false, timeout: _waitForCancellationLockPollTimeout, lockTaken: ref hasParserLock); + _parser.Connection._parserLock.Wait(canReleaseFromAnyThread: false, timeout: WaitForCancellationLockPollTimeout, lockTaken: ref hasParserLock); if (hasParserLock) { _parser.Connection.ThreadHasParserLockForClose = true; @@ -368,7 +368,7 @@ public bool TryReadByteArray(Span buff, int len, out int totalRead) _networkPacketTaskSource = new TaskCompletionSource(); Interlocked.MemoryBarrier(); - if (_forcePendingReadsToWaitForUser) + if (s_forcePendingReadsToWaitForUser) { _realNetworkPacketTaskSource = new TaskCompletionSource(); _realNetworkPacketTaskSource.SetResult(null); @@ -427,7 +427,7 @@ internal bool TryReadByte(out byte value) _networkPacketTaskSource = new TaskCompletionSource(); Interlocked.MemoryBarrier(); - if (_forcePendingReadsToWaitForUser) + if (s_forcePendingReadsToWaitForUser) { _realNetworkPacketTaskSource = new TaskCompletionSource(); _realNetworkPacketTaskSource.SetResult(null); @@ -1156,7 +1156,7 @@ internal bool TryReadNetworkPacket() if (_snapshot.Replay()) { #if DEBUG - if (_checkNetworkPacketRetryStacks) + if (s_checkNetworkPacketRetryStacks) { _snapshot.CheckStack(Environment.StackTrace); } @@ -1166,7 +1166,7 @@ internal bool TryReadNetworkPacket() #if DEBUG else { - if (_checkNetworkPacketRetryStacks) + if (s_checkNetworkPacketRetryStacks) { _lastStack = Environment.StackTrace; } @@ -1187,11 +1187,11 @@ internal bool TryReadNetworkPacket() ReadSni(new TaskCompletionSource()); #if DEBUG - if (_failAsyncPends) + if (s_failAsyncPends) { throw new InvalidOperationException("Attempted to pend a read when _failAsyncPends test hook was enabled"); } - if (_forceSyncOverAsyncAfterFirstPend) + if (s_forceSyncOverAsyncAfterFirstPend) { _syncOverAsync = true; } @@ -1241,7 +1241,7 @@ internal void ReadSniSyncOverAsync() ProcessSniPacket(readPacket, 0); #if DEBUG - if (_forcePendingReadsToWaitForUser) + if (s_forcePendingReadsToWaitForUser) { _networkPacketTaskSource = new TaskCompletionSource(); Interlocked.MemoryBarrier(); @@ -1481,7 +1481,7 @@ internal void ReadSni(TaskCompletionSource completion) } #if DEBUG - if (_forcePendingReadsToWaitForUser) + if (s_forcePendingReadsToWaitForUser) { _realNetworkPacketTaskSource = new TaskCompletionSource(); } @@ -1566,7 +1566,7 @@ internal void ReadSni(TaskCompletionSource completion) ReadSniError(this, error); #if DEBUG - if ((_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) + if ((s_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) { _realNetworkPacketTaskSource.TrySetResult(null); } @@ -1904,7 +1904,7 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) TaskCompletionSource source = _networkPacketTaskSource; #if DEBUG - if ((_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) + if ((s_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) { source = _realNetworkPacketTaskSource; } @@ -2610,7 +2610,7 @@ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = fa // This prevents a race condition between receiving the attention ACK and setting _attentionSent _attentionSending = true; #if DEBUG - if (!_skipSendAttention) + if (!s_skipSendAttention) { #endif // Take lock and send attention @@ -3027,7 +3027,7 @@ internal void CompletePendingReadWithSuccess(bool resetForcePendingReadsToWait) var realNetworkPacketTaskSource = _realNetworkPacketTaskSource; var networkPacketTaskSource = _networkPacketTaskSource; - Debug.Assert(_forcePendingReadsToWaitForUser, "Not forcing pends to wait for user - can't force complete"); + Debug.Assert(s_forcePendingReadsToWaitForUser, "Not forcing pends to wait for user - can't force complete"); Debug.Assert(networkPacketTaskSource != null, "No pending read to complete"); try @@ -3044,7 +3044,7 @@ internal void CompletePendingReadWithSuccess(bool resetForcePendingReadsToWait) { if (resetForcePendingReadsToWait) { - _forcePendingReadsToWaitForUser = false; + s_forcePendingReadsToWaitForUser = false; } networkPacketTaskSource.TrySetResult(null); @@ -3057,7 +3057,7 @@ internal void CompletePendingReadWithFailure(int errorCode, bool resetForcePendi var realNetworkPacketTaskSource = _realNetworkPacketTaskSource; var networkPacketTaskSource = _networkPacketTaskSource; - Debug.Assert(_forcePendingReadsToWaitForUser, "Not forcing pends to wait for user - can't force complete"); + Debug.Assert(s_forcePendingReadsToWaitForUser, "Not forcing pends to wait for user - can't force complete"); Debug.Assert(networkPacketTaskSource != null, "No pending read to complete"); try @@ -3074,7 +3074,7 @@ internal void CompletePendingReadWithFailure(int errorCode, bool resetForcePendi { if (resetForcePendingReadsToWait) { - _forcePendingReadsToWaitForUser = false; + s_forcePendingReadsToWaitForUser = false; } AddError(new SqlError(errorCode, 0x00, TdsEnums.FATAL_ERROR_CLASS, _parser.Server, string.Empty, string.Empty, 0)); @@ -3168,7 +3168,7 @@ partial void SetStackInternal(string value) #if DEBUG internal bool DoPend() { - if (_failAsyncPends || !_forceAllPends) + if (s_failAsyncPends || !s_forceAllPends) { return false; } 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 f15d79c55e..06ec369e0d 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 @@ -172,7 +172,7 @@ internal void Cancel(int objectID) while ((!hasLock) && (_parser.State != TdsParserState.Closed) && (_parser.State != TdsParserState.Broken)) { - Monitor.TryEnter(this, _waitForCancellationLockPollTimeout, ref hasLock); + Monitor.TryEnter(this, WaitForCancellationLockPollTimeout, ref hasLock); if (hasLock) { // Lock for the time being - since we need to synchronize the attention send. // At some point in the future, I hope to remove this. @@ -192,7 +192,7 @@ internal void Cancel(int objectID) { try { - _parser.Connection._parserLock.Wait(canReleaseFromAnyThread: false, timeout: _waitForCancellationLockPollTimeout, lockTaken: ref hasParserLock); + _parser.Connection._parserLock.Wait(canReleaseFromAnyThread: false, timeout: WaitForCancellationLockPollTimeout, lockTaken: ref hasParserLock); if (hasParserLock) { _parser.Connection.ThreadHasParserLockForClose = true; @@ -459,7 +459,7 @@ public bool TryReadByteArray(Span buff, int len, out int totalRead) _networkPacketTaskSource = new TaskCompletionSource(); Thread.MemoryBarrier(); - if (_forcePendingReadsToWaitForUser) + if (s_forcePendingReadsToWaitForUser) { _realNetworkPacketTaskSource = new TaskCompletionSource(); _realNetworkPacketTaskSource.SetResult(null); @@ -519,7 +519,7 @@ internal bool TryReadByte(out byte value) _networkPacketTaskSource = new TaskCompletionSource(); Thread.MemoryBarrier(); - if (_forcePendingReadsToWaitForUser) + if (s_forcePendingReadsToWaitForUser) { _realNetworkPacketTaskSource = new TaskCompletionSource(); _realNetworkPacketTaskSource.SetResult(null); @@ -1246,7 +1246,7 @@ internal bool TryReadNetworkPacket() if (_snapshot.Replay()) { #if DEBUG - if (_checkNetworkPacketRetryStacks) + if (s_checkNetworkPacketRetryStacks) { _snapshot.CheckStack(new StackTrace()); } @@ -1257,7 +1257,7 @@ internal bool TryReadNetworkPacket() #if DEBUG else { - if (_checkNetworkPacketRetryStacks) + if (s_checkNetworkPacketRetryStacks) { _lastStack = new StackTrace(); } @@ -1278,11 +1278,11 @@ internal bool TryReadNetworkPacket() ReadSni(new TaskCompletionSource()); #if DEBUG - if (_failAsyncPends) + if (s_failAsyncPends) { throw new InvalidOperationException("Attempted to pend a read when _failAsyncPends test hook was enabled"); } - if (_forceSyncOverAsyncAfterFirstPend) + if (s_forceSyncOverAsyncAfterFirstPend) { _syncOverAsync = true; } @@ -1338,7 +1338,7 @@ internal void ReadSniSyncOverAsync() Debug.Assert(ADP.s_ptrZero != readPacket, "ReadNetworkPacket cannot be null in synchronous operation!"); ProcessSniPacket(readPacket, 0); #if DEBUG - if (_forcePendingReadsToWaitForUser) + if (s_forcePendingReadsToWaitForUser) { _networkPacketTaskSource = new TaskCompletionSource(); Thread.MemoryBarrier(); @@ -1580,7 +1580,7 @@ internal void ReadSni(TaskCompletionSource completion) } #if DEBUG - if (_forcePendingReadsToWaitForUser) + if (s_forcePendingReadsToWaitForUser) { _realNetworkPacketTaskSource = new TaskCompletionSource(); } @@ -1664,7 +1664,7 @@ internal void ReadSni(TaskCompletionSource completion) Debug.Assert(IntPtr.Zero == readPacket, "unexpected readPacket without corresponding SNIPacketRelease"); ReadSniError(this, error); #if DEBUG - if ((_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) + if ((s_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) { _realNetworkPacketTaskSource.TrySetResult(null); } @@ -1999,7 +1999,7 @@ public void ReadAsyncCallback(IntPtr key, IntPtr packet, UInt32 error) TaskCompletionSource source = _networkPacketTaskSource; #if DEBUG - if ((_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) + if ((s_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) { source = _realNetworkPacketTaskSource; } @@ -2680,7 +2680,7 @@ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = fa _attentionSending = true; #if DEBUG - if (!_skipSendAttention) + if (!s_skipSendAttention) { #endif // Take lock and send attention @@ -3191,7 +3191,7 @@ internal void CompletePendingReadWithSuccess(bool resetForcePendingReadsToWait) var realNetworkPacketTaskSource = _realNetworkPacketTaskSource; var networkPacketTaskSource = _networkPacketTaskSource; - Debug.Assert(_forcePendingReadsToWaitForUser, "Not forcing pends to wait for user - can't force complete"); + Debug.Assert(s_forcePendingReadsToWaitForUser, "Not forcing pends to wait for user - can't force complete"); Debug.Assert(networkPacketTaskSource != null, "No pending read to complete"); try @@ -3208,7 +3208,7 @@ internal void CompletePendingReadWithSuccess(bool resetForcePendingReadsToWait) { if (resetForcePendingReadsToWait) { - _forcePendingReadsToWaitForUser = false; + s_forcePendingReadsToWaitForUser = false; } networkPacketTaskSource.TrySetResult(null); @@ -3221,7 +3221,7 @@ internal void CompletePendingReadWithFailure(int errorCode, bool resetForcePendi var realNetworkPacketTaskSource = _realNetworkPacketTaskSource; var networkPacketTaskSource = _networkPacketTaskSource; - Debug.Assert(_forcePendingReadsToWaitForUser, "Not forcing pends to wait for user - can't force complete"); + Debug.Assert(s_forcePendingReadsToWaitForUser, "Not forcing pends to wait for user - can't force complete"); Debug.Assert(networkPacketTaskSource != null, "No pending read to complete"); try @@ -3238,7 +3238,7 @@ internal void CompletePendingReadWithFailure(int errorCode, bool resetForcePendi { if (resetForcePendingReadsToWait) { - _forcePendingReadsToWaitForUser = false; + s_forcePendingReadsToWaitForUser = false; } AddError(new SqlError(errorCode, 0x00, TdsEnums.FATAL_ERROR_CLASS, _parser.Server, string.Empty, string.Empty, 0)); @@ -3305,7 +3305,7 @@ public StateSnapshot(TdsParserStateObject state) internal bool DoPend() { - if (_failAsyncPends || !_forceAllPends) + if (s_failAsyncPends || !s_forceAllPends) { return false; } 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 d2f67f301f..10956fbc74 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -20,8 +20,8 @@ sealed internal class LastIOTimer partial class TdsParserStateObject { - private static int _objectTypeCount; // EventSource counter - internal readonly int _objectID = Interlocked.Increment(ref _objectTypeCount); + private static int s_objectTypeCount; // EventSource counter + internal readonly int _objectID = Interlocked.Increment(ref s_objectTypeCount); [Flags] internal enum SnapshottedStateFlags : byte @@ -161,7 +161,7 @@ public TimeoutState(int value) // attention to the server // 3) post session close - no attention is allowed private bool _cancelled; - private const int _waitForCancellationLockPollTimeout = 100; + private const int WaitForCancellationLockPollTimeout = 100; // Cache the transaction for which this command was executed so upon completion we can // decrement the appropriate result count. @@ -253,30 +253,30 @@ public TimeoutState(int value) // field.SetValue(null, true); // } // - internal static bool _forceAllPends = false; + internal static bool s_forceAllPends = false; // set this while making a call that should not block. // instead of blocking it will fail. - internal static bool _failAsyncPends = false; + internal static bool s_failAsyncPends = false; // If this is set and an async read is made, then // we will switch to syncOverAsync mode for the // remainder of the async operation. - internal static bool _forceSyncOverAsyncAfterFirstPend = false; + internal static bool s_forceSyncOverAsyncAfterFirstPend = false; // Requests to send attention will be ignored when _skipSendAttention is true. // This is useful to simulate circumstances where timeouts do not recover. - internal static bool _skipSendAttention = false; + internal static bool s_skipSendAttention = false; // Prevents any pending read from completing until the user signals it using // CompletePendingReadWithSuccess() or CompletePendingReadWithFailure(int errorCode) in SqlCommand\SqlDataReader - internal static bool _forcePendingReadsToWaitForUser = false; + internal static bool s_forcePendingReadsToWaitForUser = false; internal TaskCompletionSource _realNetworkPacketTaskSource; // Field is never assigned to, and will always have its default value #pragma warning disable 0649 // Set to true to enable checking the call stacks match when packet retry occurs. - internal static bool _checkNetworkPacketRetryStacks = false; + internal static bool s_checkNetworkPacketRetryStacks = false; #pragma warning restore 0649 #endif From 315acb1615beee6a72d3140b3dfb854a40c3c261 Mon Sep 17 00:00:00 2001 From: panoskj Date: Wed, 5 Oct 2022 11:55:43 +0300 Subject: [PATCH 10/16] TdsParserStateObject: Removed unnecessary assignments. Used discard to ignore out parameters, instead of an unused variable. --- .../Data/SqlClient/TdsParserStateObject.cs | 18 ++++++--------- .../Data/SqlClient/TdsParserStateObject.cs | 23 ++++++++----------- 2 files changed, 17 insertions(+), 24 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 46fa274787..b894e7f141 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 @@ -564,7 +564,7 @@ internal bool TryReadInt64(out long value) // If the long isn't fully in the buffer, or if it isn't fully in the packet, // then use ReadByteArray since the logic is there to take care of that. - int bytesRead = 0; + int bytesRead; if (!TryReadByteArray(_bTmp.AsSpan(_bTmpRead), 8 - _bTmpRead, out bytesRead)) { Debug.Assert(_bTmpRead + bytesRead <= 8, "Read more data than required"); @@ -642,7 +642,7 @@ internal bool TryReadUInt32(out uint value) // If the int isn't fully in the buffer, or if it isn't fully in the packet, // then use ReadByteArray since the logic is there to take care of that. - int bytesRead = 0; + int bytesRead; if (!TryReadByteArray(_bTmp.AsSpan(_bTmpRead), 4 - _bTmpRead, out bytesRead)) { Debug.Assert(_bTmpRead + bytesRead <= 4, "Read more data than required"); @@ -789,8 +789,7 @@ internal bool TryReadStringWithEncoding(int length, System.Text.Encoding encodin // Need to skip the current column before throwing the error - this ensures that the state shared between this and the data reader is consistent when calling DrainData if (isPlp) { - ulong ignored; - if (!_parser.TrySkipPlpValue((ulong)length, this, out ignored)) + if (!_parser.TrySkipPlpValue((ulong)length, this, out _)) { value = null; return false; @@ -960,7 +959,6 @@ internal bool TryReadPlpBytes(ref byte[] buff, int offset, int len, out int tota int bytesRead; int bytesLeft; byte[] newbuf; - ulong ignored; if (_longlen == 0) { @@ -1000,7 +998,7 @@ internal bool TryReadPlpBytes(ref byte[] buff, int offset, int len, out int tota if (_longlenleft == 0) { - if (!TryReadPlpLength(false, out ignored)) + if (!TryReadPlpLength(false, out _)) { totalBytesRead = 0; return false; @@ -1052,7 +1050,7 @@ internal bool TryReadPlpBytes(ref byte[] buff, int offset, int len, out int tota if (_longlenleft == 0) { // Read the next chunk or cleanup state if hit the end - if (!TryReadPlpLength(false, out ignored)) + if (!TryReadPlpLength(false, out _)) { if (_snapshot != null) { @@ -1085,11 +1083,10 @@ internal bool TryReadPlpBytes(ref byte[] buff, int offset, int len, out int tota internal bool TrySkipLongBytes(long num) { Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - int cbSkip = 0; while (num > 0) { - cbSkip = (int)Math.Min((long)int.MaxValue, num); + int cbSkip = (int)Math.Min((long)int.MaxValue, num); if (!TryReadByteArray(Span.Empty, cbSkip)) { return false; @@ -2673,9 +2670,8 @@ private Task WriteSni(bool canAccumulate) SetBufferSecureStrings(); SetPacketData(packet, _outBuff, _outBytesUsed); - uint sniError; Debug.Assert(Parser.Connection._parserLock.ThreadMayHaveLock(), "Thread is writing without taking the connection lock"); - Task task = SNIWritePacket(packet, out sniError, canAccumulate, callerHasConnectionLock: true); + Task task = SNIWritePacket(packet, out _, canAccumulate, callerHasConnectionLock: true); // Check to see if the timeout has occurred. This time out code is special case code to allow BCP writes to timeout. Eventually we should make all writes timeout. if (_bulkCopyOpperationInProgress && 0 == GetTimeoutRemaining()) 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 06ec369e0d..5a6b66fb1d 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 @@ -76,7 +76,8 @@ internal TdsParserStateObject(TdsParser parser, SNIHandle physicalConnection, bo SNINativeMethodWrapper.ConsumerInfo myInfo = CreateConsumerInfo(async); SQLDNSInfo cachedDNSInfo; - bool ret = SQLFallbackDNSCache.Instance.GetDNSInfo(_parser.FQDNforDNSCache, out cachedDNSInfo); + + SQLFallbackDNSCache.Instance.GetDNSInfo(_parser.FQDNforDNSCache, out cachedDNSInfo); _sessionHandle = new SNIHandle(myInfo, physicalConnection, _parser.Connection.ConnectionOptions.IPAddressPreference, cachedDNSInfo); if (_sessionHandle.Status != TdsEnums.SNI_SUCCESS) @@ -678,7 +679,7 @@ internal bool TryReadInt64(out long value) // If the long isn't fully in the buffer, or if it isn't fully in the packet, // then use ReadByteArray since the logic is there to take care of that. - int bytesRead = 0; + int bytesRead; if (!TryReadByteArray(_bTmp.AsSpan(start: _bTmpRead), 8 - _bTmpRead, out bytesRead)) { Debug.Assert(_bTmpRead + bytesRead <= 8, "Read more data than required"); @@ -765,7 +766,7 @@ internal bool TryReadUInt32(out uint value) // If the int isn't fully in the buffer, or if it isn't fully in the packet, // then use ReadByteArray since the logic is there to take care of that. - int bytesRead = 0; + int bytesRead; if (!TryReadByteArray(_bTmp.AsSpan(start: _bTmpRead), 4 - _bTmpRead, out bytesRead)) { Debug.Assert(_bTmpRead + bytesRead <= 4, "Read more data than required"); @@ -918,8 +919,7 @@ internal bool TryReadStringWithEncoding(int length, System.Text.Encoding encodin // Need to skip the current column before throwing the error - this ensures that the state shared between this and the data reader is consistent when calling DrainData if (isPlp) { - ulong ignored; - if (!_parser.TrySkipPlpValue((ulong)length, this, out ignored)) + if (!_parser.TrySkipPlpValue((ulong)length, this, out _)) { value = null; return false; @@ -1084,10 +1084,9 @@ internal int ReadPlpBytesChunk(byte[] buff, int offset, int len) // Every time you call this method increment the offst and decrease len by the value of totalBytesRead internal bool TryReadPlpBytes(ref byte[] buff, int offst, int len, out int totalBytesRead) { - int bytesRead = 0; + int bytesRead; int bytesLeft; byte[] newbuf; - ulong ignored; if (_longlen == 0) { @@ -1116,7 +1115,7 @@ internal bool TryReadPlpBytes(ref byte[] buff, int offst, int len, out int total if (_longlenleft == 0) { - if (!TryReadPlpLength(false, out ignored)) + if (!TryReadPlpLength(false, out _)) { totalBytesRead = 0; return false; @@ -1161,7 +1160,7 @@ internal bool TryReadPlpBytes(ref byte[] buff, int offst, int len, out int total if (_longlenleft == 0) { // Read the next chunk or cleanup state if hit the end - if (!TryReadPlpLength(false, out ignored)) + if (!TryReadPlpLength(false, out _)) { return false; } @@ -1188,11 +1187,10 @@ internal bool TryReadPlpBytes(ref byte[] buff, int offst, int len, out int total internal bool TrySkipLongBytes(long num) { Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - int cbSkip = 0; while (num > 0) { - cbSkip = (int)Math.Min((long)Int32.MaxValue, num); + int cbSkip = (int)Math.Min((long)Int32.MaxValue, num); if (!TryReadByteArray(Span.Empty, cbSkip)) { return false; @@ -2738,9 +2736,8 @@ private Task WriteSni(bool canAccumulate) SNIPacket packet = GetResetWritePacket(); SNINativeMethodWrapper.SNIPacketSetData(packet, _outBuff, _outBytesUsed, _securePasswords, _securePasswordOffsetsInBuffer); - uint sniError; Debug.Assert(Parser.Connection._parserLock.ThreadMayHaveLock(), "Thread is writing without taking the connection lock"); - Task task = SNIWritePacket(Handle, packet, out sniError, canAccumulate, callerHasConnectionLock: true); + Task task = SNIWritePacket(Handle, packet, out _, canAccumulate, callerHasConnectionLock: true); // Check to see if the timeout has occurred. This time out code is special case code to allow BCP writes to timeout to fix bug 350558, eventually we should make all writes timeout. if (_bulkCopyOpperationInProgress && 0 == GetTimeoutRemaining()) From 87f0cba2a5c20468df35450d7e6a00c7c616407a Mon Sep 17 00:00:00 2001 From: panoskj Date: Wed, 5 Oct 2022 12:00:42 +0300 Subject: [PATCH 11/16] TdsParserStateObject: Simplified names. --- .../Data/SqlClient/TdsParserStateObject.cs | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) 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 5a6b66fb1d..ff26640565 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 @@ -116,7 +116,7 @@ internal bool HasPendingData set => _pendingData = value; } - internal UInt32 Status + internal uint Status { get { @@ -302,16 +302,16 @@ internal void CreatePhysicalSNIHandle( // Translate to SNI timeout values (Int32 milliseconds) long timeout; - if (Int64.MaxValue == timerExpire) + if (long.MaxValue == timerExpire) { - timeout = Int32.MaxValue; + timeout = int.MaxValue; } else { timeout = ADP.TimerRemainingMilliseconds(timerExpire); - if (timeout > Int32.MaxValue) + if (timeout > int.MaxValue) { - timeout = Int32.MaxValue; + timeout = int.MaxValue; } else if (0 > timeout) { @@ -623,7 +623,7 @@ internal bool TryReadInt16(out short value) } AssertValidState(); - value = (Int16)((buffer[offset + 1] << 8) + buffer[offset]); + value = (short)((buffer[offset + 1] << 8) + buffer[offset]); return true; } @@ -744,7 +744,7 @@ internal bool TryReadUInt16(out ushort value) } AssertValidState(); - value = (UInt16)((buffer[offset + 1] << 8) + buffer[offset]); + value = (ushort)((buffer[offset + 1] << 8) + buffer[offset]); return true; } @@ -941,7 +941,7 @@ internal bool TryReadStringWithEncoding(int length, System.Text.Encoding encodin if (isPlp) { - if (!TryReadPlpBytes(ref buf, 0, Int32.MaxValue, out length)) + if (!TryReadPlpBytes(ref buf, 0, int.MaxValue, out length)) { value = null; return false; @@ -1190,7 +1190,7 @@ internal bool TrySkipLongBytes(long num) while (num > 0) { - int cbSkip = (int)Math.Min((long)Int32.MaxValue, num); + int cbSkip = (int)Math.Min((long)int.MaxValue, num); if (!TryReadByteArray(Span.Empty, cbSkip)) { return false; @@ -1304,7 +1304,7 @@ internal void ReadSniSyncOverAsync() } IntPtr readPacket = IntPtr.Zero; - UInt32 error; + uint error; RuntimeHelpers.PrepareConstrainedRegions(); bool shouldDecrement = false; @@ -1585,7 +1585,7 @@ internal void ReadSni(TaskCompletionSource completion) #endif IntPtr readPacket = IntPtr.Zero; - UInt32 error = 0; + uint error = 0; RuntimeHelpers.PrepareConstrainedRegions(); try @@ -1727,7 +1727,7 @@ internal bool IsConnectionAlive(bool throwOnException) } else { - UInt32 error; + uint error; IntPtr readPacket = IntPtr.Zero; RuntimeHelpers.PrepareConstrainedRegions(); @@ -1790,7 +1790,7 @@ internal bool ValidateSNIConnection() return true; } - UInt32 error = TdsEnums.SNI_SUCCESS; + uint error = TdsEnums.SNI_SUCCESS; SniContext = SniContext.Snix_Connect; try { @@ -1809,7 +1809,7 @@ internal bool ValidateSNIConnection() } // This method should only be called by ReadSni! If not - it may have problems with timeouts! - private void ReadSniError(TdsParserStateObject stateObj, UInt32 error) + private void ReadSniError(TdsParserStateObject stateObj, uint error) { TdsParser.ReliabilitySection.Assert("unreliable call to ReadSniSyncError"); // you need to setup for a thread abort somewhere before you call this method @@ -1914,7 +1914,7 @@ private void ReadSniError(TdsParserStateObject stateObj, UInt32 error) } // TODO: - does this need to be MUSTRUN??? - public void ProcessSniPacket(IntPtr packet, UInt32 error) + public void ProcessSniPacket(IntPtr packet, uint error) { if (error != 0) { @@ -1930,8 +1930,8 @@ public void ProcessSniPacket(IntPtr packet, UInt32 error) } else { - UInt32 dataSize = 0; - UInt32 getDataError = SNINativeMethodWrapper.SNIPacketGetData(packet, _inBuff, ref dataSize); + uint dataSize = 0; + uint getDataError = SNINativeMethodWrapper.SNIPacketGetData(packet, _inBuff, ref dataSize); if (getDataError == TdsEnums.SNI_SUCCESS) { @@ -1984,7 +1984,7 @@ private void ChangeNetworkPacketTimeout(int dueTime, int period) } } - public void ReadAsyncCallback(IntPtr key, IntPtr packet, UInt32 error) + 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 @@ -2126,7 +2126,7 @@ private void ReadAsyncCallbackCaptureException(TaskCompletionSource sour #pragma warning disable 420 // a reference to a volatile field will not be treated as volatile - public void WriteAsyncCallback(IntPtr key, IntPtr packet, UInt32 sniError) + public void WriteAsyncCallback(IntPtr key, IntPtr packet, uint sniError) { // Key never used. RemovePacketFromPendingList(packet); try @@ -2320,7 +2320,7 @@ internal void WriteByte(byte b) // // Takes a byte array and writes it to the buffer. // - internal Task WriteByteArray(Byte[] b, int len, int offsetBuffer, bool canAccumulate = true, TaskCompletionSource completion = null) + internal Task WriteByteArray(byte[] b, int len, int offsetBuffer, bool canAccumulate = true, TaskCompletionSource completion = null) { try { @@ -2514,7 +2514,7 @@ private void CancelWritePacket() #pragma warning disable 420 // a reference to a volatile field will not be treated as volatile - private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out UInt32 sniError, bool canAccumulate, bool callerHasConnectionLock, bool asyncClose = false) + private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock, bool asyncClose = false) { // Check for a stored exception var delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); @@ -2697,7 +2697,7 @@ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = fa return; } - UInt32 sniError; + uint sniError; _parser._asyncWrite = false; // stop async write SNIWritePacket(Handle, attnPacket, out sniError, canAccumulate: false, callerHasConnectionLock: false, asyncClose); SqlClientEventSource.Log.TryTraceEvent(" Send Attention ASync.", "Info"); From d71028bbbb43a4d231fc356b90e6f6b5c5e7ed27 Mon Sep 17 00:00:00 2001 From: panoskj Date: Wed, 5 Oct 2022 12:02:51 +0300 Subject: [PATCH 12/16] TdsParserStateObject: Simplified default expression. --- .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 +- .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 8 ++++---- 2 files changed, 5 insertions(+), 5 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 b894e7f141..d8c17386ea 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 @@ -684,7 +684,7 @@ internal bool TryReadSingle(out float value) if (!TryReadByteArray(_bTmp, 4)) { - value = default(float); + value = default; return false; } 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 ff26640565..21ea221f2d 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 @@ -603,7 +603,7 @@ internal bool TryReadInt16(out short value) if (!TryReadByteArray(_bTmp, 2)) { - value = default(short); + value = default; return false; } @@ -724,7 +724,7 @@ internal bool TryReadUInt16(out ushort value) if (!TryReadByteArray(_bTmp, 2)) { - value = default(ushort); + value = default; return false; } @@ -809,7 +809,7 @@ internal bool TryReadSingle(out float value) if (!TryReadByteArray(_bTmp, 4)) { - value = default(float); + value = default; return false; } @@ -843,7 +843,7 @@ internal bool TryReadDouble(out double value) if (!TryReadByteArray(_bTmp, 8)) { - value = default(double); + value = default; return false; } From 3e05aeb8382f7d7cf23460b1dda1052d203d8ad5 Mon Sep 17 00:00:00 2001 From: panoskj Date: Wed, 5 Oct 2022 12:07:35 +0300 Subject: [PATCH 13/16] TdsParserStateObject: Removed unnecessary casts. --- .../Data/SqlClient/TdsParserStateObject.cs | 14 +++++++------- .../Data/SqlClient/TdsParserStateObject.cs | 14 +++++++------- 2 files changed, 14 insertions(+), 14 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 d8c17386ea..e579500c84 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 @@ -916,7 +916,7 @@ internal bool TryReadPlpLength(bool returnPlpNullIfNull, out ulong lengthLeft) } else { - _longlenleft = (ulong)chunklen; + _longlenleft = chunklen; } } @@ -992,7 +992,7 @@ internal bool TryReadPlpBytes(ref byte[] buff, int offset, int len, out int tota if ((ulong)(buff?.Length ?? 0) != _longlen) { // if the buffer is null or the wrong length create one to use - buff = new byte[(int)Math.Min((int)_longlen, len)]; + buff = new byte[(Math.Min((int)_longlen, len))]; } } @@ -1086,12 +1086,12 @@ internal bool TrySkipLongBytes(long num) while (num > 0) { - int cbSkip = (int)Math.Min((long)int.MaxValue, num); + int cbSkip = (int)Math.Min(int.MaxValue, num); if (!TryReadByteArray(Span.Empty, cbSkip)) { return false; } - num -= (long)cbSkip; + num -= cbSkip; } return true; @@ -1367,7 +1367,7 @@ private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose = { if (!_attentionSent) { - AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, (byte)0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); + AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, 0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); // Grab a reference to the _networkPacketTaskSource in case it becomes null while we are trying to use it TaskCompletionSource source = _networkPacketTaskSource; @@ -1704,7 +1704,7 @@ private void ReadSniError(TdsParserStateObject stateObj, uint error) { 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)); + AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, 0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); if (!stateObj._attentionSent) { @@ -2680,7 +2680,7 @@ private Task WriteSni(bool canAccumulate) try { 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)); + AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, 0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); _bulkCopyWriteTimeout = true; SendAttention(); _parser.ProcessPendingAck(this); 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 21ea221f2d..01312b80ed 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 @@ -1044,7 +1044,7 @@ internal bool TryReadPlpLength(bool returnPlpNullIfNull, out ulong lengthLeft) } else { - _longlenleft = (ulong)chunklen; + _longlenleft = chunklen; } } @@ -1110,7 +1110,7 @@ internal bool TryReadPlpBytes(ref byte[] buff, int offst, int len, out int total // If total length is known up front, allocate the whole buffer in one shot instead of realloc'ing and copying over each time if (buff == null && _longlen != TdsEnums.SQL_PLP_UNKNOWNLEN) { - buff = new byte[(int)Math.Min((int)_longlen, len)]; + buff = new byte[(Math.Min((int)_longlen, len))]; } if (_longlenleft == 0) @@ -1190,12 +1190,12 @@ internal bool TrySkipLongBytes(long num) while (num > 0) { - int cbSkip = (int)Math.Min((long)int.MaxValue, num); + int cbSkip = (int)Math.Min(int.MaxValue, num); if (!TryReadByteArray(Span.Empty, cbSkip)) { return false; } - num -= (long)cbSkip; + num -= cbSkip; } return true; @@ -1465,7 +1465,7 @@ private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose = { if (!_attentionSent) { - AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, (byte)0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); + AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, 0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); // Grab a reference to the _networkPacketTaskSource in case it becomes null while we are trying to use it TaskCompletionSource source = _networkPacketTaskSource; @@ -1826,7 +1826,7 @@ private void ReadSniError(TdsParserStateObject stateObj, uint error) { 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)); + AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, 0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); if (!stateObj._attentionSent) { @@ -2746,7 +2746,7 @@ private Task WriteSni(bool canAccumulate) try { 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)); + AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, 0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); _bulkCopyWriteTimeout = true; SendAttention(); _parser.ProcessPendingAck(this); From 58ae1361e1bc9acdf959c4a716759cc0693feb5c Mon Sep 17 00:00:00 2001 From: panoskj Date: Wed, 5 Oct 2022 12:11:25 +0300 Subject: [PATCH 14/16] TdsParserStateObject: Replaced var usages with explicit types. --- .../Data/SqlClient/TdsParserStateObject.cs | 26 +++++++++---------- .../Data/SqlClient/TdsParserStateObject.cs | 24 ++++++++--------- 2 files changed, 25 insertions(+), 25 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 e579500c84..35395ace1b 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 @@ -398,7 +398,7 @@ public bool TryReadByteArray(Span buff, int len, out int totalRead) Debug.Assert(bytesToRead > 0, "0 byte read in TryReadByteArray"); if (!buff.IsEmpty) { - var copyFrom = new ReadOnlySpan(_inBuff, _inBytesUsed, bytesToRead); + ReadOnlySpan copyFrom = new ReadOnlySpan(_inBuff, _inBytesUsed, bytesToRead); Span copyTo = buff.Slice(totalRead, bytesToRead); copyFrom.CopyTo(copyTo); } @@ -1292,7 +1292,7 @@ internal void OnConnectionClosed() Interlocked.MemoryBarrier(); // then check for networkPacketTaskSource - var taskSource = _networkPacketTaskSource; + TaskCompletionSource taskSource = _networkPacketTaskSource; if (taskSource != null) { taskSource.TrySetException(ADP.ExceptionWithStackTrace(ADP.ClosedConnectionError())); @@ -2055,7 +2055,7 @@ public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError) } catch (Exception e) { - var writeCompletionSource = _writeCompletionSource; + TaskCompletionSource writeCompletionSource = _writeCompletionSource; if (writeCompletionSource != null) { writeCompletionSource.TrySetException(e); @@ -2071,7 +2071,7 @@ public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError) writeCompletionSource = _writeCompletionSource; if (writeCompletionSource != null) { - var delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); + Exception delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); if (delayedException != null) { writeCompletionSource.TrySetException(delayedException); @@ -2095,7 +2095,7 @@ public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError) new Timer(obj => { Interlocked.Decrement(ref _asyncWriteCount); - var writeCompletionSource = _writeCompletionSource; + TaskCompletionSource writeCompletionSource = _writeCompletionSource; if (_asyncWriteCount == 0 && writeCompletionSource != null) { writeCompletionSource.TrySetResult(null); @@ -2116,7 +2116,7 @@ public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError) return; } #endif - var completionSource = _writeCompletionSource; + TaskCompletionSource completionSource = _writeCompletionSource; if (_asyncWriteCount == 0 && completionSource != null) { completionSource.TrySetResult(null); @@ -2158,7 +2158,7 @@ internal void ResetSecurePasswordsInformation() internal Task WaitForAccumulatedWrites() { // Checked for stored exceptions - var delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); + Exception delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); if (delayedException != null) { throw delayedException; @@ -2449,7 +2449,7 @@ private void CancelWritePacket() 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); + Exception delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); if (delayedException != null) { throw delayedException; @@ -2998,7 +2998,7 @@ internal void CheckThrowSNIException() internal void AssertStateIsClean() { // If our TdsParser is closed or broken, then we don't really care about our state - var parser = _parser; + TdsParser parser = _parser; if ((parser != null) && (parser.State != TdsParserState.Closed) && (parser.State != TdsParserState.Broken)) { // Async reads @@ -3020,8 +3020,8 @@ internal void AssertStateIsClean() #if DEBUG internal void CompletePendingReadWithSuccess(bool resetForcePendingReadsToWait) { - var realNetworkPacketTaskSource = _realNetworkPacketTaskSource; - var networkPacketTaskSource = _networkPacketTaskSource; + TaskCompletionSource realNetworkPacketTaskSource = _realNetworkPacketTaskSource; + TaskCompletionSource networkPacketTaskSource = _networkPacketTaskSource; Debug.Assert(s_forcePendingReadsToWaitForUser, "Not forcing pends to wait for user - can't force complete"); Debug.Assert(networkPacketTaskSource != null, "No pending read to complete"); @@ -3050,8 +3050,8 @@ internal void CompletePendingReadWithSuccess(bool resetForcePendingReadsToWait) internal void CompletePendingReadWithFailure(int errorCode, bool resetForcePendingReadsToWait) { - var realNetworkPacketTaskSource = _realNetworkPacketTaskSource; - var networkPacketTaskSource = _networkPacketTaskSource; + TaskCompletionSource realNetworkPacketTaskSource = _realNetworkPacketTaskSource; + TaskCompletionSource networkPacketTaskSource = _networkPacketTaskSource; Debug.Assert(s_forcePendingReadsToWaitForUser, "Not forcing pends to wait for user - can't force complete"); Debug.Assert(networkPacketTaskSource != null, "No pending read to complete"); 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 01312b80ed..698adfe5d1 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 @@ -1389,7 +1389,7 @@ internal void OnConnectionClosed() Thread.MemoryBarrier(); // then check for networkPacketTaskSource - var taskSource = _networkPacketTaskSource; + TaskCompletionSource taskSource = _networkPacketTaskSource; if (taskSource != null) { taskSource.TrySetException(ADP.ExceptionWithStackTrace(ADP.ClosedConnectionError())); @@ -2141,7 +2141,7 @@ public void WriteAsyncCallback(IntPtr key, IntPtr packet, uint sniError) } catch (Exception e) { - var writeCompletionSource = _writeCompletionSource; + TaskCompletionSource writeCompletionSource = _writeCompletionSource; if (writeCompletionSource != null) { writeCompletionSource.TrySetException(e); @@ -2157,7 +2157,7 @@ public void WriteAsyncCallback(IntPtr key, IntPtr packet, uint sniError) writeCompletionSource = _writeCompletionSource; if (writeCompletionSource != null) { - var delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); + Exception delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); if (delayedException != null) { writeCompletionSource.TrySetException(delayedException); @@ -2181,7 +2181,7 @@ public void WriteAsyncCallback(IntPtr key, IntPtr packet, uint sniError) new Timer(obj => { Interlocked.Decrement(ref _asyncWriteCount); - var writeCompletionSource = _writeCompletionSource; + TaskCompletionSource writeCompletionSource = _writeCompletionSource; if (_asyncWriteCount == 0 && writeCompletionSource != null) { writeCompletionSource.TrySetResult(null); @@ -2202,7 +2202,7 @@ public void WriteAsyncCallback(IntPtr key, IntPtr packet, uint sniError) return; } #endif - var completionSource = _writeCompletionSource; + TaskCompletionSource completionSource = _writeCompletionSource; if (_asyncWriteCount == 0 && completionSource != null) { completionSource.TrySetResult(null); @@ -2258,7 +2258,7 @@ internal Task WaitForAccumulatedWrites() { // Checked for stored exceptions #pragma warning disable 420 // A reference to a volatile field will not be treated as volatile - Disabling since the Interlocked APIs are volatile aware - var delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); + Exception delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); if (delayedException != null) { throw delayedException; @@ -2517,7 +2517,7 @@ private void CancelWritePacket() private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock, bool asyncClose = false) { // Check for a stored exception - var delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); + Exception delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); if (delayedException != null) { throw delayedException; @@ -3163,7 +3163,7 @@ internal void CheckThrowSNIException() internal void AssertStateIsClean() { // If our TdsParser is closed or broken, then we don't really care about our state - var parser = _parser; + TdsParser parser = _parser; if ((parser != null) && (parser.State != TdsParserState.Closed) && (parser.State != TdsParserState.Broken)) { // Async reads @@ -3185,8 +3185,8 @@ internal void AssertStateIsClean() #if DEBUG internal void CompletePendingReadWithSuccess(bool resetForcePendingReadsToWait) { - var realNetworkPacketTaskSource = _realNetworkPacketTaskSource; - var networkPacketTaskSource = _networkPacketTaskSource; + TaskCompletionSource realNetworkPacketTaskSource = _realNetworkPacketTaskSource; + TaskCompletionSource networkPacketTaskSource = _networkPacketTaskSource; Debug.Assert(s_forcePendingReadsToWaitForUser, "Not forcing pends to wait for user - can't force complete"); Debug.Assert(networkPacketTaskSource != null, "No pending read to complete"); @@ -3215,8 +3215,8 @@ internal void CompletePendingReadWithSuccess(bool resetForcePendingReadsToWait) internal void CompletePendingReadWithFailure(int errorCode, bool resetForcePendingReadsToWait) { - var realNetworkPacketTaskSource = _realNetworkPacketTaskSource; - var networkPacketTaskSource = _networkPacketTaskSource; + TaskCompletionSource realNetworkPacketTaskSource = _realNetworkPacketTaskSource; + TaskCompletionSource networkPacketTaskSource = _networkPacketTaskSource; Debug.Assert(s_forcePendingReadsToWaitForUser, "Not forcing pends to wait for user - can't force complete"); Debug.Assert(networkPacketTaskSource != null, "No pending read to complete"); From 5eb4a6115f06d5dbdfbe14ab010bc186bbe2134b Mon Sep 17 00:00:00 2001 From: DavoudEshtehari <61173489+DavoudEshtehari@users.noreply.github.com> Date: Tue, 8 Nov 2022 11:38:24 -0800 Subject: [PATCH 15/16] Apply suggestions from code review Co-authored-by: Lawrence Cheung <31262254+lcheunglci@users.noreply.github.com> --- .../netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs | 2 +- .../Microsoft/Data/SqlClient/TdsParserStateObject.cs | 12 +++--------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs index c84b9b549c..621d47b20f 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs @@ -55,7 +55,7 @@ internal static Task CreateContinuationTaskWithState(Task task, object state, Ac } else { - var completion = new TaskCompletionSource(); + TaskCompletionSource completion = new(); ContinueTaskWithState(task, completion, state, onSuccess: (object continueState) => { 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 10956fbc74..cce5e04c29 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -60,7 +60,6 @@ public TimeoutState(int value) // of very small open, query, close loops. private const long CheckConnectionWindow = 50000; - protected readonly TdsParser _parser; // TdsParser pointer private readonly WeakReference _owner = new(null); // the owner of this session, used to track when it's been orphaned internal SqlDataReader.SharedState _readerState; // susbset of SqlDataReader state (if it is the owner) necessary for parsing abandoned results in TDS @@ -134,7 +133,6 @@ public TimeoutState(int value) internal volatile bool _attentionSending; private readonly TimerCallback _onTimeoutAsync; - // 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. @@ -181,7 +179,6 @@ public TimeoutState(int value) internal byte[] _bShortBytes; // scratch buffer to serialize Short values (2 bytes). internal byte[] _bDecimalBytes; // scratch buffer to serialize decimal values (17 bytes). - // DO NOT USE THIS BUFFER FOR OTHER THINGS. // ProcessHeader can be called ANYTIME while doing network reads. private byte[] _partialHeaderBuffer = new byte[TdsEnums.HEADER_LEN]; // Scratch buffer for ProcessHeader @@ -190,7 +187,6 @@ public TimeoutState(int value) internal _SqlMetaDataSet _cleanupMetaData; internal _SqlMetaDataSetCollection _cleanupAltMetaDataSetArray; - private SniContext _sniContext = SniContext.Undefined; #if DEBUG private SniContext _debugOnlyCopyOfSniContext = SniContext.Undefined; @@ -280,7 +276,6 @@ public TimeoutState(int value) #pragma warning restore 0649 #endif - ////////////////// // Constructors // ////////////////// @@ -302,7 +297,6 @@ internal TdsParserStateObject(TdsParser parser) _lastSuccessfulIOTimer = new LastIOTimer(); } - //////////////// // Properties // //////////////// @@ -547,7 +541,7 @@ internal void Activate(object owner) internal void CancelRequest() { ResetBuffer(); // clear out unsent buffer - // If the first sqlbulkcopy timeout, _outputPacketNumber may not be 1, + // VSDD#903514, if the first sqlbulkcopy timeout, _outputPacketNumber may not be 1, // the next sqlbulkcopy (same connection string) requires this to be 1, hence reset // it here when exception happens in the first sqlbulkcopy ResetPacketCounters(); @@ -566,7 +560,7 @@ public void CheckSetResetConnectionState(uint error, CallbackType callbackType) // Should only be called for MARS - that is the only time we need to take // the ResetConnection lock! - // It was raised in a security review by Microsoft questioning whether + // SQL BU DT 333026 - It was raised in a security review by Microsoft questioning whether // we need to actually process the resulting packet (sp_reset ack or error) to know if the // reset actually succeeded. There was a concern that if the reset failed and we proceeded // there might be a security issue present. We have been assured by the server that if @@ -626,7 +620,7 @@ internal bool Deactivate() } if (HasOpenResult) - { + { // SQL BU DT 383773 - need to decrement openResultCount for all pending operations. DecrementOpenResultCount(); } From 0b4c3457985776cb886a1546a686a36340aaf426 Mon Sep 17 00:00:00 2001 From: Davoud Eshtehari Date: Wed, 9 Nov 2022 13:54:00 -0800 Subject: [PATCH 16/16] Address comments --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 24 ++++----- .../Data/SqlClient/TdsParserStateObject.cs | 40 +------------- .../Data/SqlClient/TdsParserStateObject.cs | 45 +--------------- .../Data/SqlClient/TdsParserStateObject.cs | 54 ++++++++++++++----- 4 files changed, 57 insertions(+), 106 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index 5dc100b5be..d1233d164a 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -1329,7 +1329,7 @@ internal void ThrowExceptionAndWarning(TdsParserStateObject stateObj, bool calle // report exception to pending async operation // before OnConnectionClosed overrides the exception // due to connection close notification through references - var taskSource = stateObj._networkPacketTaskSource; + TaskCompletionSource taskSource = stateObj._networkPacketTaskSource; if (taskSource != null) { taskSource.TrySetException(ADP.ExceptionWithStackTrace(exception)); @@ -1339,7 +1339,7 @@ internal void ThrowExceptionAndWarning(TdsParserStateObject stateObj, bool calle if (asyncClose) { // Wait until we have the parser lock, then try to close - var connHandler = _connHandler; + SqlInternalConnectionTds connHandler = _connHandler; Action wrapCloseAction = closeAction => { Task.Factory.StartNew(() => @@ -2609,7 +2609,7 @@ private bool TryProcessEnvChange(int tokenLength, TdsParserStateObject stateObj, while (tokenLength > processedLength) { - var env = new SqlEnvChange(); + SqlEnvChange env = new SqlEnvChange(); if (!stateObj.TryReadByte(out env._type)) { return false; @@ -3372,7 +3372,7 @@ private bool TryProcessDataClassification(TdsParserStateObject stateObj, out Sen { return false; } - var labels = new List