Bound the async exchange with the inline queue owner - #779
Merged
Conversation
TrySendAsync had no deadline of any kind. Configure sets SendTimeout and ReceiveTimeout, which apply only to synchronous calls, and every async read and write used the caller's token — which is default from DiffRunner.AddInlineAsync, because Verify passes none, and likewise from AddDeleteAsync and InnerLaunchAsync. So an owner that accepted the connection and then stopped answering hung the failing test indefinitely. That is not hypothetical: the owner answers on its listener thread, so a connection can sit behind an accept that is itself waiting up to ten seconds on InlineApplier's cross process mutex, and a viewer stopped in a debugger does the same thing for as long as it is stopped. The synchronous TrySend gives up after three seconds; the async path, the one Verify actually takes, waited forever. Link a CancellationTokenSource with a 30 second deadline — longer than the sync wait to leave room for the applier mutex — and use its token everywhere the caller's was used. The token also closes the socket, because that is the only thing that unblocks every target: pre-net7 ReadToEndAsync takes no token at all and net462 has no cancellable connect or write either. A timeout is reported as absence, so the caller launches a viewer or stages the patch rather than waiting on a process that has stopped listening, and traced as "present but unresponsive" so the two stay tellable apart. port and wait overrides mirror the synchronous overload so the new test can hold a connection open on its own ephemeral port without touching anything static.
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.
TrySendAsync had no deadline of any kind. Configure sets SendTimeout and ReceiveTimeout, which apply only to synchronous calls, and every async read and write used the caller's token — which is default from DiffRunner.AddInlineAsync, because Verify passes none, and likewise from AddDeleteAsync and InnerLaunchAsync.
So an owner that accepted the connection and then stopped answering hung the failing test indefinitely. That is not hypothetical: the owner answers on its listener thread, so a connection can sit behind an accept that is itself waiting up to ten seconds on InlineApplier's cross process mutex, and a viewer stopped in a debugger does the same thing for as long as it is stopped. The synchronous TrySend gives up after three seconds; the async path, the one Verify actually takes, waited forever.
Link a CancellationTokenSource with a 30 second deadline — longer than the sync wait to leave room for the applier mutex — and use its token everywhere the caller's was used. The token also closes the socket, because that is the only thing that unblocks every target: pre-net7 ReadToEndAsync takes no token at all and net462 has no cancellable connect or write either.
A timeout is reported as absence, so the caller launches a viewer or stages the patch rather than waiting on a process that has stopped listening, and traced as "present but unresponsive" so the two stay tellable apart. port and wait overrides mirror the synchronous overload so the new test can hold a connection open on its own ephemeral port without touching anything static.