Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upFix race condition in Ping tests #411
Merged
Conversation
In StartPing, when the QUIC_SEND_FLAG_FIN flag is set and there are no bytes left, if the SteamSend completes successfully, we are racing against stream callback, which deletes the instance we are currently executing. When the loop continues, BytesToSend is read again, which is the use after free. Explicitly exiting the loop upon no data left removes the race Closes #395
|
/azp run |
|
No pipelines are associated with this pull request. |
|
LGTM |
nibanks
merged commit 98614d6
into
microsoft:master
1 of 2 checks passed
1 of 2 checks passed
license/cla
All CLA requirements met.
chgray
added a commit
that referenced
this pull request
Jun 17, 2020
* Rebuild sidecar for broken event * update to experimental clog with some bug fixes (#375) * Support NoProcDump Mode for Windows Test Script (#379) * Update to Latest Googletest Master Branch (#374) * Change QUIC_API to __cdecl on Windows User Mode (#377) * Finish NoProcDump Support for Azure Pipelines (#380) * Refactor Stub QuicCertCreate (#378) * Use Full.Light Log Profile in INT Tests (#381) * Refactor Test Code (#382) * Update AZP build datestamp to use ISO 8601-like date format (#383) * Use Git Attributes File to Normal Line Endings (#338) * Fixes after merge master * Normalize Line Endings (#384) * Fix kernel build * Fix binary data in png files. (#385) * Interop Server Instructions (#386) Co-authored-by: Anthony Rossi <41394064+anrossi@users.noreply.github.com> * Create Explicit Error Codes for Interop Server Request Failures (#390) * Fix No Idle Timeout TP Bug (#389) * Fix Bug in fread Call in Interop Server (#391) * Add Random Loss to SpinQuic (#392) * Enable Address Sanitizer for Linux (#302) * Get SpinQuic Running Clean (#393) * Disable Logs in Azure Pipelines Runs (#396) * Update sidecar/manifest for new event * Remove Merge Sections from Kernel Mode (#398) * Fix crypto layer issues found via SpinQuic (#399) * Don't Accept Retry after ACK (#400) * Add SpinQuic support for DatagramSend, and other settings parameters (#401) * More SpinQuic Features and Associated Fixes (#403) * Small Refactor of Build Files (#407) * Adds Several Performance Related Fixes (#405) * Add Build Support for PGO (#406) * Add Initial PGO File (#409) * Fix race condition in Ping tests (#411) In StartPing, when the QUIC_SEND_FLAG_FIN flag is set and there are no bytes left, if the SteamSend completes successfully, we are racing against stream callback, which deletes the instance we are currently executing. When the loop continues, BytesToSend is read again, which is the use after free. Explicitly exiting the loop upon no data left removes the race Closes #395 * Improve Output when CDB.exe is Missing (#412) * Fix leak in QuicStreamSendFlush (#415) If send isn't enabled, the current send request was completed, but any existing requests left in the list were leaked. Closes #408 * Don't Use App Closed until 1-RTT Keys (#417) * Only Change Partitions for New Paths (#418) * Add cdb to path in run-executable azure script (#416) * Allow a Single Partition Update per Path (#420) * Fix Clog download * Fix copy code * fix up log script to print cpu info and timestamps * Revert "fix up log script to print cpu info and timestamps" This reverts commit b37879f. Co-authored-by: Nick Banks <nibanks@microsoft.com> Co-authored-by: Anthony Rossi <41394064+anrossi@users.noreply.github.com> Co-authored-by: Max <628527+Maximus-@users.noreply.github.com> Co-authored-by: Thad House <ThadHouse@users.noreply.github.com> Co-authored-by: Chris Gray <chgray@microsoft.com>
chgray
added a commit
that referenced
this pull request
Jun 23, 2020
* update to experimental clog with some bug fixes * Feature/clog (#511) * Rebuild sidecar for broken event * update to experimental clog with some bug fixes (#375) * Support NoProcDump Mode for Windows Test Script (#379) * Update to Latest Googletest Master Branch (#374) * Change QUIC_API to __cdecl on Windows User Mode (#377) * Finish NoProcDump Support for Azure Pipelines (#380) * Refactor Stub QuicCertCreate (#378) * Use Full.Light Log Profile in INT Tests (#381) * Refactor Test Code (#382) * Update AZP build datestamp to use ISO 8601-like date format (#383) * Use Git Attributes File to Normal Line Endings (#338) * Fixes after merge master * Normalize Line Endings (#384) * Fix kernel build * Fix binary data in png files. (#385) * Interop Server Instructions (#386) Co-authored-by: Anthony Rossi <41394064+anrossi@users.noreply.github.com> * Create Explicit Error Codes for Interop Server Request Failures (#390) * Fix No Idle Timeout TP Bug (#389) * Fix Bug in fread Call in Interop Server (#391) * Add Random Loss to SpinQuic (#392) * Enable Address Sanitizer for Linux (#302) * Get SpinQuic Running Clean (#393) * Disable Logs in Azure Pipelines Runs (#396) * Update sidecar/manifest for new event * Remove Merge Sections from Kernel Mode (#398) * Fix crypto layer issues found via SpinQuic (#399) * Don't Accept Retry after ACK (#400) * Add SpinQuic support for DatagramSend, and other settings parameters (#401) * More SpinQuic Features and Associated Fixes (#403) * Small Refactor of Build Files (#407) * Adds Several Performance Related Fixes (#405) * Add Build Support for PGO (#406) * Add Initial PGO File (#409) * Fix race condition in Ping tests (#411) In StartPing, when the QUIC_SEND_FLAG_FIN flag is set and there are no bytes left, if the SteamSend completes successfully, we are racing against stream callback, which deletes the instance we are currently executing. When the loop continues, BytesToSend is read again, which is the use after free. Explicitly exiting the loop upon no data left removes the race Closes #395 * Improve Output when CDB.exe is Missing (#412) * Fix leak in QuicStreamSendFlush (#415) If send isn't enabled, the current send request was completed, but any existing requests left in the list were leaked. Closes #408 * Don't Use App Closed until 1-RTT Keys (#417) * Only Change Partitions for New Paths (#418) * Add cdb to path in run-executable azure script (#416) * Allow a Single Partition Update per Path (#420) * Fix Clog download * Fix copy code * fix up log script to print cpu info and timestamps * Revert "fix up log script to print cpu info and timestamps" This reverts commit b37879f. Co-authored-by: Nick Banks <nibanks@microsoft.com> Co-authored-by: Anthony Rossi <41394064+anrossi@users.noreply.github.com> Co-authored-by: Max <628527+Maximus-@users.noreply.github.com> Co-authored-by: Thad House <ThadHouse@users.noreply.github.com> Co-authored-by: Chris Gray <chgray@microsoft.com> * fix up log script to print cpu info and timestamps * prepare script * update sidecar for linux * update quic with changes from clog Co-authored-by: Nick Banks <nibanks@microsoft.com> Co-authored-by: Anthony Rossi <41394064+anrossi@users.noreply.github.com> Co-authored-by: Max <628527+Maximus-@users.noreply.github.com> Co-authored-by: Thad House <ThadHouse@users.noreply.github.com> Co-authored-by: Chris Gray <chgray@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In StartPing, when the QUIC_SEND_FLAG_FIN flag is set and there are no bytes left, if the SteamSend completes successfully, we are racing against stream callback, which deletes the instance we are currently executing. When the loop continues, BytesToSend is read again, which is the use after free. Explicitly exiting the loop upon no data left removes the race
Closes #395