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

[QUIC] Fix QUIC stream handle leak #56550

Merged
merged 1 commit into from
Jul 30, 2021

Conversation

CarnaViire
Copy link
Member

Discovered that while investigating stress tests failures. We had this code in the end of HandleEventShutdownComplete, but if we received a flag that connection was closed in that event, we branched early to HandleEventConnectionClose which lacked this logic.

Fixes #56151

@ghost
Copy link

ghost commented Jul 29, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Discovered that while investigating stress tests failures. We had this code in the end of HandleEventShutdownComplete, but if we received a flag that connection was closed in that event, we branched early to HandleEventConnectionClose which lacked this logic.

Fixes #56151

Author: CarnaViire
Assignees: -
Labels:

area-System.Net.Quic

Milestone: -

@geoffkizer
Copy link
Contributor

This is one of the causes of the leak I've seen in HTTP3 unit tests. Good to get this fixed.

@CarnaViire
Copy link
Member Author

/azp run runtime-libraries stress-http

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

@CarnaViire
Copy link
Member Author

Just for the record, stress failures are known issue #56310. I hoped this could mitigate it at least a little bit, but it did not.

@CarnaViire CarnaViire merged commit acb6b9b into dotnet:main Jul 30, 2021
@CarnaViire CarnaViire deleted the fix-quic-stream-handle-leak branch July 30, 2021 12:58
@karelz karelz added this to the 6.0.0 milestone Aug 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[QUIC] Memory leak when stream is finalized
4 participants