Skip to content

CP: Do not send CONNECTION_CLOSE frames in draining state.#5492

Merged
guhetier merged 1 commit intorelease/2.5from
guhetier/conn_close_cp
Oct 7, 2025
Merged

CP: Do not send CONNECTION_CLOSE frames in draining state.#5492
guhetier merged 1 commit intorelease/2.5from
guhetier/conn_close_cp

Conversation

@guhetier
Copy link
Collaborator

@guhetier guhetier commented Oct 6, 2025

Description

Cherry-pick #5477

Some spurious test failures show a pattern when the two peers entered a loop of sending CONNECTION_CLOSE frames to each other. This regression was introduced by #5107.

According to RFC 9000:

  • When a peer sends a CONNECTION_CLOSE frame, it enters the "closing" state and should answer any incoming packet on the connection with a new CONNECTION_CLOSE frame (subject to rate limiting), to ensure the peer get the CONNECTION_CLOSE.
  • When a peer receives a CONNECTION_CLOSE frame, it enters the "draining" state and should not send any more packets.

(The RFC is not explicit about what happens when a peer in the "closing" state receives a CONNECTION_CLOSE, but since it means both peers are closing the connection, it can stop sending additional CONNECTION_CLOSE).

However, #5107 was re-sending CONNECTION_CLOSE frame both in "closing" and "draining" state. This PR fixes it.

Testing

CI. The issue is timing dependent and can't be reliably reproduced.

Documentation

N/A

@guhetier guhetier requested a review from a team as a code owner October 6, 2025 20:56
@guhetier guhetier modified the milestones: Release 2.5, Future Oct 6, 2025
@guhetier
Copy link
Collaborator Author

guhetier commented Oct 6, 2025

MIght need to CP #5453 too.

@guhetier guhetier changed the title Do not send CONNECTION_CLOSE frames in draining state. CP: Do not send CONNECTION_CLOSE frames in draining state. Oct 6, 2025
@codecov
Copy link

codecov bot commented Oct 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.97%. Comparing base (bdeea77) to head (103b62a).
⚠️ Report is 1 commits behind head on release/2.5.

Additional details and impacted files
@@               Coverage Diff               @@
##           release/2.5    #5492      +/-   ##
===============================================
- Coverage        87.38%   86.97%   -0.42%     
===============================================
  Files               59       59              
  Lines            18173    18174       +1     
===============================================
- Hits             15880    15806      -74     
- Misses            2293     2368      +75     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@guhetier guhetier force-pushed the guhetier/conn_close_cp branch from 44d70fd to 103b62a Compare October 6, 2025 23:51
@guhetier guhetier merged commit f5ebf12 into release/2.5 Oct 7, 2025
285 checks passed
@guhetier guhetier deleted the guhetier/conn_close_cp branch October 7, 2025 19:04
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.

2 participants