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

Remove assert from Http3Connection.SendAsync #74348

Conversation

rzikm
Copy link
Member

@rzikm rzikm commented Aug 22, 2022

Fixes #74162.

The assert in question was invalid, because we may close the connection gracefully without prior Http3Connection.Abort call when responding to a GOAWAY.

This PR also filters OperationAborted QuicExceptions from server-initiated streams since there is no need to call Abort when we already close the connection (and setting the _abortException in that case would be confusing).

@ghost
Copy link

ghost commented Aug 22, 2022

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

Issue Details

Fixes #74162.

The assert in question was invalid, because we may close the connection gracefully without prior Http3Connection.Abort call when responding to a GOAWAY.

This PR also filters OperationAborted QuicExceptions from server-initiated streams since there is no need to call Abort when we already close the connection (and setting the _abortException in that case would be confusing).

Author: rzikm
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@rzikm rzikm force-pushed the 74162-Assert-failed-SystemNetHttpHttp3ConnectionSendAsync branch from 4963dff to 59e06e8 Compare August 22, 2022 14:41
@rzikm rzikm requested a review from a team August 22, 2022 14:43
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

@rzikm rzikm merged commit 1f761f5 into dotnet:main Aug 23, 2022
@karelz karelz modified the milestones: 7.0.0, 8.0.0 Aug 23, 2022
@rzikm
Copy link
Member Author

rzikm commented Aug 23, 2022

/backport to release/7.0

@github-actions
Copy link
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/2911235661

@github-actions
Copy link
Contributor

@rzikm backporting to release/7.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Patch format detection failed.
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

rzikm added a commit to rzikm/dotnet-runtime that referenced this pull request Aug 23, 2022
* Remove assert in System.Net.Http.Http3Connection.SendAsync

* Minor change
rzikm added a commit to rzikm/dotnet-runtime that referenced this pull request Aug 24, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 22, 2022
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.

Assert failed: System.Net.Http.Http3Connection.SendAsync in coreclr Linux_musl x64 Debug
3 participants