Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add partial packet detection and fixup #2714

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add partial packet detection and fixup #2714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a42c7fa
reimplementation of experimental branch on main
Wraith2 Jul 24, 2024
f9e5786
integrated multiplexer tests and align multiplexer with dev version
Wraith2 Jun 25, 2024
89cfbb6
address feedback, minor fixes and tuning
Wraith2 Jul 24, 2024
935a70d
add Packet comments
Wraith2 Jul 25, 2024
8b57818
Fix async cancellation and add test coverage for the scenario.
Wraith2 Aug 9, 2024
7fa46d0
reduce CancelAsyncConnections sensitivity to match main
Wraith2 Aug 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\OnChangedEventHandler.cs">
<Link>Microsoft\Data\SqlClient\OnChangedEventHandler.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\Packet.cs">
<Link>Microsoft\Data\SqlClient\Packet.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ParameterPeekAheadValue.cs">
<Link>Microsoft\Data\SqlClient\ParameterPeekAheadValue.cs</Link>
</Compile>
Expand Down Expand Up @@ -591,6 +594,9 @@
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\TdsParserStateObject.cs">
<Link>Microsoft\Data\SqlClient\TdsParserStateObject.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\TdsParserStateObject.Multiplexer.cs">
<Link>Microsoft\Data\SqlClient\TdsParserStateObject.Multiplexer.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\TdsParserStaticMethods.cs">
<Link>Microsoft\Data\SqlClient\TdsParserStaticMethods.cs</Link>
</Compile>
Expand Down Expand Up @@ -958,7 +964,7 @@
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Data.SqlClient.SNI.runtime" Version="$(MicrosoftDataSqlClientSNIRuntimeVersion)" />
<PackageReference Include="Microsoft.Extensions.Caching.Memory" Version="$(MicrosoftExtensionsCachingMemoryVersion)" />
<PackageReference Include="Microsoft.Extensions.Caching.Memory" Version="$(MicrosoftExtensionsCachingMemoryVersion)" />
<!-- Enable the project reference for debugging purposes. -->
<!-- <ProjectReference Include="$(SqlServerSourceCode)\Microsoft.SqlServer.Server.csproj" /> -->
<PackageReference Include="Microsoft.SqlServer.Server" Version="$(MicrosoftSqlServerServerVersion)" />
Expand Down
9 changes: 5 additions & 4 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3508,7 +3508,7 @@ private TdsOperationStatus TryNextResult(out bool more)

/// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlDataReader.xml' path='docs/members[@name="SqlDataReader"]/Read/*' />
// user must call Read() to position on the first row
override public bool Read()
public override bool Read()
{
if (_currentTask != null)
{
Expand Down Expand Up @@ -4296,9 +4296,10 @@ internal TdsOperationStatus TrySetMetaData(_SqlMetaDataSet metaData, bool moreIn
_metaDataConsumed = true;

if (_parser != null)
{ // There is a valid case where parser is null
// Peek, and if row token present, set _hasRows true since there is a
// row in the result
{
// There is a valid case where parser is null
// Peek, and if row token present, set _hasRows true since there is a
// row in the result
byte b;
result = _stateObj.TryPeekByte(out b);
if (result != TdsOperationStatus.Done)
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4147,6 +4147,7 @@ internal TdsOperationStatus TryProcessReturnValue(int length, TdsParserStateObje
{
return result;
}

byte len;
result = stateObj.TryReadByte(out len);
if (result != TdsOperationStatus.Done)
Expand Down Expand Up @@ -4546,7 +4547,6 @@ internal TdsOperationStatus TryProcessCollation(TdsParserStateObject stateObj, o
collation = null;
return result;
}

if (SqlCollation.Equals(_cachedCollation, info, sortId))
{
collation = _cachedCollation;
Expand Down Expand Up @@ -5272,7 +5272,7 @@ private TdsOperationStatus TryCommonProcessMetaData(TdsParserStateObject stateOb
{
// If the column is encrypted, we should have a valid cipherTable
if (cipherTable != null)
{
{
result = TryProcessTceCryptoMetadata(stateObj, col, cipherTable, columnEncryptionSetting, isReturnValue: false);
if (result != TdsOperationStatus.Done)
{
Expand Down
57 changes: 3 additions & 54 deletions ...osoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,60 +393,9 @@ private void ReadSniError(TdsParserStateObject stateObj, uint error)
AssertValidState();
}

public void ProcessSniPacket(PacketHandle packet, uint error)
private uint GetSniPacket(PacketHandle packet, ref uint dataSize)
{
if (error != 0)
{
if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken))
{
// Do nothing with callback if closed or broken and error not 0 - callback can occur
// after connection has been closed. PROBLEM IN NETLIB - DESIGN FLAW.
return;
}

AddError(_parser.ProcessSNIError(this));
AssertValidState();
}
else
{
uint dataSize = 0;

uint getDataError = SNIPacketGetData(packet, _inBuff, ref dataSize);

if (getDataError == TdsEnums.SNI_SUCCESS)
{
if (_inBuff.Length < dataSize)
{
Debug.Assert(true, "Unexpected dataSize on Read");
throw SQL.InvalidInternalPacketSize(StringsHelper.GetString(Strings.SqlMisc_InvalidArraySizeMessage));
}

_lastSuccessfulIOTimer._value = DateTime.UtcNow.Ticks;
_inBytesRead = (int)dataSize;
_inBytesUsed = 0;

if (_snapshot != null)
{
_snapshot.AppendPacketData(_inBuff, _inBytesRead);
if (_snapshotReplay)
{
_snapshot.MoveNext();
#if DEBUG
_snapshot.AssertCurrent();
#endif
}
}

SniReadStatisticsAndTracing();
SqlClientEventSource.Log.TryAdvancedTraceBinEvent("TdsParser.ReadNetworkPacketAsyncCallback | INFO | ADV | State Object Id {0}, Packet read. In Buffer {1}, In Bytes Read: {2}", ObjectID, _inBuff, (ushort)_inBytesRead);

AssertValidState();
}
else
{
throw SQL.ParsingError(ParsingErrorState.ProcessSniPacketFailed);
}
}
return SNIPacketGetData(packet, _inBuff, ref dataSize);
}

private void ChangeNetworkPacketTimeout(int dueTime, int period)
Expand Down Expand Up @@ -1634,7 +1583,7 @@ internal void AssertStateIsClean()
if ((parser != null) && (parser.State != TdsParserState.Closed) && (parser.State != TdsParserState.Broken))
{
// Async reads
Debug.Assert(_snapshot == null && !_snapshotReplay, "StateObj has leftover snapshot state");
Debug.Assert(_snapshot == null && _snapshotStatus == SnapshotStatus.NotActive, "StateObj has leftover snapshot state");
Debug.Assert(!_asyncReadWithoutSnapshot, "StateObj has AsyncReadWithoutSnapshot still enabled");
Debug.Assert(_executionContext == null, "StateObj has a stored execution context from an async read");
// Async writes
Expand Down
8 changes: 7 additions & 1 deletion src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,9 @@
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\OnChangedEventHandler.cs">
<Link>Microsoft\Data\SqlClient\OnChangedEventHandler.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\Packet.cs">
<Link>Microsoft\Data\SqlClient\Packet.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ParameterPeekAheadValue.cs">
<Link>Microsoft\Data\SqlClient\ParameterPeekAheadValue.cs</Link>
</Compile>
Expand Down Expand Up @@ -612,6 +615,9 @@
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\TdsParserStateObject.cs">
<Link>Microsoft\Data\SqlClient\TdsParserStateObject.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\TdsParserStateObject.Multiplexer.cs">
<Link>Microsoft\Data\SqlClient\TdsParserStateObject.Multiplexer.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\TdsParserStaticMethods.cs">
<Link>Microsoft\Data\SqlClient\TdsParserStaticMethods.cs</Link>
</Compile>
Expand Down Expand Up @@ -756,7 +762,7 @@
</COMReference>
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Caching.Memory" Version="$(MicrosoftExtensionsCachingMemoryVersion)" />
<PackageReference Include="Microsoft.Extensions.Caching.Memory" Version="$(MicrosoftExtensionsCachingMemoryVersion)" />
<PackageReference Include="System.Text.Encodings.Web">
<Version>$(SystemTextEncodingsWebVersion)</Version>
</PackageReference>
Expand Down
5 changes: 3 additions & 2 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2851,7 +2851,6 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle
result = stateObj.TryPeekByte(out peekedToken);
if (result != TdsOperationStatus.Done)
{
// temporarily cache next byte
return result;
}

Expand Down Expand Up @@ -4704,12 +4703,14 @@ internal TdsOperationStatus TryProcessReturnValue(int length,
return result;
}
}
byte len; // Length of parameter name

byte len;
result = stateObj.TryReadByte(out len);
if (result != TdsOperationStatus.Done)
{
return result;
}

rec.parameter = null;
if (len > 0)
{
Expand Down
57 changes: 3 additions & 54 deletions ...Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,60 +524,9 @@ private void ReadSniError(TdsParserStateObject stateObj, uint error)
AssertValidState();
}

public void ProcessSniPacket(PacketHandle packet, uint error)
private uint GetSniPacket(PacketHandle packet, ref uint dataSize)
{
if (error != 0)
{
if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken))
{
// Do nothing with callback if closed or broken and error not 0 - callback can occur
// after connection has been closed. PROBLEM IN NETLIB - DESIGN FLAW.
return;
}

AddError(_parser.ProcessSNIError(this));
AssertValidState();
}
else
{
uint dataSize = 0;

uint getDataError = SNINativeMethodWrapper.SNIPacketGetData(packet, _inBuff, ref dataSize);

if (getDataError == TdsEnums.SNI_SUCCESS)
{
if (_inBuff.Length < dataSize)
{
Debug.Assert(true, "Unexpected dataSize on Read");
throw SQL.InvalidInternalPacketSize(StringsHelper.GetString(Strings.SqlMisc_InvalidArraySizeMessage));
}

_lastSuccessfulIOTimer._value = DateTime.UtcNow.Ticks;
_inBytesRead = (int)dataSize;
_inBytesUsed = 0;

if (_snapshot != null)
{
_snapshot.AppendPacketData(_inBuff, _inBytesRead);
if (_snapshotReplay)
{
_snapshot.MoveNext();
#if DEBUG
_snapshot.AssertCurrent();
#endif
}
}

SniReadStatisticsAndTracing();
SqlClientEventSource.Log.TryAdvancedTraceBinEvent("TdsParser.ReadNetworkPacketAsyncCallback | INFO | ADV | State Object Id {0}, Packet read. In Buffer {1}, In Bytes Read: {2}", ObjectID, _inBuff, (ushort)_inBytesRead);

AssertValidState();
}
else
{
throw SQL.ParsingError(ParsingErrorState.ProcessSniPacketFailed);
}
}
return SNINativeMethodWrapper.SNIPacketGetData(packet, _inBuff, ref dataSize);
}

private void ChangeNetworkPacketTimeout(int dueTime, int period)
Expand Down Expand Up @@ -1774,7 +1723,7 @@ internal void AssertStateIsClean()
if ((parser != null) && (parser.State != TdsParserState.Closed) && (parser.State != TdsParserState.Broken))
{
// Async reads
Debug.Assert(_snapshot == null && !_snapshotReplay, "StateObj has leftover snapshot state");
Debug.Assert(_snapshot == null && _snapshotStatus == SnapshotStatus.NotActive, "StateObj has leftover snapshot state");
Debug.Assert(!_asyncReadWithoutSnapshot, "StateObj has AsyncReadWithoutSnapshot still enabled");
Debug.Assert(_executionContext == null, "StateObj has a stored execution context from an async read");
// Async writes
Expand Down
Loading
Loading