Skip to content

Remove assert from Http3Connection.SendAsync - #74348

Merged
rzikm merged 2 commits into
dotnet:mainfrom
rzikm:74162-Assert-failed-SystemNetHttpHttp3ConnectionSendAsync
Aug 23, 2022
Merged

Remove assert from Http3Connection.SendAsync#74348
rzikm merged 2 commits into
dotnet:mainfrom
rzikm:74162-Assert-failed-SystemNetHttpHttp3ConnectionSendAsync

Conversation

@rzikm

Copy link
Copy Markdown
Member

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).

@ghostghost assigned rzikmAug 22, 2022
@ghost

Copy link
Copy Markdown

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

@wfurtwfurt left a comment

Copy link
Copy Markdown
Member

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:mainAug 23, 2022
@karelzkarelz modified the milestones: 7.0.0, 8.0.0Aug 23, 2022
@rzikm

Copy link
Copy Markdown
MemberAuthor

/backport to release/7.0

@github-actions

Copy link
Copy Markdown
Contributor

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

@github-actions

Copy link
Copy Markdown
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
@ghostghost locked as resolved and limited conversation to collaborators Sep 22, 2022
Sign up for freeto 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

@rzikm@wfurt@karelz