Skip to content

Fix flaky AbortedStream test in Kestrel #8010

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 28, 2019
Merged

Conversation

jkotalik
Copy link
Contributor

I think this is causing https://github.com/aspnet/AspNetCore-Internal/issues/1879. This failure is only occurring on linux agents, so will run a few times to verify.

@jkotalik
Copy link
Contributor Author

jkotalik commented Feb 27, 2019

The reason I think my PR introduced it was because the issue I filed was originally on https://dev.azure.com/dnceng/public/_build/results?buildId=106545, which was the original PR. I thought this test was an artifact of 57092e9, but I think it's due to #7933 because the failure is due to pipe completion.

@Tratcher
Copy link
Member

Tratcher commented Feb 27, 2019

Your PR changes the request body pipe, but the stacktrace for the test is in the response connection pipe. They shouldn't be related. [Edit] nevermind, that test is writing a request frame, but it's still for the connection pipe rather than the body pipe. At most you changed the timing.

@halter73
Copy link
Member

halter73 commented Feb 28, 2019

We should add the following to the top of the flaky test instead.

            // Remove callback that completes _pair.Application.Output on abort.
            _mockConnectionContext.Reset()

Edit: and start logging the exception in the default _mockConnectionContext.Abort callback.

@jkotalik jkotalik force-pushed the jkotalik/AbortedStream branch from bc4e767 to 0dc64be Compare February 28, 2019 00:19
Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pending most recent comment about the exception stack trace

@jkotalik jkotalik changed the title Revert Call Complete on Http2Stream when stream is done earlier Fix flaky AbortedStream test in Kestrel Feb 28, 2019
@jkotalik jkotalik merged commit 555e460 into master Feb 28, 2019
@jkotalik jkotalik deleted the jkotalik/AbortedStream branch February 28, 2019 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants