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: Add API to allow detecting when peer aborts receive #58229

Closed
geoffkizer opened this issue Aug 27, 2021 · 3 comments · Fixed by #58236
Closed

QUIC: Add API to allow detecting when peer aborts receive #58229

geoffkizer opened this issue Aug 27, 2021 · 3 comments · Fixed by #58236
Assignees
Labels
area-System.Net.Quic enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@geoffkizer
Copy link
Contributor

Currently, the only way to detect that a peer has aborted receiving is to try to send and see if it fails.

If a user is periodically sending data on the QuicStream, for example from a backend SQL query or other source, this means they will not discover the abort until they finish the query and try to write to the QuicStream. If this is expensive and/or long-running, that could be problematic. It would be better if they could discover the peer abort promptly and cancel any in-progress work.

This happens specifically in ASP.NET with a "streamed response", where the request handling logic is writing to the response stream periodically. Currently, Kestrel has no way to be notified that the HTTP client has aborted the response.

To address this, we should add an API like so:

    public Task WaitForWriteCompletionAsync(CancellationToken cancellationToken);

If the peer aborts writes (or closes the connection, or there’s a connection failure), this would throw the appropriate error (QuicStreamAbortedException or QuicConnectionAbortedException, etc).
If the write stream completes successfully, this would return without exception.

The logic to implement this should be similar to the old ShutdownWriteCompleted method that got removed here: #55981. This code would detect when the write stream completed successfully.

However, note that the old ShutdownWriteCompleted code does not seem to have handled the peer receive abort case. We will need to add code in HandleEventPeerRecvAborted to do this.

@geoffkizer geoffkizer added this to the 6.0.0 milestone Aug 27, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Aug 27, 2021
@ghost
Copy link

ghost commented Aug 27, 2021

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

Issue Details

Currently, the only way to detect that a peer has aborted receiving is to try to send and see if it fails.

If a user is periodically sending data on the QuicStream, for example from a backend SQL query or other source, this means they will not discover the abort until they finish the query and try to write to the QuicStream. If this is expensive and/or long-running, that could be problematic. It would be better if they could discover the peer abort promptly and cancel any in-progress work.

This happens specifically in ASP.NET with a "streamed response", where the request handling logic is writing to the response stream periodically. Currently, Kestrel has no way to be notified that the HTTP client has aborted the response.

To address this, we should add an API like so:

    public Task WaitForWriteCompletionAsync(CancellationToken cancellationToken);

If the peer aborts writes (or closes the connection, or there’s a connection failure), this would throw the appropriate error (QuicStreamAbortedException or QuicConnectionAbortedException, etc).
If the write stream completes successfully, this would return without exception.

The logic to implement this should be similar to the old ShutdownWriteCompleted method that got removed here: #55981. This code would detect when the write stream completed successfully.

However, note that the old ShutdownWriteCompleted code does not seem to have handled the peer receive abort case. We will need to add code in HandleEventPeerRecvAborted to do this.

Author: geoffkizer
Assignees: -
Labels:

area-System.Net.Quic

Milestone: 6.0.0

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 27, 2021
@karelz karelz added enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels Aug 27, 2021
@karelz
Copy link
Member

karelz commented Sep 1, 2021

Reopening to track backport to 6.0 branch

@karelz karelz reopened this Sep 1, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Sep 1, 2021
@karelz
Copy link
Member

karelz commented Sep 1, 2021

Fixed in 6.0 RC2 in PR #58415.
Fixed in 7.0 in PR #58236.

@karelz karelz closed this as completed Sep 1, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Oct 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Quic enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants