- Notifications
You must be signed in to change notification settings - Fork311
Add partial packet detection and fixup#2714
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Uh oh!
There was an error while loading.Please reload this page.
Merged
Changes from1 commit
Commits
Show all changes
30 commits Select commitHold shift + click to select a range
94b6808 reimplementation of experimental branch on main
Wraith253c1139 integrated multiplexer tests and align multiplexer with dev version
Wraith23622f9c address feedback, minor fixes and tuning
Wraith2d0fd956 add Packet comments
Wraith225f0c62 Fix async cancellation and add test coverage for the scenario.
Wraith2cc694b9 reduce CancelAsyncConnections sensitivity to match main
Wraith2fb9be3b make multiplexer not require snapshot to consume partial packets
Wraith27f262eb refine AppendPacketData checks and fix behaviour that was causing it …
Wraith267b7762 add more debugging
Wraith257c8b86 add debug fail to sanity check in multiplexer to make it clear that i…
Wraith2b00a379 update multiplexer to deal with multiple sequential packets less than…
Wraith2352ac25 change multiplexer to deal with trailing partial packets correctly wh…
Wraith251137f9 fix rebase conflicts
Wraith217ad988 review feedback and misc fix
Wraith2eae759a protect missed locations where network reads can happen
Wraith229ad90b fix CheckPacket assertion
Wraith25d3f8a6 add try catch around pending read to align with others
Wraith236cdd88 Merge branch 'main' of https://github.com/dotnet/SqlClient into opera…
David-Engel433ccf7 Merge branch 'main' of https://github.com/dotnet/SqlClient into opera…
David-Engelc65da55 Apply suggestions from code review
David-Engelb4460ec Fix merge error
David-Engel80cbb6e add DumpBuffer function description and format code
Wraith2bb4fa9e review feedback 2
Wraith279f9fbc move Packet disposal to debug only
Wraith2d31e908 review feedback 3
Wraith2429b7ac review feedback 4
Wraith211dd84e add app context switched behaviour to allow reverting to previous pro…
Wraith27684861 revirew feedback 5
Wraith2cd1e025 fix tests build
Wraith23ed9c83 fix netfx builds
Wraith2File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
add app context switched behaviour to allow reverting to previous pro…
…cess sni.
- Loading branch information
Uh oh!
There was an error while loading.Please reload this page.
commit11dd84e6b529a69576ffdd3451d90fb1615e9409
There are no files selected for viewing
20 changes: 20 additions & 0 deletionssrc/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
67 changes: 67 additions & 0 deletions...Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.Multiplexer.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -17,6 +17,12 @@ partial class TdsParserStateObject | ||
| public void ProcessSniPacket(PacketHandle packet, uint error) | ||
| { | ||
| if (LocalAppContextSwitches.UseCompatibilityProcessSni) | ||
| { | ||
| ProcessSniPacketCompat(packet, error); | ||
| return; | ||
| } | ||
| if (error != 0) | ||
| { | ||
| if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Consider There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. This falls into the category of maintaining consistent style. The change wouldn't improve functionality and would leave us with files in the solution using different syntax to do the same thing which can cause you not to be able to find them or have to mentally determine that both syntaxes mean the same thing. If we're going to make this change I'd rather do it in a dedicated style update PR. I don't object to the new syntax I just think we should be consistent. | ||
| @@ -488,5 +494,66 @@ private static bool SetupRemainderPacket(Packet packet) | ||
| return containsFullPacket; | ||
| } | ||
| public void ProcessSniPacketCompat(PacketHandle packet, uint error) | ||
| { | ||
| 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 = | ||
| #if NETFRAMEWORK | ||
| SniNativeWrapper. | ||
| #endif | ||
| 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 (_snapshotStatus != SnapshotStatus.NotActive) | ||
| { | ||
| _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, _inBytesRead); | ||
| AssertValidState(); | ||
| } | ||
| else | ||
| { | ||
| throw SQL.ParsingError(ParsingErrorState.ProcessSniPacketFailed); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
1 change: 1 addition & 0 deletionssrc/Microsoft.Data.SqlClient/tests/FunctionalTests/LocalAppContextSwitchesTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
40 changes: 27 additions & 13 deletionssrc/Microsoft.Data.SqlClient/tests/FunctionalTests/MultiplexerTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.