isolating and improving timeout tests#11839
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the C# TimeoutToken test script to avoid high CPU usage and thread blocking during timeout scenarios, improving CI stability under resource-constrained conditions.
Changes:
- Replaced the tight spin-wait loop with an infinite
Task.Delay(..., token)that completes immediately on cancellation. - Replaced
Thread.Sleepwithawait Task.Delayin the non-token scenario to avoid blocking a thread.
cjaliaga
approved these changes
Jun 18, 2026
soninaren
approved these changes
Jun 18, 2026
Member
Author
|
ok that clearly didn't work -- adding diagnostics to see if we can catch it and determine where mem is leaking. i'll re-ping when it's sready for another review |
Member
Author
|
/azp run host.integration-tests |
|
Azure Pipelines failed to run 1 pipeline(s). |
Member
Author
|
/azp run host.integration-tests |
|
Azure Pipelines failed to run 1 pipeline(s). |
Member
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s), but failed to run 1 pipeline(s). |
Member
Author
|
/azp run host.integration-tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Member
Author
|
/azp run host.integration-tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
510ed9b to
651dbc3
Compare
kshyju
approved these changes
Jun 19, 2026
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
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.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TheTimeoutTest_UsingToken_CSharpwould sometimes fail -- and it seemed to be whenever the CI machine was reporting health issues (low-memory). Analysis suggests that the tight while loop this test used may have caused it to fail in such conditions by consuming a thread while waiting for the test scenario to run.Changing the behavior in the test function to be more cpu-friendly by reacting directly to the token rather than spinning. Also swapping the other test scenario to use aTaskrather than blocking a thread.Not 100% guaranteed that this will fix the issue, but it seems like the right direction.It's entirely possible that the low-memory conditions are caused by other tests leaking somewhere, but that's analysis I'll be doing elsewhere.Edited:
It turns out that the 3 timeout tests were leaking massive memory. During some runs they'd use upwards of 2.4gb. Dump analysis showed tens of thousands of CancellationToken registrations that appeared to come from configuration somewhere.
The one big oddity with these tests was that they wired up both the production configuration and the test configurations. It's possible that something in there was causing some cyclical change token registration causing chaos.
In reality, these tests are testing in-proc cancellation and likely don't even impact anything we do today which is strictly out-of-proc. So rather than chase it down to the exact culprit, I:
I've run this several times, both with and without memory diagnostics running and it seems stable now.
Pull request checklist
IMPORTANT: Currently, changes must be backported to the
in-procbranch to be included in Core Tools and non-Flex deployments.in-procbranch is not requiredrelease_notes.md