From ba88736c5e87cd655398ed0f7d71ead0a77ecb1c Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Wed, 19 Feb 2025 01:45:42 +0000 Subject: [PATCH 1/8] port experimental implementation --- .../Data/SqlClient/TdsParserStateObject.cs | 129 +++++++++++++++++- 1 file changed, 123 insertions(+), 6 deletions(-) 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 6cd3900906..5c0895356a 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1853,6 +1853,11 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len // and try to use it if it is the right length buff = _snapshot._plpBuffer; _snapshot._plpBuffer = null; + if (_snapshot.ContinueEnabled) + { + offset = _snapshot.GetPacketDataOffset(); + totalBytesRead = offset; + } } if ((ulong)(buff?.Length ?? 0) != _longlen) @@ -1882,8 +1887,10 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len { buff = new byte[_longlenleft]; } + while (bytesLeft > 0) { + bool stored = false; int bytesToRead = (int)Math.Min(_longlenleft, (ulong)bytesLeft); if (buff.Length < (offset + bytesToRead)) { @@ -1911,6 +1918,18 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len } return result; } + else + { + if (_snapshot != null) + { + _snapshot._plpBuffer = buff; + if (_snapshotStatus != SnapshotStatus.NotActive && _snapshot.ContinueEnabled) + { + StoreReadPlpBytesProgress(this, bytesRead); + stored = true; + } + } + } if (_longlenleft == 0) { @@ -1918,11 +1937,9 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len result = TryReadPlpLength(false, out _); if (result != TdsOperationStatus.Done) { - if (_snapshot != null) + if (!stored && result == TdsOperationStatus.NeedMoreData && _snapshot != null && _snapshotStatus != SnapshotStatus.NotActive && _snapshot.ContinueEnabled) { - // a partial read has happened so store the target buffer in the snapshot - // so it can be re-used when another packet arrives and we read again - _snapshot._plpBuffer = buff; + StoreReadPlpBytesProgress(this, bytesRead); } return result; } @@ -1932,9 +1949,19 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len // Catch the point where we read the entire plp data stream and clean up state if (_longlenleft == 0) // Data read complete + { break; + } } return TdsOperationStatus.Done; + + static void StoreReadPlpBytesProgress(TdsParserStateObject stateObject, int size) + { + Debug.Assert(stateObject._snapshot != null, "_snapshot must exist to store plp read progress"); + Debug.Assert(stateObject._snapshotStatus != SnapshotStatus.NotActive, "_snapshot must be active to store plp read progress"); + + stateObject._snapshot.SetPacketPayloadSize(size); + } } ///////////////////////////////////////// @@ -2015,6 +2042,12 @@ internal TdsOperationStatus TryReadNetworkPacket() _lastStack = stackTrace; } #endif + if (_bTmpRead == 0 && _partialHeaderBytesRead == 0 && _longlenleft == 0 && _snapshot.ContinueEnabled) + { + // no temp between packets + // mark this point as continue-able + _snapshot.CaptureAsContinue(this); + } } } @@ -2063,7 +2096,10 @@ internal TdsOperationStatus TryReadNetworkPacket() internal void PrepareReplaySnapshot() { _networkPacketTaskSource = null; - _snapshot.MoveToStart(); + if (!_snapshot.MoveToContinue()) + { + _snapshot.MoveToStart(); + } } internal void ReadSniSyncOverAsync() @@ -2780,6 +2816,18 @@ private sealed partial class PacketData public PacketData NextPacket; public PacketData PrevPacket; + public int TotalSize; + + internal int GetPacketDataOffset() + { + int previous = 0; + if (PrevPacket != null) + { + previous = PrevPacket.TotalSize; + } + return TotalSize - (TotalSize - previous); + } + internal void Clear() { Buffer = null; @@ -3078,14 +3126,18 @@ internal void Restore(TdsParserStateObject stateObj) private TdsParserStateObject _stateObj; private StateObjectData _replayStateData; + private StateObjectData _continueStateData; internal byte[] _plpBuffer; private PacketData _lastPacket; private PacketData _firstPacket; private PacketData _current; + private PacketData _continuePacket; private PacketData _sparePacket; + private bool? _continueSupported; + #if DEBUG private int _packetCounter; private int _rollingPend = 0; @@ -3127,6 +3179,17 @@ internal void CheckStack(string trace) } } #endif + public bool ContinueEnabled + { + get + { + if (_continueSupported == null) + { + _continueSupported = AppContext.TryGetSwitch("Switch.Microsoft.Data.SqlClient.UseExperimentalAsyncContinue", out bool value) ? value : false; + } + return _continueSupported.Value; + } + } internal void CloneNullBitmapInfo() { @@ -3211,7 +3274,6 @@ internal bool MoveNext() if (moved) { _stateObj.SetBuffer(_current.Buffer, 0, _current.Read); - _current.CheckDebugDataHash(); _stateObj._snapshotStatus = moveToMode; retval = true; } @@ -3228,6 +3290,22 @@ internal void MoveToStart() _stateObj.AssertValidState(); } + internal bool MoveToContinue() + { + if (ContinueEnabled) + { + if (_continuePacket != null && _continuePacket != _current) + { + _continueStateData.Restore(_stateObj); + _stateObj.SetBuffer(_current.Buffer, 0, _current.Read); + _stateObj._snapshotStatus = SnapshotStatus.ReplayRunning; + _stateObj.AssertValidState(); + return true; + } + } + return false; + } + internal void CaptureAsStart(TdsParserStateObject stateObj) { _firstPacket = null; @@ -3248,6 +3326,43 @@ internal void CaptureAsStart(TdsParserStateObject stateObj) AppendPacketData(stateObj._inBuff, stateObj._inBytesRead); } + internal void CaptureAsContinue(TdsParserStateObject stateObj) + { + if (ContinueEnabled) + { + Debug.Assert(_stateObj == stateObj); + if (_current is not null) + { + _continueStateData ??= new StateObjectData(); + _continueStateData.Capture(stateObj, trackStack: false); + _continuePacket = _current; + } + } + } + + internal void SetPacketPayloadSize(int size) + { + if (_current == null) + { + throw new InvalidOperationException(); + } + int total = 0; + if (_current.PrevPacket != null) + { + total = _current.PrevPacket.TotalSize; + } + _current.TotalSize = total + size; + } + + internal int GetPacketDataOffset() + { + if (_current == null) + { + throw new InvalidOperationException(); + } + return _current.GetPacketDataOffset(); + } + internal void Clear() { ClearState(); @@ -3259,6 +3374,7 @@ private void ClearPackets() PacketData packet = _firstPacket; _firstPacket = null; _lastPacket = null; + _continuePacket = null; _current = null; packet.Clear(); _sparePacket = packet; @@ -3267,6 +3383,7 @@ private void ClearPackets() private void ClearState() { _replayStateData.Clear(_stateObj); + _continueStateData?.Clear(_stateObj, trackStack: false); #if DEBUG _rollingPend = 0; _rollingPendCount = 0; From b3ee81b13b6c8188e8e48dae46ca2a01a0de36a9 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Sun, 23 Feb 2025 11:28:58 +0000 Subject: [PATCH 2/8] add Switch.Microsoft.Data.SqlClient.UseCompatibilityAsyncBehaviour switch --- .../Data/SqlClient/LocalAppContextSwitches.cs | 42 +++++++++++++++++++ .../Data/SqlClient/TdsParserStateObject.cs | 14 +------ .../TdsParserStateObject.TestHarness.cs | 11 +++++ 3 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index b66154a2ae..64a0083f0c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -21,6 +21,7 @@ private enum Tristate : byte internal const string UseMinimumLoginTimeoutString = @"Switch.Microsoft.Data.SqlClient.UseOneSecFloorInTimeoutCalculationDuringLogin"; internal const string LegacyVarTimeZeroScaleBehaviourString = @"Switch.Microsoft.Data.SqlClient.LegacyVarTimeZeroScaleBehaviour"; internal const string UseCompatibilityProcessSniString = @"Switch.Microsoft.Data.SqlClient.UseCompatibilityProcessSni"; + internal const string UseCompatibilityAsyncBehaviourString = @"Switch.Microsoft.Data.SqlClient.UseCompatibilityAsyncBehaviour"; // this field is accessed through reflection in tests and should not be renamed or have the type changed without refactoring NullRow related tests private static Tristate s_legacyRowVersionNullBehavior; @@ -30,6 +31,7 @@ private enum Tristate : byte // this field is accessed through reflection in Microsoft.Data.SqlClient.Tests.SqlParameterTests and should not be renamed or have the type changed without refactoring related tests private static Tristate s_legacyVarTimeZeroScaleBehaviour; private static Tristate s_useCompatProcessSni; + private static Tristate s_useCompatAsyncBehaviour; #if NET static LocalAppContextSwitches() @@ -85,6 +87,12 @@ public static bool DisableTNIRByDefault } } #endif + /// + /// In TdsParser the ProcessSni function changed significantly when the packet + /// multiplexing code needed for high speed multi-packet column values was added. + /// In case of compatibility problems this switch will change TdsParser to use + /// the previous version of the function. + /// public static bool UseCompatibilityProcessSni { get @@ -104,6 +112,40 @@ public static bool UseCompatibilityProcessSni } } + /// + /// In TdsParser the async multi-packet column value fetch behaviour is capable of + /// using a continue snapshot state in addition to the original replay from start + /// logic + /// This switch disables use of the continue snapshot state. This switch will always + /// return tru if is enables because the + /// continue state is not stable without the multiplexer. + /// + public static bool UseCompatibilityAsyncBehaviour + { + get + { + // async continue functionality is not stable without the packet multiplexer + // so if the multiplexer is disabled then this setting MUST return true + if (UseCompatibilityProcessSni) + { + return true; + } + + if (s_useCompatAsyncBehaviour == Tristate.NotInitialized) + { + if (AppContext.TryGetSwitch(UseCompatibilityAsyncBehaviourString, out bool returnedValue) && returnedValue) + { + s_useCompatAsyncBehaviour = Tristate.True; + } + else + { + s_useCompatAsyncBehaviour = Tristate.False; + } + } + return s_useCompatAsyncBehaviour == Tristate.True; + } + } + /// /// When using Encrypt=false in the connection string, a security warning is output to the console if the TLS version is 1.2 or lower. /// This warning can be suppressed by enabling this AppContext switch. 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 5c0895356a..329897b6d3 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -3136,8 +3136,6 @@ internal void Restore(TdsParserStateObject stateObj) private PacketData _continuePacket; private PacketData _sparePacket; - private bool? _continueSupported; - #if DEBUG private int _packetCounter; private int _rollingPend = 0; @@ -3179,17 +3177,7 @@ internal void CheckStack(string trace) } } #endif - public bool ContinueEnabled - { - get - { - if (_continueSupported == null) - { - _continueSupported = AppContext.TryGetSwitch("Switch.Microsoft.Data.SqlClient.UseExperimentalAsyncContinue", out bool value) ? value : false; - } - return _continueSupported.Value; - } - } + public bool ContinueEnabled => !LocalAppContextSwitches.UseCompatibilityAsyncBehaviour; internal void CloneNullBitmapInfo() { diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/TdsParserStateObject.TestHarness.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/TdsParserStateObject.TestHarness.cs index c512c3385b..7fecbf09e1 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/TdsParserStateObject.TestHarness.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/TdsParserStateObject.TestHarness.cs @@ -159,8 +159,19 @@ public static bool UseCompatibilityProcessSni return (bool)switchesType.GetProperty(nameof(UseCompatibilityProcessSni), BindingFlags.Public | BindingFlags.Static).GetValue(null); } } + + public static bool UseCompatibilityAsyncBehaviour + { + get + { + var switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); + + return (bool)switchesType.GetProperty(nameof(UseCompatibilityAsyncBehaviour), BindingFlags.Public | BindingFlags.Static).GetValue(null); + } + } } + #if NETFRAMEWORK private SniNativeWrapperImpl _native; internal SniNativeWrapperImpl SniNativeWrapper From 2886672fed442036a826649aa6a27345698d3ff4 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Mon, 24 Feb 2025 17:31:10 +0000 Subject: [PATCH 3/8] address feedback --- .../Data/SqlClient/LocalAppContextSwitches.cs | 10 ++++++---- .../Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index 64a0083f0c..23eae838ae 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -115,19 +115,21 @@ public static bool UseCompatibilityProcessSni /// /// In TdsParser the async multi-packet column value fetch behaviour is capable of /// using a continue snapshot state in addition to the original replay from start - /// logic + /// logic. /// This switch disables use of the continue snapshot state. This switch will always - /// return tru if is enables because the + /// return true if is enabled because the /// continue state is not stable without the multiplexer. /// public static bool UseCompatibilityAsyncBehaviour { get { - // async continue functionality is not stable without the packet multiplexer - // so if the multiplexer is disabled then this setting MUST return true if (UseCompatibilityProcessSni) { + // If ProcessSni compatibility mode has been enabled then the packet + // multiplexer has been disabled. The new async behaviour using continue + // point capture is only stable if the multiplexer is enabled so we must + // return true to enable compatibility async behaviour using only restarts. return true; } 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 329897b6d3..93ff4d950a 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -2825,7 +2825,7 @@ internal int GetPacketDataOffset() { previous = PrevPacket.TotalSize; } - return TotalSize - (TotalSize - previous); + return previous; } internal void Clear() From b93ac47dde0eaac074918e4398779e9afedbc412 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Tue, 4 Mar 2025 20:10:57 +0000 Subject: [PATCH 4/8] Operations status part3 dev --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 301 ++++++++++++++--- .../Microsoft/Data/SqlClient/SqlDataReader.cs | 7 +- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 302 +++++++++++++++--- .../Data/SqlClient/SqlCachedBuffer.cs | 50 ++- .../Data/SqlClient/TdsParserStateObject.cs | 297 ++++++++++++++--- .../TdsParserStateObjectHelper.cs | 42 ++- .../DataReaderTest/DataReaderStreamsTest.cs | 2 +- 7 files changed, 830 insertions(+), 171 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 a03a92ad67..2e9319338d 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 @@ -5998,34 +5998,17 @@ private TdsOperationStatus TryReadSqlStringValue(SqlBuffer value, byte type, int if (isPlp) { - char[] cc = null; - bool buffIsRented = false; - result = TryReadPlpUnicodeChars(ref cc, 0, length >> 1, stateObj, out length, supportRentedBuff: true, rentedBuff: ref buffIsRented); + result = TryReadPlpUnicodeCharsWithContinue( + stateObj, + length, + out string resultString + ); if (result == TdsOperationStatus.Done) { - if (length > 0) - { - s = new string(cc, 0, length); - } - else - { - s = string.Empty; - } + s = resultString; } - - if (buffIsRented) - { - // do not use clearArray:true on the rented array because it can be massively larger - // than the space we've used and we would incur performance clearing memory that - // we haven't used and can't leak out information. - // clear only the length that we know we have used. - cc.AsSpan(0, length).Clear(); - ArrayPool.Shared.Return(cc, clearArray: false); - cc = null; - } - - if (result != TdsOperationStatus.Done) + else { return result; } @@ -6379,9 +6362,7 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value, SqlMetaDataPriv md, } else { - //Debug.Assert(length > 0 && length < (long)(Int32.MaxValue), "Bad length for column"); - b = new byte[length]; - result = stateObj.TryReadByteArray(b, length); + result = TryReadByteArrayWithContinue(stateObj, length, out b); if (result != TdsOperationStatus.Done) { return result; @@ -6502,6 +6483,48 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value, SqlMetaDataPriv md, return TdsOperationStatus.Done; } + private TdsOperationStatus TryReadByteArrayWithContinue(TdsParserStateObject stateObj, int length, out byte[] bytes) + { + bytes = null; + int offset = 0; + byte[] temp = null; + (bool isAvailable, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses(); + if (isAvailable) + { + if (isContinuing || isStarting) + { + temp = (byte[])stateObj.TryTakeSnapshotStorage(); + Debug.Assert(bytes == null || bytes.Length == length, "stored buffer length must be null or must have been created with the correct length"); + } + if (temp != null) + { + offset = stateObj.GetSnapshotTotalSize(); + } + } + + + if (temp == null) + { + temp = new byte[length]; + } + + TdsOperationStatus result = stateObj.TryReadByteArray(temp, length, out _, offset, isStarting || isContinuing); + + if (result == TdsOperationStatus.Done) + { + bytes = temp; + } + else if (result == TdsOperationStatus.NeedMoreData) + { + if (isStarting || isContinuing) + { + stateObj.SetSnapshotStorage(temp); + } + } + + return result; + } + private TdsOperationStatus TryReadSqlDateTime(SqlBuffer value, byte tdsType, int length, byte scale, TdsParserStateObject stateObj) { Span datetimeBuffer = ((uint)length <= 16) ? stackalloc byte[16] : new byte[length]; @@ -8071,14 +8094,22 @@ internal TdsOperationStatus TryGetTokenLength(byte token, TdsParserStateObject s case TdsEnums.SQLVarCnt: if (0 != (token & 0x80)) { - ushort value; - result = stateObj.TryReadUInt16(out value); - if (result != TdsOperationStatus.Done) + if (stateObj.IsSnapshotContinuing()) { - tokenLength = 0; - return result; + tokenLength = stateObj.GetSnapshotStorageLength(); + Debug.Assert(tokenLength != 0, "stored buffer length on continue must contain the length of the data required for the token"); + } + else + { + ushort value; + result = stateObj.TryReadUInt16(out value); + if (result != TdsOperationStatus.Done) + { + tokenLength = 0; + return result; + } + tokenLength = value; } - tokenLength = value; return TdsOperationStatus.Done; } else if (0 == (token & 0x0c)) @@ -12872,6 +12903,85 @@ internal int ReadPlpUnicodeChars(ref char[] buff, int offst, int len, TdsParserS return charsRead; } + internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObject stateObj, int length, out string resultString) + { + resultString = null; + char[] temp = null; + bool buffIsRented = false; + int startOffset = 0; + (bool isAvailable, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses(); + + if (isAvailable) + { + if (isContinuing || isStarting) + { + temp = (char[])stateObj.TryTakeSnapshotStorage(); + Debug.Assert(temp == null || length == int.MaxValue || temp.Length == length, "stored buffer length must be null or must have been created with the correct length"); + } + if (temp != null) + { + startOffset = stateObj.GetSnapshotTotalSize(); + } + } + + TdsOperationStatus result = TryReadPlpUnicodeChars( + ref temp, + 0, + length >> 1, + stateObj, + out length, + supportRentedBuff: !isAvailable, // do not use the arraypool if we are going to keep the buffer in the snapshot + rentedBuff: ref buffIsRented, + startOffset, + isStarting || isContinuing + ); + + if (result == TdsOperationStatus.Done) + { + if (length > 0) + { + resultString = new string(temp, 0, length); + } + else + { + resultString = string.Empty; + } + + if (buffIsRented) + { + // do not use clearArray:true on the rented array because it can be massively larger + // than the space we've used and we would incur performance clearing memory that + // we haven't used and can't leak out information. + // clear only the length that we know we have used. + temp.AsSpan(0, length).Clear(); + ArrayPool.Shared.Return(temp, clearArray: false); + temp = null; + } + } + else if (result == TdsOperationStatus.NeedMoreData) + { + if (isStarting || isContinuing) + { + stateObj.SetSnapshotStorage(temp); + } + } + + return result; + } + + internal TdsOperationStatus TryReadPlpUnicodeChars( + ref char[] buff, + int offst, + int len, + TdsParserStateObject stateObj, + out int totalCharsRead, + bool supportRentedBuff, + ref bool rentedBuff + ) + { + return TryReadPlpUnicodeChars(ref buff, offst, len, stateObj, out totalCharsRead, supportRentedBuff, ref rentedBuff, 0, false); + } + // Reads the requested number of chars from a plp data stream, or the entire data if // requested length is -1 or larger than the actual length of data. First call to this method // should be preceeded by a call to ReadPlpLength or ReadDataLength. @@ -12883,11 +12993,13 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( TdsParserStateObject stateObj, out int totalCharsRead, bool supportRentedBuff, - ref bool rentedBuff) + ref bool rentedBuff, + int startOffsetByteCount, + bool writeDataSizeToSnapshot + ) { int charsRead = 0; int charsLeft = 0; - char[] newbuf; if (stateObj._longlen == 0) { @@ -12897,8 +13009,14 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( } Debug.Assert((ulong)stateObj._longlen != TdsEnums.SQL_PLP_NULL, "Out of sync plp read request"); - - Debug.Assert((buff == null && offst == 0) || (buff.Length >= offst + len), "Invalid length sent to ReadPlpUnicodeChars()!"); + Debug.Assert( + (buff == null && offst == 0) + || + (buff.Length >= offst + len) + || + (buff.Length == (startOffsetByteCount >> 1) + 1), + "Invalid length sent to ReadPlpUnicodeChars()!" + ); charsLeft = len; // If total length is known up front, the length isn't specified as unknown @@ -12919,6 +13037,11 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( } TdsOperationStatus result; + + bool partialReadInProgress = (startOffsetByteCount & 0x1) == 1; + bool restartingDataSizeCount = startOffsetByteCount == 0; + int currentPacketId = 0; + if (stateObj._longlenleft == 0) { result = stateObj.TryReadPlpLength(false, out _); @@ -12934,12 +13057,21 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( } } - totalCharsRead = 0; + totalCharsRead = (startOffsetByteCount >> 1); + charsLeft -= totalCharsRead; + offst = totalCharsRead; + + while (charsLeft > 0) { + if (partialReadInProgress) + { + goto resumePartialRead; + } charsRead = (int)Math.Min((stateObj._longlenleft + 1) >> 1, (ulong)charsLeft); if ((buff == null) || (buff.Length < (offst + charsRead))) { + char[] newbuf; bool returnRentedBufferAfterCopy = rentedBuff; if (supportRentedBuff && (offst + charsRead) < 1073741824) // 1 Gib { @@ -12962,6 +13094,7 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( } } buff = newbuf; + newbuf = null; } if (charsRead > 0) { @@ -12973,24 +13106,54 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( charsLeft -= charsRead; offst += charsRead; totalCharsRead += charsRead; + + if (writeDataSizeToSnapshot) + { + currentPacketId = IncrementSnapshotDataSize(stateObj, restartingDataSizeCount, currentPacketId, charsRead * 2); + } } - // Special case single byte left - if (stateObj._longlenleft == 1 && (charsLeft > 0)) + + resumePartialRead: + // Special case single byte + if ( + (stateObj._longlenleft == 1 || partialReadInProgress) + && (charsLeft > 0) + ) { - byte b1; - result = stateObj.TryReadByte(out b1); - if (result != TdsOperationStatus.Done) + byte b1 = 0; + byte b2 = 0; + if (partialReadInProgress) { - return result; + partialReadInProgress = false; + // we're resuming with a partial char in the buffer so we need to load the byte + // from the char buffer and put it into b1 so we can combine it with the second + // half later + b1 = (byte)(buff[offst] & 0x00ff); } - stateObj._longlenleft--; - result = stateObj.TryReadPlpLength(false, out _); - if (result != TdsOperationStatus.Done) + else { - return result; + result = stateObj.TryReadByte(out b1); + if (result != TdsOperationStatus.Done) + { + return result; + } + stateObj._longlenleft--; + if (writeDataSizeToSnapshot) + { + // we need to write the single b1 byte to the array because we may run out of data + // and need to wait for another packet + buff[offst] = (char)((b1 & 0xff)); + currentPacketId = IncrementSnapshotDataSize(stateObj, restartingDataSizeCount, currentPacketId, 1); + } + + result = stateObj.TryReadPlpLength(false, out _); + if (result != TdsOperationStatus.Done) + { + return result; + } + Debug.Assert((stateObj._longlenleft != 0), "ReadPlpUnicodeChars: Odd byte left at the end!"); } - Debug.Assert((stateObj._longlenleft != 0), "ReadPlpUnicodeChars: Odd byte left at the end!"); - byte b2; + result = stateObj.TryReadByte(out b2); if (result != TdsOperationStatus.Done) { @@ -13003,6 +13166,11 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( charsRead++; charsLeft--; totalCharsRead++; + + if (writeDataSizeToSnapshot) + { + currentPacketId = IncrementSnapshotDataSize(stateObj, restartingDataSizeCount, currentPacketId, 1); + } } if (stateObj._longlenleft == 0) { @@ -13015,11 +13183,44 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( } if (stateObj._longlenleft == 0) // Data read complete + { break; + } } return TdsOperationStatus.Done; + + static int IncrementSnapshotDataSize(TdsParserStateObject stateObj, bool resetting, int previousPacketId, int value) + { + int current = 0; + if (resetting) + { + int currentPacketId = stateObj.GetSnapshotPacketID(); + if (previousPacketId == currentPacketId) + { + // we have already reset it the first time we saw it so just add normally + current = stateObj.GetSnapshotDataSize(); + } + else + { + // a packet we haven't seen before, reset the size + current = 0; + } + + stateObj.SetSnapshotDataSize(current + value); + + // return new packetid so next time we see this packet we know it isn't new + return currentPacketId; + } + else + { + current = stateObj.GetSnapshotDataSize(); + stateObj.SetSnapshotDataSize(current + value); + return previousPacketId; + } + } } + internal int ReadPlpAnsiChars(ref char[] buff, int offst, int len, SqlMetaDataPriv metadata, TdsParserStateObject stateObj) { int charsRead = 0; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs index 60a8cf6153..4f308d3c5f 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs @@ -4524,7 +4524,12 @@ private TdsOperationStatus TryResetBlobState() #if DEBUG else { - Debug.Assert((_sharedState._columnDataBytesRemaining == 0 || _sharedState._columnDataBytesRemaining == -1) && _stateObj._longlen == 0, "Haven't read header yet, but column is partially read?"); + Debug.Assert( + (_sharedState._columnDataBytesRemaining == 0 || _sharedState._columnDataBytesRemaining == -1) + && + (_stateObj._longlen == 0 || _stateObj.IsSnapshotContinuing()), + "Haven't read header yet, but column is partially read?" + ); } #endif 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 06ffdcd629..fc0ef79db0 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 @@ -6470,34 +6470,17 @@ private TdsOperationStatus TryReadSqlStringValue(SqlBuffer value, byte type, int if (isPlp) { - char[] cc = null; - bool buffIsRented = false; - result = TryReadPlpUnicodeChars(ref cc, 0, length >> 1, stateObj, out length, supportRentedBuff: true, rentedBuff: ref buffIsRented); + result = TryReadPlpUnicodeCharsWithContinue( + stateObj, + length, + out string resultString + ); if (result == TdsOperationStatus.Done) { - if (length > 0) - { - s = new string(cc, 0, length); - } - else - { - s = string.Empty; - } + s = resultString; } - - if (buffIsRented) - { - // do not use clearArray:true on the rented array because it can be massively larger - // than the space we've used and we would incur performance clearing memory that - // we haven't used and can't leak out information. - // clear only the length that we know we have used. - cc.AsSpan(0, length).Clear(); - ArrayPool.Shared.Return(cc, clearArray: false); - cc = null; - } - - if (result != TdsOperationStatus.Done) + else { return result; } @@ -6856,9 +6839,7 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value, } else { - //Debug.Assert(length > 0 && length < (long)(Int32.MaxValue), "Bad length for column"); - b = new byte[length]; - result = stateObj.TryReadByteArray(b, length); + result = TryReadByteArrayWithContinue(stateObj, length, out b); if (result != TdsOperationStatus.Done) { return result; @@ -6979,6 +6960,48 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value, return TdsOperationStatus.Done; } + private TdsOperationStatus TryReadByteArrayWithContinue(TdsParserStateObject stateObj, int length, out byte[] bytes) + { + bytes = null; + int offset = 0; + byte[] temp = null; + (bool isAvailable, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses(); + if (isAvailable) + { + if (isContinuing || isStarting) + { + temp = (byte[])stateObj.TryTakeSnapshotStorage(); + Debug.Assert(bytes == null || bytes.Length == length, "stored buffer length must be null or must have been created with the correct length"); + } + if (temp != null) + { + offset = stateObj.GetSnapshotTotalSize(); + } + } + + + if (temp == null) + { + temp = new byte[length]; + } + + TdsOperationStatus result = stateObj.TryReadByteArray(temp, length, out _, offset, isStarting || isContinuing); + + if (result == TdsOperationStatus.Done) + { + bytes = temp; + } + else if (result == TdsOperationStatus.NeedMoreData) + { + if (isStarting || isContinuing) + { + stateObj.SetSnapshotStorage(temp); + } + } + + return result; + } + private TdsOperationStatus TryReadSqlDateTime(SqlBuffer value, byte tdsType, int length, byte scale, TdsParserStateObject stateObj) { Span datetimeBuffer = ((uint)length <= 16) ? stackalloc byte[16] : new byte[length]; @@ -8541,14 +8564,22 @@ internal TdsOperationStatus TryGetTokenLength(byte token, TdsParserStateObject s case TdsEnums.SQLVarCnt: if (0 != (token & 0x80)) { - ushort value; - result = stateObj.TryReadUInt16(out value); - if (result != TdsOperationStatus.Done) + if (stateObj.IsSnapshotContinuing()) { - tokenLength = 0; - return result; + tokenLength = stateObj.GetSnapshotStorageLength(); + Debug.Assert(tokenLength != 0, "stored buffer length on continue must contain the length of the data required for the token"); + } + else + { + ushort value; + result = stateObj.TryReadUInt16(out value); + if (result != TdsOperationStatus.Done) + { + tokenLength = 0; + return result; + } + tokenLength = value; } - tokenLength = value; return TdsOperationStatus.Done; } else if (0 == (token & 0x0c)) @@ -13415,6 +13446,85 @@ internal int ReadPlpUnicodeChars(ref char[] buff, int offst, int len, TdsParserS return charsRead; } + internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObject stateObj, int length, out string resultString) + { + resultString = null; + char[] temp = null; + bool buffIsRented = false; + int startOffset = 0; + (bool isAvailable, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses(); + + if (isAvailable) + { + if (isContinuing || isStarting) + { + temp = (char[])stateObj.TryTakeSnapshotStorage(); + Debug.Assert(temp == null || length == int.MaxValue || temp.Length == length, "stored buffer length must be null or must have been created with the correct length"); + } + if (temp != null) + { + startOffset = stateObj.GetSnapshotTotalSize(); + } + } + + TdsOperationStatus result = TryReadPlpUnicodeChars( + ref temp, + 0, + length >> 1, + stateObj, + out length, + supportRentedBuff: !isAvailable, // do not use the arraypool if we are going to keep the buffer in the snapshot + rentedBuff: ref buffIsRented, + startOffset, + isStarting || isContinuing + ); + + if (result == TdsOperationStatus.Done) + { + if (length > 0) + { + resultString = new string(temp, 0, length); + } + else + { + resultString = string.Empty; + } + + if (buffIsRented) + { + // do not use clearArray:true on the rented array because it can be massively larger + // than the space we've used and we would incur performance clearing memory that + // we haven't used and can't leak out information. + // clear only the length that we know we have used. + temp.AsSpan(0, length).Clear(); + ArrayPool.Shared.Return(temp, clearArray: false); + temp = null; + } + } + else if (result == TdsOperationStatus.NeedMoreData) + { + if (isStarting || isContinuing) + { + stateObj.SetSnapshotStorage(temp); + } + } + + return result; + } + + internal TdsOperationStatus TryReadPlpUnicodeChars( + ref char[] buff, + int offst, + int len, + TdsParserStateObject stateObj, + out int totalCharsRead, + bool supportRentedBuff, + ref bool rentedBuff + ) + { + return TryReadPlpUnicodeChars(ref buff, offst, len, stateObj, out totalCharsRead, supportRentedBuff, ref rentedBuff, 0, false); + } + // Reads the requested number of chars from a plp data stream, or the entire data if // requested length is -1 or larger than the actual length of data. First call to this method // should be preceeded by a call to ReadPlpLength or ReadDataLength. @@ -13426,12 +13536,14 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( TdsParserStateObject stateObj, out int totalCharsRead, bool supportRentedBuff, - ref bool rentedBuff) + ref bool rentedBuff, + int startOffsetByteCount, + bool writeDataSizeToSnapshot + ) { int charsRead = 0; int charsLeft = 0; - char[] newbuf; - + if (stateObj._longlen == 0) { Debug.Assert(stateObj._longlenleft == 0); @@ -13440,8 +13552,14 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( } Debug.Assert((ulong)stateObj._longlen != TdsEnums.SQL_PLP_NULL, "Out of sync plp read request"); - - Debug.Assert((buff == null && offst == 0) || (buff.Length >= offst + len), "Invalid length sent to ReadPlpUnicodeChars()!"); + Debug.Assert( + (buff == null && offst == 0) + || + (buff.Length >= offst + len) + || + (buff.Length == (startOffsetByteCount >> 1) + 1), + "Invalid length sent to ReadPlpUnicodeChars()!" + ); charsLeft = len; // If total length is known up front, the length isn't specified as unknown @@ -13462,6 +13580,11 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( } TdsOperationStatus result; + + bool partialReadInProgress = (startOffsetByteCount & 0x1) == 1; + bool restartingDataSizeCount = startOffsetByteCount == 0; + int currentPacketId = 0; + if (stateObj._longlenleft == 0) { result = stateObj.TryReadPlpLength(false, out _); @@ -13477,12 +13600,21 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( } } - totalCharsRead = 0; + totalCharsRead = (startOffsetByteCount >> 1); + charsLeft -= totalCharsRead; + offst = totalCharsRead; + + while (charsLeft > 0) { + if (partialReadInProgress) + { + goto resumePartialRead; + } charsRead = (int)Math.Min((stateObj._longlenleft + 1) >> 1, (ulong)charsLeft); if ((buff == null) || (buff.Length < (offst + charsRead))) { + char[] newbuf; bool returnRentedBufferAfterCopy = rentedBuff; if (supportRentedBuff && (offst + charsRead) < 1073741824) // 1 Gib { @@ -13505,6 +13637,7 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( } } buff = newbuf; + newbuf = null; } if (charsRead > 0) { @@ -13516,24 +13649,54 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( charsLeft -= charsRead; offst += charsRead; totalCharsRead += charsRead; + + if (writeDataSizeToSnapshot) + { + currentPacketId = IncrementSnapshotDataSize(stateObj, restartingDataSizeCount, currentPacketId, charsRead * 2); + } } - // Special case single byte left - if (stateObj._longlenleft == 1 && (charsLeft > 0)) + + resumePartialRead: + // Special case single byte + if ( + (stateObj._longlenleft == 1 || partialReadInProgress ) + && (charsLeft > 0) + ) { - byte b1; - result = stateObj.TryReadByte(out b1); - if (result != TdsOperationStatus.Done) + byte b1=0; + byte b2=0; + if (partialReadInProgress) { - return result; + partialReadInProgress = false; + // we're resuming with a partial char in the buffer so we need to load the byte + // from the char buffer and put it into b1 so we can combine it with the second + // half later + b1 = (byte)(buff[offst] & 0x00ff); } - stateObj._longlenleft--; - result = stateObj.TryReadPlpLength(false, out _); - if (result != TdsOperationStatus.Done) + else { - return result; + result = stateObj.TryReadByte(out b1); + if (result != TdsOperationStatus.Done) + { + return result; + } + stateObj._longlenleft--; + if (writeDataSizeToSnapshot) + { + // we need to write the single b1 byte to the array because we may run out of data + // and need to wait for another packet + buff[offst] = (char)((b1 & 0xff)); + currentPacketId = IncrementSnapshotDataSize(stateObj, restartingDataSizeCount, currentPacketId, 1); + } + + result = stateObj.TryReadPlpLength(false, out _); + if (result != TdsOperationStatus.Done) + { + return result; + } + Debug.Assert((stateObj._longlenleft != 0), "ReadPlpUnicodeChars: Odd byte left at the end!"); } - Debug.Assert((stateObj._longlenleft != 0), "ReadPlpUnicodeChars: Odd byte left at the end!"); - byte b2; + result = stateObj.TryReadByte(out b2); if (result != TdsOperationStatus.Done) { @@ -13546,6 +13709,11 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( charsRead++; charsLeft--; totalCharsRead++; + + if (writeDataSizeToSnapshot) + { + currentPacketId = IncrementSnapshotDataSize(stateObj, restartingDataSizeCount, currentPacketId, 1); + } } if (stateObj._longlenleft == 0) { @@ -13558,9 +13726,41 @@ internal TdsOperationStatus TryReadPlpUnicodeChars( } if (stateObj._longlenleft == 0) // Data read complete + { break; + } } return TdsOperationStatus.Done; + + static int IncrementSnapshotDataSize(TdsParserStateObject stateObj, bool resetting, int previousPacketId, int value) + { + int current = 0; + if (resetting) + { + int currentPacketId = stateObj.GetSnapshotPacketID(); + if (previousPacketId == currentPacketId) + { + // we have already reset it the first time we saw it so just add normally + current = stateObj.GetSnapshotDataSize(); + } + else + { + // a packet we haven't seen before, reset the size + current = 0; + } + + stateObj.SetSnapshotDataSize(current + value); + + // return new packetid so next time we see this packet we know it isn't new + return currentPacketId; + } + else + { + current = stateObj.GetSnapshotDataSize(); + stateObj.SetSnapshotDataSize(current + value); + return previousPacketId; + } + } } internal int ReadPlpAnsiChars(ref char[] buff, int offst, int len, SqlMetaDataPriv metadata, TdsParserStateObject stateObj) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs index 2b656501a5..ce248797f0 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs @@ -37,11 +37,26 @@ private SqlCachedBuffer(List cachedBytes) /// internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser parser, TdsParserStateObject stateObj, out SqlCachedBuffer buffer) { - byte[] byteArr; - - List cachedBytes = new(); buffer = null; + (bool isAvailable, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses(); + + List cachedBytes = null; + if (isAvailable) + { + cachedBytes = stateObj.TryTakeSnapshotStorage() as List; + if (cachedBytes != null && !isStarting && !isContinuing) + { + stateObj.SetSnapshotStorage(null); + } + } + + if (cachedBytes == null) + { + cachedBytes = new List(); + } + + // the very first length is already read. TdsOperationStatus result = parser.TryPlpBytesLeft(stateObj, out ulong plplength); if (result != TdsOperationStatus.Done) @@ -49,6 +64,7 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser return result; } + // For now we only handle Plp data from the parser directly. Debug.Assert(metadata.metaType.IsPlp, "SqlCachedBuffer call on a non-plp data"); do @@ -59,13 +75,25 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser } do { + bool returnAfterAdd = false; int cb = (plplength > (ulong)MaxChunkSize) ? MaxChunkSize : (int)plplength; - byteArr = new byte[cb]; - result = stateObj.TryReadPlpBytes(ref byteArr, 0, cb, out cb); + byte[] byteArr = new byte[cb]; + // pass false for the writeDataSizeToSnapshot parameter because we want to only take data + // from the current packet and not try to do a continue-capable multi packet read + result = stateObj.TryReadPlpBytes(ref byteArr, 0, cb, out cb, writeDataSizeToSnapshot: false); if (result != TdsOperationStatus.Done) { - return result; + if (result == TdsOperationStatus.NeedMoreData && isAvailable && cb == byteArr.Length) + { + // succeeded in getting the data but failed to find the next plp length + returnAfterAdd = true; + } + else + { + return result; + } } + Debug.Assert(cb == byteArr.Length); if (cachedBytes.Count == 0) { @@ -74,6 +102,16 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser } cachedBytes.Add(byteArr); plplength -= (ulong)cb; + + if (returnAfterAdd) + { + if (isStarting || isContinuing) + { + stateObj.SetSnapshotStorage(cachedBytes); + } + return result; + } + } while (plplength > 0); result = parser.TryPlpBytesLeft(stateObj, out plplength); 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 93ff4d950a..1e0993cdb6 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -69,7 +69,8 @@ private enum SnapshotStatus { NotActive, ReplayStarting, - ReplayRunning + ReplayRunning, + ContinueRunning } private const int AttentionTimeoutSeconds = 5; @@ -1199,12 +1200,17 @@ internal TdsOperationStatus TryPeekByte(out byte value) // bytes from the in buffer. public TdsOperationStatus TryReadByteArray(Span buff, int len) { - return TryReadByteArray(buff, len, out _); + return TryReadByteArray(buff, len, out _, 0, false); + } + + public TdsOperationStatus TryReadByteArray(Span buff, int len, out int totalRead) + { + return TryReadByteArray(buff, len, out totalRead, 0, false); } // NOTE: This method must be retriable WITHOUT replaying a snapshot // Every time you call this method increment the offset and decrease len by the value of totalRead - public TdsOperationStatus TryReadByteArray(Span buff, int len, out int totalRead) + public TdsOperationStatus TryReadByteArray(Span buff, int len, out int totalRead, int startOffset, bool writeDataSizeToSnapshot) { totalRead = 0; @@ -1229,6 +1235,10 @@ public TdsOperationStatus TryReadByteArray(Span buff, int len, out int tot Debug.Assert(buff.IsEmpty || buff.Length >= len, "Invalid length sent to ReadByteArray()!"); + + totalRead += startOffset; + len -= startOffset; + // loop through and read up to array length while (len > 0) { @@ -1255,6 +1265,11 @@ public TdsOperationStatus TryReadByteArray(Span buff, int len, out int tot _inBytesPacket -= bytesToRead; len -= bytesToRead; + if (writeDataSizeToSnapshot) + { + SetSnapshotDataSize(bytesToRead); + } + AssertValidState(); } @@ -1669,6 +1684,7 @@ internal TdsOperationStatus TryReadStringWithEncoding(int length, System.Text.En } byte[] buf = null; int offset = 0; + (bool isAvailable, bool isStarting, bool isContinuing) = GetSnapshotStatuses(); if (isPlp) { @@ -1685,21 +1701,40 @@ internal TdsOperationStatus TryReadStringWithEncoding(int length, System.Text.En { if (((_inBytesUsed + length) > _inBytesRead) || (_inBytesPacket < length)) { - if (_bTmp == null || _bTmp.Length < length) + int startOffset = 0; + if (isAvailable) { - _bTmp = new byte[length]; + if (isContinuing || isStarting) + { + buf = (byte[])TryTakeSnapshotStorage(); + Debug.Assert(buf == null || buf.Length == length, "stored buffer length must be null or must have been created with the correct length"); + } + if (buf != null) + { + startOffset = GetSnapshotTotalSize(); + } } - TdsOperationStatus result = TryReadByteArray(_bTmp, length); + if (buf == null || buf.Length < length) + { + buf = new byte[length]; + } + + TdsOperationStatus result = TryReadByteArray(buf, length, out _, startOffset, isAvailable); + if (result != TdsOperationStatus.Done) { + if (result == TdsOperationStatus.NeedMoreData) + { + if (isStarting || isContinuing) + { + SetSnapshotStorage(buf); + } + } value = null; return result; } - // assign local to point to parser scratch buffer - buf = _bTmp; - AssertValidState(); } else @@ -1813,19 +1848,20 @@ internal int ReadPlpBytesChunk(byte[] buff, int offset, int len) return value; } + internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len, out int totalBytesRead) + { + return TryReadPlpBytes(ref buff, offset, len, out totalBytesRead, IsSnapshotAvailable()); + } // Reads the requested number of bytes from a plp data stream, or the entire data if // requested length is -1 or larger than the actual length of data. First call to this method // should be preceeded by a call to ReadPlpLength or ReadDataLength. // Returns the actual bytes read. // NOTE: This method must be retriable WITHOUT replaying a snapshot // Every time you call this method increment the offset and decrease len by the value of totalBytesRead - internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len, out int totalBytesRead) + internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len, out int totalBytesRead, bool writeDataSizeToSnapshot) { totalBytesRead = 0; - int bytesRead; - int bytesLeft; - byte[] newbuf; - + if (_longlen == 0) { Debug.Assert(_longlenleft == 0); @@ -1842,18 +1878,17 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len Debug.Assert(_longlen != TdsEnums.SQL_PLP_NULL, "Out of sync plp read request"); Debug.Assert((buff == null && offset == 0) || (buff.Length >= offset + len), "Invalid length sent to ReadPlpBytes()!"); - bytesLeft = len; + int bytesLeft = len; // 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) { - if (_snapshot != null && _snapshotStatus != SnapshotStatus.NotActive) + if (writeDataSizeToSnapshot) { // if there is a snapshot and it contains a stored plp buffer take it // and try to use it if it is the right length - buff = _snapshot._plpBuffer; - _snapshot._plpBuffer = null; - if (_snapshot.ContinueEnabled) + buff = (byte[])TryTakeSnapshotStorage(); + if (buff != null) { offset = _snapshot.GetPacketDataOffset(); totalBytesRead = offset; @@ -1895,12 +1930,13 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len if (buff.Length < (offset + bytesToRead)) { // Grow the array - newbuf = new byte[offset + bytesToRead]; + byte[] newbuf = new byte[offset + bytesToRead]; Buffer.BlockCopy(buff, 0, newbuf, 0, offset); buff = newbuf; + newbuf = null; } - TdsOperationStatus result = TryReadByteArray(buff.AsSpan(offset), bytesToRead, out bytesRead); + TdsOperationStatus result = TryReadByteArray(buff.AsSpan(offset), bytesToRead, out int bytesRead); Debug.Assert(bytesRead <= bytesLeft, "Read more bytes than we needed"); Debug.Assert((ulong)bytesRead <= _longlenleft, "Read more bytes than is available"); @@ -1910,22 +1946,22 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len _longlenleft -= (ulong)bytesRead; if (result != TdsOperationStatus.Done) { - if (_snapshot != null) + if (writeDataSizeToSnapshot) { // a partial read has happened so store the target buffer in the snapshot // so it can be re-used when another packet arrives and we read again - _snapshot._plpBuffer = buff; + SetSnapshotStorage(buff); } return result; } else { - if (_snapshot != null) + if (writeDataSizeToSnapshot) { - _snapshot._plpBuffer = buff; - if (_snapshotStatus != SnapshotStatus.NotActive && _snapshot.ContinueEnabled) + SetSnapshotStorage(buff); + if (_snapshot.ContinueEnabled) { - StoreReadPlpBytesProgress(this, bytesRead); + SetSnapshotDataSize(bytesRead); stored = true; } } @@ -1937,9 +1973,9 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len result = TryReadPlpLength(false, out _); if (result != TdsOperationStatus.Done) { - if (!stored && result == TdsOperationStatus.NeedMoreData && _snapshot != null && _snapshotStatus != SnapshotStatus.NotActive && _snapshot.ContinueEnabled) + if (result == TdsOperationStatus.NeedMoreData && writeDataSizeToSnapshot && !stored) { - StoreReadPlpBytesProgress(this, bytesRead); + SetSnapshotDataSize(bytesRead); } return result; } @@ -1954,14 +1990,6 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len } } return TdsOperationStatus.Done; - - static void StoreReadPlpBytesProgress(TdsParserStateObject stateObject, int size) - { - Debug.Assert(stateObject._snapshot != null, "_snapshot must exist to store plp read progress"); - Debug.Assert(stateObject._snapshotStatus != SnapshotStatus.NotActive, "_snapshot must be active to store plp read progress"); - - stateObject._snapshot.SetPacketPayloadSize(size); - } } ///////////////////////////////////////// @@ -2024,6 +2052,18 @@ internal TdsOperationStatus TryReadNetworkPacket() stackTrace = Environment.StackTrace; } #endif + bool capturedAsContinue = false; + if (_snapshotStatus == SnapshotStatus.ReplayRunning || _snapshotStatus == SnapshotStatus.ReplayStarting) + { + if (_bTmpRead == 0 && _partialHeaderBytesRead == 0 && _longlenleft == 0 && _snapshot.ContinueEnabled) + { + // no temp between packets + // mark this point as continue-able + _snapshot.CaptureAsContinue(this); + capturedAsContinue = true; + } + } + if (_snapshot.MoveNext()) { #if DEBUG @@ -2042,11 +2082,12 @@ internal TdsOperationStatus TryReadNetworkPacket() _lastStack = stackTrace; } #endif - if (_bTmpRead == 0 && _partialHeaderBytesRead == 0 && _longlenleft == 0 && _snapshot.ContinueEnabled) + if (_bTmpRead == 0 && _partialHeaderBytesRead == 0 && _longlenleft == 0 && _snapshot.ContinueEnabled && !capturedAsContinue) { // no temp between packets // mark this point as continue-able _snapshot.CaptureAsContinue(this); + capturedAsContinue = true; } } } @@ -2791,6 +2832,7 @@ internal void SetSnapshot() snapshot.Clear(); } _snapshot = snapshot; + Debug.Assert(_snapshot._storage == null); _snapshot.CaptureAsStart(this); _snapshotStatus = SnapshotStatus.NotActive; } @@ -2801,13 +2843,103 @@ internal void ResetSnapshot() { StateSnapshot snapshot = _snapshot; _snapshot = null; + Debug.Assert(snapshot._storage == null); snapshot.Clear(); Interlocked.CompareExchange(ref _cachedSnapshot, snapshot, null); } _snapshotStatus = SnapshotStatus.NotActive; } - sealed partial class StateSnapshot + internal bool IsSnapshotAvailable() + { + return _snapshot != null && _snapshot.ContinueEnabled; + } + /// + /// Returns true if the state object is in the state of continuing from a previously stored snapshot packet + /// meaning that consumers should resume from the point where they last needed more data instead of beginning + /// to process packets in the snapshot from the beginning again + /// + /// + internal bool IsSnapshotContinuing() + { + return _snapshot != null && + _snapshot.ContinueEnabled && + _snapshotStatus == TdsParserStateObject.SnapshotStatus.ContinueRunning; + } + + internal (bool IsAvailable, bool IsStarting, bool IsContinuing) GetSnapshotStatuses() + { + bool isAvailable = _snapshot != null && _snapshot.ContinueEnabled; + bool isStarting = false; + bool isContinuing = false; + if (isAvailable) + { + isStarting = _snapshotStatus == SnapshotStatus.ReplayStarting; + isContinuing = _snapshotStatus == SnapshotStatus.ContinueRunning; + } + return (isAvailable, isStarting, isContinuing); + } + + internal int GetSnapshotStorageLength() + { + Debug.Assert(_snapshot != null && _snapshot.ContinueEnabled, "should not access snapshot accessor functions without first checking that the snapshot is available"); + return (_snapshot?._storage as IList)?.Count ?? 0; + } + + internal object TryTakeSnapshotStorage() + { + Debug.Assert(_snapshot != null && _snapshot.ContinueEnabled, "should not access snapshot accessor functions without first checking that the snapshot is present"); + object buffer = null; + if (_snapshot != null) + { + buffer = _snapshot._storage; + _snapshot._storage = null; + } + return buffer; + } + + internal void SetSnapshotStorage(object buffer) + { + Debug.Assert(_snapshot != null && _snapshot.ContinueEnabled, "should not access snapshot accessor functions without first checking that the snapshot is available"); + Debug.Assert(_snapshot._storage == null, "should not overwrite snapshot stored buffer"); + if (_snapshot != null) + { + _snapshot._storage = buffer; + } + } + + /// + /// stores the countOfBytesCopiedFromCurrentPacket of bytes copied from the current packet in the snapshot allowing the total + /// countOfBytesCopiedFromCurrentPacket to be calculated + /// + /// + internal void SetSnapshotDataSize(int countOfBytesCopiedFromCurrentPacket) + { + Debug.Assert(_snapshot != null && _snapshot.ContinueEnabled, "_snapshot must exist to store packet data size"); + _snapshot.SetPacketDataSize(countOfBytesCopiedFromCurrentPacket); + } + + internal int GetSnapshotTotalSize() + { + Debug.Assert(_snapshot != null && _snapshot.ContinueEnabled, "_snapshot must exist to read total size"); + Debug.Assert(_snapshotStatus != SnapshotStatus.NotActive, "_snapshot must be active read total size"); + return _snapshot.GetPacketDataOffset(); + } + + internal int GetSnapshotDataSize() + { + Debug.Assert(_snapshot != null && _snapshot.ContinueEnabled, "_snapshot must exist to read packet data size"); + Debug.Assert(_snapshotStatus != SnapshotStatus.NotActive, "_snapshot must be active read packet data size"); + return _snapshot.GetPacketDataSize(); + } + + internal int GetSnapshotPacketID() + { + Debug.Assert(_snapshot != null && _snapshot.ContinueEnabled, "_snapshot must exist to read packet data size"); + return _snapshot.GetPacketID(); + } + + internal sealed partial class StateSnapshot { private sealed partial class PacketData { @@ -2816,17 +2948,32 @@ private sealed partial class PacketData public PacketData NextPacket; public PacketData PrevPacket; - public int TotalSize; + /// + /// Stores the data size of the total snapshot so far so that enumeration is not needed + /// to get the offset of the previous packet data in the stored buffer + /// + public int RunningDataSize; + + public int PacketID => Packet.GetIDFromHeader(Buffer.AsSpan(0, TdsEnums.HEADER_LEN)); internal int GetPacketDataOffset() { int previous = 0; if (PrevPacket != null) { - previous = PrevPacket.TotalSize; + previous = PrevPacket.RunningDataSize; } return previous; } + internal int GetPacketDataSize() + { + int previous = 0; + if (PrevPacket != null) + { + previous = PrevPacket.RunningDataSize; + } + return Math.Max(RunningDataSize - previous, 0); + } internal void Clear() { @@ -2974,6 +3121,8 @@ public string Status public ReadOnlySpan Data => _data.Buffer.AsSpan(TdsEnums.HEADER_LEN); + public int RunningDataSize => _data.RunningDataSize; + public PacketData NextPacket => _data.NextPacket; public PacketData PrevPacket => _data.PrevPacket; } @@ -2982,8 +3131,6 @@ public string Status public string Stack; public byte[] Hash; - public int PacketID => Packet.GetIDFromHeader(Buffer.AsSpan(0, TdsEnums.HEADER_LEN)); - public int SPID => Packet.GetSpidFromHeader(Buffer.AsSpan(0, TdsEnums.HEADER_LEN)); public bool IsEOM => Packet.GetIsEOMFromHeader(Buffer.AsSpan(0, TdsEnums.HEADER_LEN)); @@ -3038,6 +3185,11 @@ partial void CheckDebugDataHashImpl() } } } + + public override string ToString() + { + return $"{PacketID}({GetPacketDataOffset()},{GetPacketDataSize()})"; + } } #endif @@ -3128,7 +3280,7 @@ internal void Restore(TdsParserStateObject stateObj) private StateObjectData _replayStateData; private StateObjectData _continueStateData; - internal byte[] _plpBuffer; + internal object _storage; private PacketData _lastPacket; private PacketData _firstPacket; @@ -3255,6 +3407,10 @@ internal bool MoveNext() } else if (_current.NextPacket != null) { + if (_stateObj._snapshotStatus == SnapshotStatus.ContinueRunning) + { + moveToMode = SnapshotStatus.ContinueRunning; + } _current = _current.NextPacket; moved = true; } @@ -3286,7 +3442,7 @@ internal bool MoveToContinue() { _continueStateData.Restore(_stateObj); _stateObj.SetBuffer(_current.Buffer, 0, _current.Read); - _stateObj._snapshotStatus = SnapshotStatus.ReplayRunning; + _stateObj._snapshotStatus = SnapshotStatus.ContinueRunning; _stateObj.AssertValidState(); return true; } @@ -3328,27 +3484,60 @@ internal void CaptureAsContinue(TdsParserStateObject stateObj) } } - internal void SetPacketPayloadSize(int size) + internal void SetPacketDataSize(int size) { - if (_current == null) + PacketData target = _current; + // special case for the start of a snapshot when we expect to have only a single packet + // but have no current packet because we haven't started to replay yet. + if ( + target == null && + _firstPacket != null && + _firstPacket == _lastPacket + ) + { + target = _firstPacket; + } + + if (target == null) { throw new InvalidOperationException(); } int total = 0; - if (_current.PrevPacket != null) + if (target.PrevPacket != null) { - total = _current.PrevPacket.TotalSize; + total = target.PrevPacket.RunningDataSize; } - _current.TotalSize = total + size; + target.RunningDataSize = total + size; } internal int GetPacketDataOffset() { - if (_current == null) + int offset = 0; + if (_current != null) { - throw new InvalidOperationException(); + offset = _current.GetPacketDataOffset(); + } + return offset; + } + + internal int GetPacketDataSize() + { + int offset = 0; + if (_current != null) + { + offset = _current.GetPacketDataSize(); + } + return offset; + } + + internal int GetPacketID() + { + int id = 0; + if (_current != null) + { + id = _current.PacketID; } - return _current.GetPacketDataOffset(); + return id; } internal void Clear() @@ -3370,6 +3559,8 @@ private void ClearPackets() private void ClearState() { + Debug.Assert(_storage == null); + _storage = null; _replayStateData.Clear(_stateObj); _continueStateData?.Clear(_stateObj, trackStack: false); #if DEBUG diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/TdsParserStateObjectHelper.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/TdsParserStateObjectHelper.cs index 2d6e234f55..9f484093b1 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/TdsParserStateObjectHelper.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/Common/SystemDataInternals/TdsParserStateObjectHelper.cs @@ -3,22 +3,38 @@ // See the LICENSE file in the project root for more information. using System; +using System.Diagnostics; using System.Reflection; namespace Microsoft.Data.SqlClient.ManualTesting.Tests.SystemDataInternals { internal static class TdsParserStateObjectHelper { - private static readonly Assembly s_systemDotData = typeof(Microsoft.Data.SqlClient.SqlConnection).GetTypeInfo().Assembly; - private static readonly Type s_tdsParserStateObject = s_systemDotData.GetType("Microsoft.Data.SqlClient.TdsParserStateObject"); - private static readonly FieldInfo s_forceAllPends = s_tdsParserStateObject.GetField("s_forceAllPends", BindingFlags.Static | BindingFlags.NonPublic); - private static readonly FieldInfo s_skipSendAttention = s_tdsParserStateObject.GetField("s_skipSendAttention", BindingFlags.Static | BindingFlags.NonPublic); - private static readonly FieldInfo s_forceSyncOverAsyncAfterFirstPend = s_tdsParserStateObject.GetField("s_forceSyncOverAsyncAfterFirstPend", BindingFlags.Static | BindingFlags.NonPublic); - private static readonly FieldInfo s_failAsyncPends = s_tdsParserStateObject.GetField("s_failAsyncPends", BindingFlags.Static | BindingFlags.NonPublic); - private static readonly FieldInfo s_forcePendingReadsToWaitForUser = s_tdsParserStateObject.GetField("s_forcePendingReadsToWaitForUser", BindingFlags.Static | BindingFlags.NonPublic); - private static readonly Type s_tdsParserStateObjectManaged = s_systemDotData.GetType("Microsoft.Data.SqlClient.SNI.TdsParserStateObjectManaged"); - private static readonly FieldInfo s_tdsParserStateObjectManagedSessionHandle = s_tdsParserStateObjectManaged.GetField("_sessionHandle", BindingFlags.Instance | BindingFlags.NonPublic); + private static readonly Assembly s_systemDotData; + private static readonly Type s_tdsParserStateObject; + private static readonly FieldInfo s_forceAllPends; + private static readonly FieldInfo s_skipSendAttention; + private static readonly FieldInfo s_forceSyncOverAsyncAfterFirstPend; + private static readonly FieldInfo s_failAsyncPends; + private static readonly FieldInfo s_forcePendingReadsToWaitForUser; + private static readonly Type s_tdsParserStateObjectManaged; + private static readonly FieldInfo s_tdsParserStateObjectManagedSessionHandle; + static TdsParserStateObjectHelper() + { + s_systemDotData = typeof(Microsoft.Data.SqlClient.SqlConnection).GetTypeInfo().Assembly; + s_tdsParserStateObject = s_systemDotData.GetType("Microsoft.Data.SqlClient.TdsParserStateObject"); + s_forceAllPends = s_tdsParserStateObject.GetField("s_forceAllPends", BindingFlags.Static | BindingFlags.NonPublic); + s_skipSendAttention = s_tdsParserStateObject.GetField("s_skipSendAttention", BindingFlags.Static | BindingFlags.NonPublic); + s_forceSyncOverAsyncAfterFirstPend = s_tdsParserStateObject.GetField("s_forceSyncOverAsyncAfterFirstPend", BindingFlags.Static | BindingFlags.NonPublic); + s_failAsyncPends = s_tdsParserStateObject.GetField("s_failAsyncPends", BindingFlags.Static | BindingFlags.NonPublic); + s_forcePendingReadsToWaitForUser = s_tdsParserStateObject.GetField("s_forcePendingReadsToWaitForUser", BindingFlags.Static | BindingFlags.NonPublic); + s_tdsParserStateObjectManaged = s_systemDotData.GetType("Microsoft.Data.SqlClient.SNI.TdsParserStateObjectManaged"); + if (s_tdsParserStateObjectManaged != null) + { + s_tdsParserStateObjectManagedSessionHandle = s_tdsParserStateObjectManaged.GetField("_sessionHandle", BindingFlags.Instance | BindingFlags.NonPublic); + } + } internal static bool ForceAllPends { get { return (bool)s_forceAllPends.GetValue(null); } @@ -52,9 +68,17 @@ internal static bool FailAsyncPends private static void VerifyObjectIsTdsParserStateObject(object stateObject) { if (stateObject == null) + { throw new ArgumentNullException(nameof(stateObject)); + } + if (s_tdsParserStateObjectManaged == null) + { + throw new ArgumentException("Library being tested does not implement TdsParserStateObjectManaged", nameof(stateObject)); + } if (!s_tdsParserStateObjectManaged.IsInstanceOfType(stateObject)) + { throw new ArgumentException("Object provided was not a TdsParserStateObjectManaged", nameof(stateObject)); + } } internal static object GetSessionHandle(object stateObject) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderStreamsTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderStreamsTest.cs index a180c21b3b..adc20b5e92 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderStreamsTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderStreamsTest.cs @@ -20,7 +20,7 @@ public static class DataReaderStreamsTest [MemberData(nameof(GetCommandBehavioursAndIsAsync))] public static async Task GetFieldValueAsync_OfStream(CommandBehavior behavior, bool isExecuteAsync) { - const int PacketSize = 512; // force minimun packet size so that the test data spans multiple packets to test sequential access spanning + const int PacketSize = 512; // force minimum packet size so that the test data spans multiple packets to test sequential access spanning string connectionString = SetConnectionStringPacketSize(DataTestUtility.TCPConnectionString, PacketSize); byte[] originalData = CreateBinaryData(PacketSize, forcedPacketCount: 4); string query = CreateBinaryDataQuery(originalData); From 0fbae86f4e90dfd21b5839ac886c783c0b38657c Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Tue, 4 Mar 2025 02:08:19 +0000 Subject: [PATCH 5/8] address asserts and refine TryReadPlpBytes --- .../Microsoft/Data/SqlClient/SqlDataReader.cs | 7 +++- .../Data/SqlClient/TdsParserStateObject.cs | 41 +++++++++++-------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs index c825af39ad..7fdae67cac 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs @@ -4206,7 +4206,12 @@ private TdsOperationStatus TryResetBlobState() #if DEBUG else { - Debug.Assert((_sharedState._columnDataBytesRemaining == 0 || _sharedState._columnDataBytesRemaining == -1) && _stateObj._longlen == 0, "Haven't read header yet, but column is partially read?"); + Debug.Assert( + (_sharedState._columnDataBytesRemaining == 0 || _sharedState._columnDataBytesRemaining == -1) + && + (_stateObj._longlen == 0 || _stateObj.IsSnapshotContinuing()), + "Haven't read header yet, but column is partially read?" + ); } #endif 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 1e0993cdb6..e83c77a3b7 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1850,7 +1850,8 @@ internal int ReadPlpBytesChunk(byte[] buff, int offset, int len) internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len, out int totalBytesRead) { - return TryReadPlpBytes(ref buff, offset, len, out totalBytesRead, IsSnapshotAvailable()); + ( _, bool isStarting, bool isContinuing) = GetSnapshotStatuses(); + return TryReadPlpBytes(ref buff, offset, len, out totalBytesRead, isStarting || isContinuing); } // Reads the requested number of bytes from a plp data stream, or the entire data if // requested length is -1 or larger than the actual length of data. First call to this method @@ -1894,6 +1895,11 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len totalBytesRead = offset; } } + else if (_snapshot != null) + { + // legacy replay path perf optimization + buff = (byte[])TryTakeSnapshotStorage(); + } if ((ulong)(buff?.Length ?? 0) != _longlen) { @@ -1925,7 +1931,6 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len while (bytesLeft > 0) { - bool stored = false; int bytesToRead = (int)Math.Min(_longlenleft, (ulong)bytesLeft); if (buff.Length < (offset + bytesToRead)) { @@ -1951,20 +1956,15 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len // a partial read has happened so store the target buffer in the snapshot // so it can be re-used when another packet arrives and we read again SetSnapshotStorage(buff); + SetSnapshotDataSize(bytesRead); + } - return result; - } - else - { - if (writeDataSizeToSnapshot) + else if (_snapshot != null) { + // legacy replay path perf optimization SetSnapshotStorage(buff); - if (_snapshot.ContinueEnabled) - { - SetSnapshotDataSize(bytesRead); - stored = true; - } } + return result; } if (_longlenleft == 0) @@ -1973,9 +1973,18 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len result = TryReadPlpLength(false, out _); if (result != TdsOperationStatus.Done) { - if (result == TdsOperationStatus.NeedMoreData && writeDataSizeToSnapshot && !stored) + if (result == TdsOperationStatus.NeedMoreData) { - SetSnapshotDataSize(bytesRead); + if (writeDataSizeToSnapshot) + { + SetSnapshotStorage(buff); + SetSnapshotDataSize(bytesRead); + } + else if (_snapshot != null) + { + // legacy replay path perf optimization + SetSnapshotStorage(buff); + } } return result; } @@ -2888,7 +2897,7 @@ internal int GetSnapshotStorageLength() internal object TryTakeSnapshotStorage() { - Debug.Assert(_snapshot != null && _snapshot.ContinueEnabled, "should not access snapshot accessor functions without first checking that the snapshot is present"); + Debug.Assert(_snapshot != null, "should not access snapshot accessor functions without first checking that the snapshot is present"); object buffer = null; if (_snapshot != null) { @@ -2900,7 +2909,7 @@ internal object TryTakeSnapshotStorage() internal void SetSnapshotStorage(object buffer) { - Debug.Assert(_snapshot != null && _snapshot.ContinueEnabled, "should not access snapshot accessor functions without first checking that the snapshot is available"); + Debug.Assert(_snapshot != null, "should not access snapshot accessor functions without first checking that the snapshot is available"); Debug.Assert(_snapshot._storage == null, "should not overwrite snapshot stored buffer"); if (_snapshot != null) { From 7db3c50fa29c71545e3be355113e256ceffdd8e3 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Tue, 4 Mar 2025 21:33:16 +0000 Subject: [PATCH 6/8] clarify compatability paths through TryReadPlpBytes use as instead of casts for safety --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 4 +- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 4 +- .../Data/SqlClient/SqlCachedBuffer.cs | 2 +- .../Data/SqlClient/TdsParserStateObject.cs | 45 ++++++++++++------- 4 files changed, 33 insertions(+), 22 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 2e9319338d..dc7c579a11 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 @@ -6493,7 +6493,7 @@ private TdsOperationStatus TryReadByteArrayWithContinue(TdsParserStateObject sta { if (isContinuing || isStarting) { - temp = (byte[])stateObj.TryTakeSnapshotStorage(); + temp = stateObj.TryTakeSnapshotStorage() as byte[]; Debug.Assert(bytes == null || bytes.Length == length, "stored buffer length must be null or must have been created with the correct length"); } if (temp != null) @@ -12915,7 +12915,7 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj { if (isContinuing || isStarting) { - temp = (char[])stateObj.TryTakeSnapshotStorage(); + temp = stateObj.TryTakeSnapshotStorage() as char[]; Debug.Assert(temp == null || length == int.MaxValue || temp.Length == length, "stored buffer length must be null or must have been created with the correct length"); } if (temp != null) 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 fc0ef79db0..b2bd88e58a 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 @@ -6970,7 +6970,7 @@ private TdsOperationStatus TryReadByteArrayWithContinue(TdsParserStateObject sta { if (isContinuing || isStarting) { - temp = (byte[])stateObj.TryTakeSnapshotStorage(); + temp = stateObj.TryTakeSnapshotStorage() as byte[]; Debug.Assert(bytes == null || bytes.Length == length, "stored buffer length must be null or must have been created with the correct length"); } if (temp != null) @@ -13458,7 +13458,7 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj { if (isContinuing || isStarting) { - temp = (char[])stateObj.TryTakeSnapshotStorage(); + temp = stateObj.TryTakeSnapshotStorage() as char[]; Debug.Assert(temp == null || length == int.MaxValue || temp.Length == length, "stored buffer length must be null or must have been created with the correct length"); } if (temp != null) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs index ce248797f0..1f1670b97f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs @@ -80,7 +80,7 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser byte[] byteArr = new byte[cb]; // pass false for the writeDataSizeToSnapshot parameter because we want to only take data // from the current packet and not try to do a continue-capable multi packet read - result = stateObj.TryReadPlpBytes(ref byteArr, 0, cb, out cb, writeDataSizeToSnapshot: false); + result = stateObj.TryReadPlpBytes(ref byteArr, 0, cb, out cb, writeDataSizeToSnapshot: false, compatibilityMode: false); if (result != TdsOperationStatus.Done) { if (result == TdsOperationStatus.NeedMoreData && isAvailable && cb == byteArr.Length) 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 e83c77a3b7..e9a3552008 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1706,7 +1706,7 @@ internal TdsOperationStatus TryReadStringWithEncoding(int length, System.Text.En { if (isContinuing || isStarting) { - buf = (byte[])TryTakeSnapshotStorage(); + buf = TryTakeSnapshotStorage() as byte[]; Debug.Assert(buf == null || buf.Length == length, "stored buffer length must be null or must have been created with the correct length"); } if (buf != null) @@ -1850,8 +1850,14 @@ internal int ReadPlpBytesChunk(byte[] buff, int offset, int len) internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len, out int totalBytesRead) { - ( _, bool isStarting, bool isContinuing) = GetSnapshotStatuses(); - return TryReadPlpBytes(ref buff, offset, len, out totalBytesRead, isStarting || isContinuing); + bool isStarting = false; + bool isContinuing = false; + bool compatibilityMode = LocalAppContextSwitches.UseCompatibilityAsyncBehaviour; + if (!compatibilityMode) + { + (_, isStarting, isContinuing) = GetSnapshotStatuses(); + } + return TryReadPlpBytes(ref buff, offset, len, out totalBytesRead, isStarting || isContinuing, compatibilityMode); } // Reads the requested number of bytes from a plp data stream, or the entire data if // requested length is -1 or larger than the actual length of data. First call to this method @@ -1859,10 +1865,10 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len // Returns the actual bytes read. // NOTE: This method must be retriable WITHOUT replaying a snapshot // Every time you call this method increment the offset and decrease len by the value of totalBytesRead - internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len, out int totalBytesRead, bool writeDataSizeToSnapshot) + internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len, out int totalBytesRead, bool writeDataSizeToSnapshot, bool compatibilityMode) { totalBytesRead = 0; - + if (_longlen == 0) { Debug.Assert(_longlenleft == 0); @@ -1888,17 +1894,19 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len { // if there is a snapshot and it contains a stored plp buffer take it // and try to use it if it is the right length - buff = (byte[])TryTakeSnapshotStorage(); + buff = TryTakeSnapshotStorage() as byte[]; if (buff != null) { offset = _snapshot.GetPacketDataOffset(); totalBytesRead = offset; } } - else if (_snapshot != null) + else if (compatibilityMode && _snapshot != null && _snapshotStatus != SnapshotStatus.NotActive) { // legacy replay path perf optimization - buff = (byte[])TryTakeSnapshotStorage(); + // if there is a snapshot and it contains a stored plp buffer take it + // and try to use it if it is the right length + buff = TryTakeSnapshotStorage() as byte[]; } if ((ulong)(buff?.Length ?? 0) != _longlen) @@ -1957,11 +1965,13 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len // so it can be re-used when another packet arrives and we read again SetSnapshotStorage(buff); SetSnapshotDataSize(bytesRead); - + } - else if (_snapshot != null) + else if (compatibilityMode && _snapshot != null) { // legacy replay path perf optimization + // a partial read has happened so store the target buffer in the snapshot + // so it can be re-used when another packet arrives and we read again SetSnapshotStorage(buff); } return result; @@ -1973,18 +1983,19 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len result = TryReadPlpLength(false, out _); if (result != TdsOperationStatus.Done) { - if (result == TdsOperationStatus.NeedMoreData) + if (writeDataSizeToSnapshot) { - if (writeDataSizeToSnapshot) + if (result == TdsOperationStatus.NeedMoreData) { SetSnapshotStorage(buff); SetSnapshotDataSize(bytesRead); } - else if (_snapshot != null) - { - // legacy replay path perf optimization - SetSnapshotStorage(buff); - } + } + else if (compatibilityMode && _snapshot != null) + { + // a partial read has happened so store the target buffer in the snapshot + // so it can be re-used when another packet arrives and we read again + SetSnapshotStorage(buff); } return result; } From a2ad9d2752fd57e530241c10bd1a150f90df9d37 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Wed, 5 Mar 2025 20:14:42 +0000 Subject: [PATCH 7/8] fix ef core test and add coverage --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 6 ++ .../src/Microsoft/Data/SqlClient/TdsParser.cs | 6 ++ .../SQL/DataReaderTest/DataReaderTest.cs | 100 ++++++++++++++++++ 3 files changed, 112 insertions(+) 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 dc7c579a11..8fcf5cfa3d 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 @@ -5556,6 +5556,12 @@ private TdsOperationStatus TryProcessColumnHeaderNoNBC(SqlMetaDataPriv col, TdsP { if (col.metaType.IsLong && !col.metaType.IsPlp) { + if (stateObj.IsSnapshotContinuing()) + { + length = (ulong)stateObj.GetSnapshotStorageLength(); + isNull = length == 0; + return TdsOperationStatus.Done; + } // // we don't care about TextPtrs, simply go after the data after it // 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 b2bd88e58a..e95d11456a 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 @@ -6019,6 +6019,12 @@ private TdsOperationStatus TryProcessColumnHeaderNoNBC(SqlMetaDataPriv col, TdsP { if (col.metaType.IsLong && !col.metaType.IsPlp) { + if (stateObj.IsSnapshotContinuing()) + { + length = (ulong)stateObj.GetSnapshotStorageLength(); + isNull = length == 0; + return TdsOperationStatus.Done; + } // // we don't care about TextPtrs, simply go after the data after it // diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs index d00ea1d226..8a04ba7e66 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs @@ -9,6 +9,7 @@ using System.Reflection; using System.Text; using System.Threading; +using System.Threading.Tasks; using Xunit; namespace Microsoft.Data.SqlClient.ManualTesting.Tests @@ -300,6 +301,105 @@ public static void CheckNullRowVersionIsBDNull() } } + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static int CanReadEmployeesTableCompletely() + { + int counter = 0; + + using (var conn = new SqlConnection(DataTestUtility.TCPConnectionString)) + { + using (var cmd = new SqlCommand("SELECT EmployeeID,LastName,FirstName,Title,TitleOfCourtesy,BirthDate,HireDate,Address,City,Region,PostalCode,Country,HomePhone,Extension,Photo,Notes,ReportsTo,PhotoPath FROM Employees WHERE ReportsTo = @p0 OR (ReportsTo IS NULL AND @p0 IS NULL)", conn)) + { + cmd.Parameters.AddWithValue("@p0", 5); + + conn.Open(); + + using (var reader = cmd.ExecuteReader()) + { + if (reader.Read()) + { + for (int index = 0; index < reader.FieldCount; index++) + { + if (!reader.IsDBNull(index)) + { + object value = reader[index]; + counter += 1; + } + } + } + } + } + } + + return counter; + } + + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static async Task CanReadEmployeesTableCompletelyAsync() + { + int counter = 0; + + using (var conn = new SqlConnection(DataTestUtility.TCPConnectionString)) + { + using (var cmd = new SqlCommand("SELECT EmployeeID,LastName,FirstName,Title,TitleOfCourtesy,BirthDate,HireDate,Address,City,Region,PostalCode,Country,HomePhone,Extension,Photo,Notes,ReportsTo,PhotoPath FROM Employees WHERE ReportsTo = @p0 OR (ReportsTo IS NULL AND @p0 IS NULL)", conn)) + { + cmd.Parameters.AddWithValue("@p0", 5); + + await conn.OpenAsync(); + + using (var reader = await cmd.ExecuteReaderAsync()) + { + if (await reader.ReadAsync()) + { + for (int index = 0; index < reader.FieldCount; index++) + { + if (!await reader.IsDBNullAsync(index)) + { + object value = reader[index]; + counter += 1; + } + } + } + } + } + } + + return counter; + } + + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static async Task CanReadEmployeesTableCompletelyWithNullImageAsync() + { + int counter = 0; + + using (var conn = new SqlConnection(DataTestUtility.TCPConnectionString)) + { + using (var cmd = new SqlCommand("SELECT EmployeeID,LastName,FirstName,Title,TitleOfCourtesy,BirthDate,HireDate,Address,City,Region,PostalCode,Country,HomePhone,Extension,convert(image,NULL) as Photo,Notes,ReportsTo,PhotoPath FROM Employees WHERE ReportsTo = @p0 OR (ReportsTo IS NULL AND @p0 IS NULL)", conn)) + { + cmd.Parameters.AddWithValue("@p0", 5); + + await conn.OpenAsync(); + + using (var reader = await cmd.ExecuteReaderAsync()) + { + if (await reader.ReadAsync()) + { + for (int index = 0; index < reader.FieldCount; index++) + { + if (!await reader.IsDBNullAsync(index)) + { + object value = reader[index]; + counter += 1; + } + } + } + } + } + } + + return counter; + } + // Synapse: Cannot find data type 'rowversion'. [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))] public static void CheckLegacyNullRowVersionIsEmptyArray() From 671580c3a91e6b0805a9f15c9df307ea5963abaf Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Thu, 13 Mar 2025 20:54:18 +0000 Subject: [PATCH 8/8] address review feedback --- .../Microsoft/Data/SqlClient/SqlDataReader.cs | 2 +- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 77 +++++++++---------- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 76 +++++++++--------- 3 files changed, 75 insertions(+), 80 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs index ac09899757..0932bccb51 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs @@ -3766,7 +3766,7 @@ private TdsOperationStatus TryNextResult(out bool more) if (result != TdsOperationStatus.Done) { more = false; - return TdsOperationStatus.Done; + return result; } // In the case of not closing the reader, null out the metadata AFTER 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 8fcf5cfa3d..43ad123d4a 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 @@ -13070,56 +13070,53 @@ bool writeDataSizeToSnapshot while (charsLeft > 0) { - if (partialReadInProgress) + if (!partialReadInProgress) { - goto resumePartialRead; - } - charsRead = (int)Math.Min((stateObj._longlenleft + 1) >> 1, (ulong)charsLeft); - if ((buff == null) || (buff.Length < (offst + charsRead))) - { - char[] newbuf; - bool returnRentedBufferAfterCopy = rentedBuff; - if (supportRentedBuff && (offst + charsRead) < 1073741824) // 1 Gib - { - newbuf = ArrayPool.Shared.Rent(offst + charsRead); - rentedBuff = true; - } - else + charsRead = (int)Math.Min((stateObj._longlenleft + 1) >> 1, (ulong)charsLeft); + if ((buff == null) || (buff.Length < (offst + charsRead))) { - newbuf = new char[offst + charsRead]; - rentedBuff = false; - } + char[] newbuf; + bool returnRentedBufferAfterCopy = rentedBuff; + if (supportRentedBuff && (offst + charsRead) < 1073741824) // 1 Gib + { + newbuf = ArrayPool.Shared.Rent(offst + charsRead); + rentedBuff = true; + } + else + { + newbuf = new char[offst + charsRead]; + rentedBuff = false; + } - if (buff != null) - { - Buffer.BlockCopy(buff, 0, newbuf, 0, offst * 2); - if (returnRentedBufferAfterCopy) + if (buff != null) { - buff.AsSpan(0, offst).Clear(); - ArrayPool.Shared.Return(buff, clearArray: false); + Buffer.BlockCopy(buff, 0, newbuf, 0, offst * 2); + if (returnRentedBufferAfterCopy) + { + buff.AsSpan(0, offst).Clear(); + ArrayPool.Shared.Return(buff, clearArray: false); + } } + buff = newbuf; + newbuf = null; } - buff = newbuf; - newbuf = null; - } - if (charsRead > 0) - { - result = TryReadPlpUnicodeCharsChunk(buff, offst, charsRead, stateObj, out charsRead); - if (result != TdsOperationStatus.Done) + if (charsRead > 0) { - return result; - } - charsLeft -= charsRead; - offst += charsRead; - totalCharsRead += charsRead; + result = TryReadPlpUnicodeCharsChunk(buff, offst, charsRead, stateObj, out charsRead); + if (result != TdsOperationStatus.Done) + { + return result; + } + charsLeft -= charsRead; + offst += charsRead; + totalCharsRead += charsRead; - if (writeDataSizeToSnapshot) - { - currentPacketId = IncrementSnapshotDataSize(stateObj, restartingDataSizeCount, currentPacketId, charsRead * 2); + if (writeDataSizeToSnapshot) + { + currentPacketId = IncrementSnapshotDataSize(stateObj, restartingDataSizeCount, currentPacketId, charsRead * 2); + } } } - - resumePartialRead: // Special case single byte if ( (stateObj._longlenleft == 1 || partialReadInProgress) 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 8c4b38532e..6dc02c7505 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 @@ -13613,56 +13613,54 @@ bool writeDataSizeToSnapshot while (charsLeft > 0) { - if (partialReadInProgress) + if (!partialReadInProgress) { - goto resumePartialRead; - } - charsRead = (int)Math.Min((stateObj._longlenleft + 1) >> 1, (ulong)charsLeft); - if ((buff == null) || (buff.Length < (offst + charsRead))) - { - char[] newbuf; - bool returnRentedBufferAfterCopy = rentedBuff; - if (supportRentedBuff && (offst + charsRead) < 1073741824) // 1 Gib + charsRead = (int)Math.Min((stateObj._longlenleft + 1) >> 1, (ulong)charsLeft); + if ((buff == null) || (buff.Length < (offst + charsRead))) { - newbuf = ArrayPool.Shared.Rent(offst + charsRead); - rentedBuff = true; - } - else - { - newbuf = new char[offst + charsRead]; - rentedBuff = false; - } + char[] newbuf; + bool returnRentedBufferAfterCopy = rentedBuff; + if (supportRentedBuff && (offst + charsRead) < 1073741824) // 1 Gib + { + newbuf = ArrayPool.Shared.Rent(offst + charsRead); + rentedBuff = true; + } + else + { + newbuf = new char[offst + charsRead]; + rentedBuff = false; + } - if (buff != null) - { - Buffer.BlockCopy(buff, 0, newbuf, 0, offst * 2); - if (returnRentedBufferAfterCopy) + if (buff != null) { - buff.AsSpan(0, offst).Clear(); - ArrayPool.Shared.Return(buff, clearArray: false); + Buffer.BlockCopy(buff, 0, newbuf, 0, offst * 2); + if (returnRentedBufferAfterCopy) + { + buff.AsSpan(0, offst).Clear(); + ArrayPool.Shared.Return(buff, clearArray: false); + } } + buff = newbuf; + newbuf = null; } - buff = newbuf; - newbuf = null; - } - if (charsRead > 0) - { - result = TryReadPlpUnicodeCharsChunk(buff, offst, charsRead, stateObj, out charsRead); - if (result != TdsOperationStatus.Done) + if (charsRead > 0) { - return result; - } - charsLeft -= charsRead; - offst += charsRead; - totalCharsRead += charsRead; + result = TryReadPlpUnicodeCharsChunk(buff, offst, charsRead, stateObj, out charsRead); + if (result != TdsOperationStatus.Done) + { + return result; + } + charsLeft -= charsRead; + offst += charsRead; + totalCharsRead += charsRead; - if (writeDataSizeToSnapshot) - { - currentPacketId = IncrementSnapshotDataSize(stateObj, restartingDataSizeCount, currentPacketId, charsRead * 2); + if (writeDataSizeToSnapshot) + { + currentPacketId = IncrementSnapshotDataSize(stateObj, restartingDataSizeCount, currentPacketId, charsRead * 2); + } } } - resumePartialRead: // Special case single byte if ( (stateObj._longlenleft == 1 || partialReadInProgress )