Skip to content
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

Quarantine StreamPool_StreamIsInvalidState_DontReturnedToPool #40626

Closed
TanayParikh opened this issue Mar 9, 2022 · 7 comments · Fixed by #57847
Closed

Quarantine StreamPool_StreamIsInvalidState_DontReturnedToPool #40626

TanayParikh opened this issue Mar 9, 2022 · 7 comments · Fixed by #57847
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-kestrel HTTP2 test-failure test-fixed
Milestone

Comments

@TanayParikh
Copy link
Contributor

TanayParikh commented Mar 9, 2022

Failing Test(s)

  • Microsoft.AspNetCore.Server.Kestrel.Core.Tests.Http2ConnectionTests.StreamPool_StreamIsInvalidState_DontReturnedToPool

Error Message

Assert.True() Failure
Expected: True
Actual:   False

Stacktrace

   at Microsoft.AspNetCore.Server.Kestrel.Core.Tests.Http2ConnectionTests.StreamPool_StreamIsInvalidState_DontReturnedToPool() in /_/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs:line 578
--- End of stack trace from previous location ---

Logs

Http2ConnectionTests_StreamPool_StreamIsInvalidState_DontReturnedToPool.log

Build

https://dev.azure.com/dnceng/public/_build/results?buildId=1651060&view=ms.vss-test-web.build-test-results-tab&runId=45546776&resultId=108023&paneView=dotnet-dnceng.dnceng-build-release-tasks.helix-test-information-tab

@amcasey
Copy link
Member

amcasey commented Aug 22, 2024

This test hasn't failed in 30 days.

@amcasey
Copy link
Member

amcasey commented Aug 22, 2024

Lines have moved, but it's probably this assert.

@amcasey
Copy link
Member

amcasey commented Aug 22, 2024

The test is waiting for the stream to complete, but it's not obvious that that implies the stream will also have been disposed (which is what causes the output producer to be disposed).

@amcasey
Copy link
Member

amcasey commented Aug 22, 2024

While it's not clear to me why we can assume that dispose will have been called before the tick handler returns, putting a delay ahead of the dispose call doesn't break the test.

@amcasey
Copy link
Member

amcasey commented Sep 9, 2024

It certainly looks like the continuation of Pipe.CancelPendingRead is happening on the same thread and that's why the output producer is disposed by the time TriggerTick returns. This seems like it might not happen with a ThreadPoolScheduler, but locally, I'm seeing an InlineScheduler.

@amcasey
Copy link
Member

amcasey commented Sep 9, 2024

It looks like the write scheduler is unconditionally inline, whereas the read scheduler comes from the ServiceContext.

Edit: ...which has an inline scheduler in this test.

@amcasey
Copy link
Member

amcasey commented Sep 10, 2024

I can't explain what changed, but the code (linked above) doesn't look racy now.

amcasey added a commit to amcasey/aspnetcore that referenced this issue Sep 12, 2024
It hasn't failed in 30 days and code inspection suggests it can't fail in the same way as it did two years ago.

Fixes dotnet#40626
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-kestrel HTTP2 test-failure test-fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants