Skip to content

fix: Return early if connection errors while processing first packet in datagram#3110

Merged
maddeleine merged 6 commits into
aws:mainfrom
maddeleine:conn_close_error
Jun 5, 2026
Merged

fix: Return early if connection errors while processing first packet in datagram#3110
maddeleine merged 6 commits into
aws:mainfrom
maddeleine:conn_close_error

Conversation

@maddeleine

@maddeleine maddeleine commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Release Summary:

Resolved issues:

N/A

Description of changes:

We had a regression in s2n-quic a while ago, we removed a return statement when the connection is closing due to an error. This causes s2n-quic to continue reading packets in a datagram after connection closure, which can make dc-quic panic under atypical network conditions. I wrote a test to reproduce the failure scenario and added the return statement back in.

Call-outs:

Testing:

Includes test

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@maddeleine maddeleine requested review from a team as code owners June 4, 2026 18:52
measure#recovery_metrics.congestion_window=12000
measure#recovery_metrics.bytes_in_flight=[REDACTED]
count#recovery_metrics.congestion_limited=false
count#packet_received=1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This change is due to the fact that now the client is now not continuing to process OneRtt packets after receiving the first ConnectionClosure above on line 256.

@WesleyRosenblum

Copy link
Copy Markdown
Contributor

Do we need to make any change to stop processing packets from subsequent datagrams when the connection is closing after an error?

@maddeleine

Copy link
Copy Markdown
Contributor Author

Do we need to make any change to stop processing packets from subsequent datagrams when the connection is closing after an error?

I want to put that fix in a separate PR.

Comment thread dc/s2n-quic-dc/src/path/secret/map/handshake.rs Outdated

@WesleyRosenblum WesleyRosenblum left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you check on those interop failures before merging?

@maddeleine maddeleine enabled auto-merge (squash) June 5, 2026 00:17
@maddeleine maddeleine merged commit b493d2a into aws:main Jun 5, 2026
251 of 254 checks passed
@maddeleine maddeleine deleted the conn_close_error branch June 5, 2026 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants