From ac5c4f6deed21d1202adc1d95691de9b9e00e6c5 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Wed, 13 Sep 2023 20:09:15 +0100 Subject: [PATCH 1/2] move state flags to shared and update snapshot logic --- .../SqlClient/TdsParserStateObject.netcore.cs | 50 ----------------- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 14 ++--- .../SqlClient/TdsParserStateObject.netfx.cs | 56 ++----------------- .../Data/SqlClient/TdsParserStateObject.cs | 51 +++++++++++++++++ 4 files changed, 63 insertions(+), 108 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index d66c55a9a2..60e17daaa3 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -21,7 +21,6 @@ internal abstract partial class TdsParserStateObject private readonly WeakReference _cancellationOwner = new WeakReference(null); // Async - private SnapshottedStateFlags _snapshottedState; ////////////////// // Constructors // @@ -279,53 +278,6 @@ internal void StartSession(object cancellationOwner) _cancellationOwner.Target = cancellationOwner; } - private void SetSnapshottedState(SnapshottedStateFlags flag, bool value) - { - if (value) - { - _snapshottedState |= flag; - } - else - { - _snapshottedState &= ~flag; - } - } - - private bool GetSnapshottedState(SnapshottedStateFlags flag) - { - return (_snapshottedState & flag) == flag; - } - - internal bool HasOpenResult - { - get => GetSnapshottedState(SnapshottedStateFlags.OpenResult); - set => SetSnapshottedState(SnapshottedStateFlags.OpenResult, value); - } - - internal bool HasPendingData - { - get => GetSnapshottedState(SnapshottedStateFlags.PendingData); - set => SetSnapshottedState(SnapshottedStateFlags.PendingData, value); - } - - internal bool HasReceivedError - { - get => GetSnapshottedState(SnapshottedStateFlags.ErrorTokenReceived); - set => SetSnapshottedState(SnapshottedStateFlags.ErrorTokenReceived, value); - } - - internal bool HasReceivedAttention - { - get => GetSnapshottedState(SnapshottedStateFlags.AttentionReceived); - set => SetSnapshottedState(SnapshottedStateFlags.AttentionReceived, value); - } - - internal bool HasReceivedColumnMetadata - { - get => GetSnapshottedState(SnapshottedStateFlags.ColMetaDataReceived); - set => SetSnapshottedState(SnapshottedStateFlags.ColMetaDataReceived, value); - } - /////////////////////////////////////// // Buffer read methods - data values // /////////////////////////////////////// @@ -3114,8 +3066,6 @@ partial void SetStackInternal(string value) private int _snapshotInBuffCount; - private SnapshottedStateFlags _state; - #if DEBUG internal void AssertCurrent() { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 2bdca1a9e0..4aa369203e 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -2480,7 +2480,7 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead { if (token == TdsEnums.SQLERROR) { - stateObj._errorTokenReceived = true; // Keep track of the fact error token was received - for Done processing. + stateObj.HasReceivedError = true; // Keep track of the fact error token was received - for Done processing. } SqlError error; @@ -3538,13 +3538,13 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio } // Skip the bogus DONE counts sent by the server - if (stateObj._receivedColMetaData || (curCmd != TdsEnums.SELECT)) + if (stateObj.HasReceivedColumnMetadata || (curCmd != TdsEnums.SELECT)) { cmd.OnStatementCompleted(count); } } - stateObj._receivedColMetaData = false; + stateObj.HasReceivedColumnMetadata = false; // Surface exception for DONE_ERROR in the case we did not receive an error token // in the stream, but an error occurred. In these cases, we throw a general server error. The @@ -3553,7 +3553,7 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio // the server has reached its max connection limit. Bottom line, we need to throw general // error in the cases where we did not receive a error token along with the DONE_ERROR. if ((TdsEnums.DONE_ERROR == (TdsEnums.DONE_ERROR & status)) && stateObj.ErrorCount == 0 && - stateObj._errorTokenReceived == false && (RunBehavior.Clean != (RunBehavior.Clean & run))) + stateObj.HasReceivedError == false && (RunBehavior.Clean != (RunBehavior.Clean & run))) { stateObj.AddError(new SqlError(0, 0, TdsEnums.MIN_ERROR_CLASS, _server, SQLMessage.SevereError(), "", 0)); @@ -3587,7 +3587,7 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio // stop if the DONE_MORE bit isn't set (see above for attention handling) if (TdsEnums.DONE_MORE != (status & TdsEnums.DONE_MORE)) { - stateObj._errorTokenReceived = false; + stateObj.HasReceivedError = false; if (stateObj._inBytesUsed >= stateObj._inBytesRead) { stateObj.HasPendingData = false; @@ -3597,7 +3597,7 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio // _pendingData set by e.g. 'TdsExecuteSQLBatch' // _hasOpenResult always set to true by 'WriteMarsHeader' // - if (!stateObj.HasPendingData && stateObj._hasOpenResult) + if (!stateObj.HasPendingData && stateObj.HasOpenResult) { /* Debug.Assert(!((sqlTransaction != null && _distributedTransaction != null) || @@ -5743,7 +5743,7 @@ private bool TryCommonProcessMetaData(TdsParserStateObject stateObj, _SqlMetaDat // We get too many DONE COUNTs from the server, causing too meany StatementCompleted event firings. // We only need to fire this event when we actually have a meta data stream with 0 or more rows. - stateObj._receivedColMetaData = true; + stateObj.HasReceivedColumnMetadata = true; return true; } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 0670c7c622..6fa185d8ee 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -20,9 +20,6 @@ internal partial class TdsParserStateObject { private SNIHandle _sessionHandle = null; // the SNI handle we're to work on - private bool _pendingData = false; - 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 // 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 @@ -32,25 +29,18 @@ internal partial class TdsParserStateObject // Async variables private GCHandle _gcHandle; // keeps this object alive until we're closed. - // Timeout variables - private bool _attentionReceived = false; // NOTE: Received is not volatile as it is only ever accessed\modified by TryRun its callees (i.e. single threaded access) - // 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 // the code but does not remember either. At some point, we need to research killing this // logic. private volatile int _allowObjectID; - internal bool _hasOpenResult = false; - // Used for blanking out password in trace. internal int _tracePasswordOffset = 0; internal int _tracePasswordLength = 0; internal int _traceChangePasswordOffset = 0; internal int _traceChangePasswordLength = 0; - internal bool _receivedColMetaData; // Used to keep track of when to fire StatementCompleted event. - ////////////////// // Constructors // ////////////////// @@ -100,18 +90,6 @@ internal SNIHandle Handle return _sessionHandle; } } - - internal bool HasOpenResult - { - get => _hasOpenResult; - set => _hasOpenResult = value; - } - - internal bool HasPendingData - { - get => _pendingData; - set => _pendingData = value; - } internal uint Status { @@ -394,12 +372,6 @@ internal void StartSession(int objectID) _allowObjectID = objectID; } - internal bool HasReceivedAttention - { - get => _attentionReceived; - set => _attentionReceived = value; - } - /////////////////////////////////////// // Buffer read methods - data values // /////////////////////////////////////// @@ -3230,11 +3202,6 @@ sealed partial class StateSnapshot { private List _snapshotInBuffs; - private bool _snapshotPendingData = false; - private bool _snapshotErrorTokenReceived = false; - private bool _snapshotHasOpenResult = false; - private bool _snapshotReceivedColumnMetadata = false; - private bool _snapshotAttentionReceived; public StateSnapshot() { @@ -3308,8 +3275,6 @@ internal void Snap(TdsParserStateObject state) _snapshotInBuffCurrent = 0; _snapshotInBytesUsed = _stateObj._inBytesUsed; _snapshotInBytesPacket = _stateObj._inBytesPacket; - _snapshotPendingData = _stateObj._pendingData; - _snapshotErrorTokenReceived = _stateObj._errorTokenReceived; _snapshotMessageStatus = _stateObj._messageStatus; // _nullBitmapInfo must be cloned before it is updated _snapshotNullBitmapInfo = _stateObj._nullBitmapInfo; @@ -3320,9 +3285,8 @@ internal void Snap(TdsParserStateObject state) _snapshotCleanupMetaData = _stateObj._cleanupMetaData; // _cleanupAltMetaDataSetArray must be cloned bofore it is updated _snapshotCleanupAltMetaDataSetArray = _stateObj._cleanupAltMetaDataSetArray; - _snapshotHasOpenResult = _stateObj._hasOpenResult; - _snapshotReceivedColumnMetadata = _stateObj._receivedColMetaData; - _snapshotAttentionReceived = _stateObj._attentionReceived; + + _state = _stateObj._snapshottedState; #if DEBUG _rollingPend = 0; _rollingPendCount = 0; @@ -3343,26 +3307,22 @@ internal void ResetSnapshotState() _stateObj._inBytesUsed = _snapshotInBytesUsed; _stateObj._inBytesPacket = _snapshotInBytesPacket; - _stateObj._pendingData = _snapshotPendingData; - _stateObj._errorTokenReceived = _snapshotErrorTokenReceived; _stateObj._messageStatus = _snapshotMessageStatus; _stateObj._nullBitmapInfo = _snapshotNullBitmapInfo; _stateObj._cleanupMetaData = _snapshotCleanupMetaData; _stateObj._cleanupAltMetaDataSetArray = _snapshotCleanupAltMetaDataSetArray; // Make sure to go through the appropriate increment/decrement methods if changing HasOpenResult - if (!_stateObj._hasOpenResult && _snapshotHasOpenResult) + if (!_stateObj.HasOpenResult && ((_state & SnapshottedStateFlags.OpenResult) == SnapshottedStateFlags.OpenResult)) { _stateObj.IncrementAndObtainOpenResultCount(_stateObj._executedUnderTransaction); } - else if (_stateObj._hasOpenResult && !_snapshotHasOpenResult) + else if (_stateObj.HasOpenResult && ((_state & SnapshottedStateFlags.OpenResult) != SnapshottedStateFlags.OpenResult)) { _stateObj.DecrementOpenResultCount(); } //else _stateObj._hasOpenResult is already == _snapshotHasOpenResult - - _stateObj._receivedColMetaData = _snapshotReceivedColumnMetadata; - _stateObj._attentionReceived = _snapshotAttentionReceived; + _stateObj._snapshottedState = _state; // Reset partially read state (these only need to be maintained if doing async without snapshot) _stateObj._bTmpRead = 0; @@ -3381,12 +3341,6 @@ internal void Clear() { _snapshotInBuffs.Clear(); - _snapshotPendingData = false; - _snapshotErrorTokenReceived = false; - _snapshotHasOpenResult = false; - _snapshotReceivedColumnMetadata = false; - _snapshotAttentionReceived = false; - ClearCore(); } } 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 c7dcb4a417..03030d9a98 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -64,6 +64,7 @@ public TimeoutState(int value) 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 + private SnapshottedStateFlags _snapshottedState; // 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, @@ -298,6 +299,53 @@ internal TdsParserStateObject(TdsParser parser) _lastSuccessfulIOTimer = new LastIOTimer(); } + private void SetSnapshottedState(SnapshottedStateFlags flag, bool value) + { + if (value) + { + _snapshottedState |= flag; + } + else + { + _snapshottedState &= ~flag; + } + } + + private bool GetSnapshottedState(SnapshottedStateFlags flag) + { + return (_snapshottedState & flag) == flag; + } + + internal bool HasOpenResult + { + get => GetSnapshottedState(SnapshottedStateFlags.OpenResult); + set => SetSnapshottedState(SnapshottedStateFlags.OpenResult, value); + } + + internal bool HasPendingData + { + get => GetSnapshottedState(SnapshottedStateFlags.PendingData); + set => SetSnapshottedState(SnapshottedStateFlags.PendingData, value); + } + + internal bool HasReceivedError + { + get => GetSnapshottedState(SnapshottedStateFlags.ErrorTokenReceived); + set => SetSnapshottedState(SnapshottedStateFlags.ErrorTokenReceived, value); + } + + internal bool HasReceivedAttention + { + get => GetSnapshottedState(SnapshottedStateFlags.AttentionReceived); + set => SetSnapshottedState(SnapshottedStateFlags.AttentionReceived, value); + } + + internal bool HasReceivedColumnMetadata + { + get => GetSnapshottedState(SnapshottedStateFlags.ColMetaDataReceived); + set => SetSnapshottedState(SnapshottedStateFlags.ColMetaDataReceived, value); + } + //////////////// // Properties // //////////////// @@ -1158,6 +1206,8 @@ public PLPData(ulong snapshotLongLen, ulong snapshotLongLenLeft) private _SqlMetaDataSetCollection _snapshotCleanupAltMetaDataSetArray; private TdsParserStateObject _stateObj; + private SnapshottedStateFlags _state; + #if DEBUG private int _rollingPend = 0; private int _rollingPendCount = 0; @@ -1211,6 +1261,7 @@ internal void ClearCore() _plpData = null; _snapshotCleanupMetaData = null; _snapshotCleanupAltMetaDataSetArray = null; + _state = SnapshottedStateFlags.None; #if DEBUG _rollingPend = 0; _rollingPendCount = 0; From 58bb96371860999cf6b6bae92f094eeb02844a68 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Wed, 13 Sep 2023 20:16:29 +0100 Subject: [PATCH 2/2] move ResetSnapshotState to shared --- .../SqlClient/TdsParserStateObject.netcore.cs | 39 ------------------- .../SqlClient/TdsParserStateObject.netfx.cs | 39 ------------------- .../Data/SqlClient/TdsParserStateObject.cs | 39 +++++++++++++++++++ 3 files changed, 39 insertions(+), 78 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index 60e17daaa3..0ff5a42d23 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -3168,45 +3168,6 @@ internal void Snap(TdsParserStateObject state) PushBuffer(_stateObj._inBuff, _stateObj._inBytesRead); } - internal void ResetSnapshotState() - { - // go back to the beginning - _snapshotInBuffCurrent = 0; - - Replay(); - - _stateObj._inBytesUsed = _snapshotInBytesUsed; - _stateObj._inBytesPacket = _snapshotInBytesPacket; - - _stateObj._messageStatus = _snapshotMessageStatus; - _stateObj._nullBitmapInfo = _snapshotNullBitmapInfo; - _stateObj._cleanupMetaData = _snapshotCleanupMetaData; - _stateObj._cleanupAltMetaDataSetArray = _snapshotCleanupAltMetaDataSetArray; - - // Make sure to go through the appropriate increment/decrement methods if changing the OpenResult flag - if (!_stateObj.HasOpenResult && ((_state & SnapshottedStateFlags.OpenResult) == SnapshottedStateFlags.OpenResult)) - { - _stateObj.IncrementAndObtainOpenResultCount(_stateObj._executedUnderTransaction); - } - else if (_stateObj.HasOpenResult && ((_state & SnapshottedStateFlags.OpenResult) != SnapshottedStateFlags.OpenResult)) - { - _stateObj.DecrementOpenResultCount(); - } - _stateObj._snapshottedState = _state; - - // Reset partially read state (these only need to be maintained if doing async without snapshot) - _stateObj._bTmpRead = 0; - _stateObj._partialHeaderBytesRead = 0; - - // reset plp state - _stateObj._longlen = _plpData?.SnapshotLongLen ?? 0; - _stateObj._longlenleft = _plpData?.SnapshotLongLenLeft ?? 0; - - _stateObj._snapshotReplay = true; - - _stateObj.AssertValidState(); - } - internal void Clear() { PacketData packet = _snapshotInBuffList; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 6fa185d8ee..b20614325a 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -3298,45 +3298,6 @@ internal void Snap(TdsParserStateObject state) PushBuffer(_stateObj._inBuff, _stateObj._inBytesRead); } - internal void ResetSnapshotState() - { - // go back to the beginning - _snapshotInBuffCurrent = 0; - - Replay(); - - _stateObj._inBytesUsed = _snapshotInBytesUsed; - _stateObj._inBytesPacket = _snapshotInBytesPacket; - _stateObj._messageStatus = _snapshotMessageStatus; - _stateObj._nullBitmapInfo = _snapshotNullBitmapInfo; - _stateObj._cleanupMetaData = _snapshotCleanupMetaData; - _stateObj._cleanupAltMetaDataSetArray = _snapshotCleanupAltMetaDataSetArray; - - // Make sure to go through the appropriate increment/decrement methods if changing HasOpenResult - if (!_stateObj.HasOpenResult && ((_state & SnapshottedStateFlags.OpenResult) == SnapshottedStateFlags.OpenResult)) - { - _stateObj.IncrementAndObtainOpenResultCount(_stateObj._executedUnderTransaction); - } - else if (_stateObj.HasOpenResult && ((_state & SnapshottedStateFlags.OpenResult) != SnapshottedStateFlags.OpenResult)) - { - _stateObj.DecrementOpenResultCount(); - } - //else _stateObj._hasOpenResult is already == _snapshotHasOpenResult - _stateObj._snapshottedState = _state; - - // Reset partially read state (these only need to be maintained if doing async without snapshot) - _stateObj._bTmpRead = 0; - _stateObj._partialHeaderBytesRead = 0; - - // reset plp state - _stateObj._longlen = _plpData?.SnapshotLongLen ?? 0; - _stateObj._longlenleft = _plpData?.SnapshotLongLenLeft ?? 0; - - _stateObj._snapshotReplay = true; - - _stateObj.AssertValidState(); - } - internal void Clear() { _snapshotInBuffs.Clear(); 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 03030d9a98..23af47f063 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1246,6 +1246,45 @@ internal void CloneCleanupAltMetaDataSetArray() } } + internal void ResetSnapshotState() + { + // go back to the beginning + _snapshotInBuffCurrent = 0; + + Replay(); + + _stateObj._inBytesUsed = _snapshotInBytesUsed; + _stateObj._inBytesPacket = _snapshotInBytesPacket; + _stateObj._messageStatus = _snapshotMessageStatus; + _stateObj._nullBitmapInfo = _snapshotNullBitmapInfo; + _stateObj._cleanupMetaData = _snapshotCleanupMetaData; + _stateObj._cleanupAltMetaDataSetArray = _snapshotCleanupAltMetaDataSetArray; + + // Make sure to go through the appropriate increment/decrement methods if changing HasOpenResult + if (!_stateObj.HasOpenResult && ((_state & SnapshottedStateFlags.OpenResult) == SnapshottedStateFlags.OpenResult)) + { + _stateObj.IncrementAndObtainOpenResultCount(_stateObj._executedUnderTransaction); + } + else if (_stateObj.HasOpenResult && ((_state & SnapshottedStateFlags.OpenResult) != SnapshottedStateFlags.OpenResult)) + { + _stateObj.DecrementOpenResultCount(); + } + //else _stateObj._hasOpenResult is already == _snapshotHasOpenResult + _stateObj._snapshottedState = _state; + + // Reset partially read state (these only need to be maintained if doing async without snapshot) + _stateObj._bTmpRead = 0; + _stateObj._partialHeaderBytesRead = 0; + + // reset plp state + _stateObj._longlen = _plpData?.SnapshotLongLen ?? 0; + _stateObj._longlenleft = _plpData?.SnapshotLongLenLeft ?? 0; + + _stateObj._snapshotReplay = true; + + _stateObj.AssertValidState(); + } + internal void PrepareReplay() { ResetSnapshotState();