From 692fc524c28737796e85af6c0a9f38d629860de2 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Tue, 9 Sep 2025 11:55:20 +0100 Subject: [PATCH 1/2] add continue request usage --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 23 +----- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 22 +----- .../Data/SqlClient/SqlCachedBuffer.cs | 7 +- .../Data/SqlClient/TdsParserStateObject.cs | 78 ++++++++++++------- 4 files changed, 66 insertions(+), 64 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 8fbeb3050c..d7c9b3175b 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 @@ -6377,25 +6377,10 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value, case TdsEnums.SQLVARBINARY: case TdsEnums.SQLIMAGE: byte[] b = null; - - // If varbinary(max), we only read the first chunk here, expecting the caller to read the rest - if (isPlp) - { - // If we are given -1 for length, then we read the entire value, - // otherwise only the requested amount, usually first chunk. - result = stateObj.TryReadPlpBytes(ref b, 0, length, out _); - if (result != TdsOperationStatus.Done) - { - return result; - } - } - else + result = stateObj.TryReadByteArrayWithContinue(length, isPlp, out b); + if (result != TdsOperationStatus.Done) { - result = stateObj.TryReadByteArrayWithContinue(length, out b); - if (result != TdsOperationStatus.Done) - { - return result; - } + return result; } if (md.isEncrypted @@ -6790,7 +6775,7 @@ internal TdsOperationStatus TryReadSqlValueInternal(SqlBuffer value, byte tdsTyp // Note: Better not come here with plp data!! Debug.Assert(length <= TdsEnums.MAXSIZE); byte[] b = new byte[length]; - result = stateObj.TryReadByteArray(b, length); + result = stateObj.TryReadByteArrayWithContinue(length, isPlp: false, out b); if (result != TdsOperationStatus.Done) { return result; 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 d38190a359..83109f30c8 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 @@ -6541,24 +6541,10 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value, case TdsEnums.SQLIMAGE: byte[] b = null; - // If varbinary(max), we only read the first chunk here, expecting the caller to read the rest - if (isPlp) - { - // If we are given -1 for length, then we read the entire value, - // otherwise only the requested amount, usually first chunk. - result = stateObj.TryReadPlpBytes(ref b, 0, length, out _); - if (result != TdsOperationStatus.Done) - { - return result; - } - } - else + result = stateObj.TryReadByteArrayWithContinue(length, isPlp, out b); + if (result != TdsOperationStatus.Done) { - result = stateObj.TryReadByteArrayWithContinue(length, out b); - if (result != TdsOperationStatus.Done) - { - return result; - } + return result; } if (md.isEncrypted @@ -6625,7 +6611,7 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value, case TdsEnums.SQLVECTOR: // Vector data is read as non-plp binary value. // This is same as reading varbinary(8000). - result = stateObj.TryReadByteArrayWithContinue(length, out b); + result = stateObj.TryReadByteArrayWithContinue(length, isPlp: false, out b); if (result != TdsOperationStatus.Done) { return result; 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 a03eefce09..f0f7edf099 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs @@ -39,6 +39,7 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser { buffer = null; + stateObj.RequestContinue(true); (bool canContinue, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses(); List cachedBytes = null; @@ -81,13 +82,17 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser result = stateObj.TryReadPlpBytes(ref byteArr, 0, cb, out cb, canContinue, writeDataSizeToSnapshot: false, compatibilityMode: false); if (result != TdsOperationStatus.Done) { - if (result == TdsOperationStatus.NeedMoreData && canContinue && cb == byteArr.Length && (isContinuing || !isStarting)) + if (result == TdsOperationStatus.NeedMoreData && canContinue && cb == byteArr.Length) { // succeeded in getting the data but failed to find the next plp length returnAfterAdd = true; } else { + if (canContinue) + { + stateObj.SetSnapshotStorage(cachedBytes); + } return result; } } 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 b72e38631f..4a732ccf4a 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1560,45 +1560,71 @@ public TdsOperationStatus TryReadByteArray(Span buff, int len, out int tot return TdsOperationStatus.Done; } - public TdsOperationStatus TryReadByteArrayWithContinue(int length, out byte[] bytes) + internal TdsOperationStatus TryReadByteArrayWithContinue(int length, out byte[] value) { - bytes = null; - int offset = 0; - byte[] temp = null; + return TryReadByteArrayWithContinue(length, false, out value); + } + internal TdsOperationStatus TryReadByteArrayWithContinue(int length, bool isPlp, out byte[] value) + { + Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); + + byte[] buf = null; + RequestContinue(true); (bool canContinue, bool isStarting, bool isContinuing) = GetSnapshotStatuses(); - if (canContinue) + + if (isPlp) { - temp = TryTakeSnapshotStorage() as byte[]; - Debug.Assert(temp != null || !isContinuing, "if continuing stored buffer must be present to contain previous data to continue from"); - 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) + bool compatibilityMode = LocalAppContextSwitches.UseCompatibilityAsyncBehaviour; + TdsOperationStatus result = TryReadPlpBytes(ref buf, 0, int.MaxValue, out length, canContinue && !compatibilityMode, canContinue && !compatibilityMode, compatibilityMode); + if (result != TdsOperationStatus.Done) { - offset = GetSnapshotTotalSize(); + value = null; + return result; } + + AssertValidState(); } + else + { + int startOffset = 0; + if (canContinue) + { + buf = TryTakeSnapshotStorage() as byte[]; + Debug.Assert(buf != null || !isContinuing, "if continuing stored buffer must be present to contain previous data to continue from"); + 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(); + } + } - if (temp == null) - { - temp = new byte[length]; - } + if (buf == null || buf.Length < length) + { + buf = new byte[length]; + } - TdsOperationStatus result = TryReadByteArray(temp, length, out _, offset, isStarting || isContinuing); + TdsOperationStatus result = TryReadByteArray(buf, length, out _, startOffset, canContinue); - if (result == TdsOperationStatus.Done) - { - bytes = temp; - } - else if (result == TdsOperationStatus.NeedMoreData) - { - if (canContinue) + if (result != TdsOperationStatus.Done) { - SetSnapshotStorage(temp); + if (result == TdsOperationStatus.NeedMoreData) + { + if (canContinue) + { + SetSnapshotStorage(buf); + } + } + value = null; + return result; } + + AssertValidState(); + } - return result; + value = buf; + return TdsOperationStatus.Done; } // Takes no arguments and returns a byte from the buffer. If the buffer is empty, it is filled @@ -2841,7 +2867,7 @@ internal TdsOperationStatus TryReadNetworkPacket() } // previous buffer is in snapshot - _inBuff = new byte[_inBuff.Length]; + NewBuffer(_inBuff.Length); result = TdsOperationStatus.NeedMoreData; } From df52b34d9fbbf773ee511117e763b9dbdfef5544 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Fri, 12 Sep 2025 22:02:26 +0100 Subject: [PATCH 2/2] unify TryReadByteArrayWithContinue overloads --- .../netcore/src/Microsoft/Data/SqlClient/TdsParser.cs | 2 +- .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 8 ++------ 2 files changed, 3 insertions(+), 7 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 d7c9b3175b..54a9d584af 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 @@ -6447,7 +6447,7 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value, case TdsEnums.SQLVECTOR: // Vector data is read as non-plp binary value. // This is same as reading varbinary(8000). - result = stateObj.TryReadByteArrayWithContinue(length, out b); + result = stateObj.TryReadByteArrayWithContinue(length, isPlp: false, out b); if (result != TdsOperationStatus.Done) { return result; 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 4a732ccf4a..fc7371d317 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1560,17 +1560,13 @@ public TdsOperationStatus TryReadByteArray(Span buff, int len, out int tot return TdsOperationStatus.Done; } - internal TdsOperationStatus TryReadByteArrayWithContinue(int length, out byte[] value) - { - return TryReadByteArrayWithContinue(length, false, out value); - } internal TdsOperationStatus TryReadByteArrayWithContinue(int length, bool isPlp, out byte[] value) { Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); byte[] buf = null; RequestContinue(true); - (bool canContinue, bool isStarting, bool isContinuing) = GetSnapshotStatuses(); + (bool canContinue, bool _, bool isContinuing) = GetSnapshotStatuses(); if (isPlp) { @@ -1973,7 +1969,7 @@ internal TdsOperationStatus TryReadString(int length, out string value) if (((_inBytesUsed + cBytes) > _inBytesRead) || (_inBytesPacket < cBytes)) { - TdsOperationStatus result = TryReadByteArrayWithContinue(cBytes, out buf); + TdsOperationStatus result = TryReadByteArrayWithContinue(cBytes, isPlp: false, out buf); if (result != TdsOperationStatus.Done) { value = null;