Prevent sending CONN_CLOSE when closing the connection silently#5522
Prevent sending CONN_CLOSE when closing the connection silently#5522
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5522 +/- ##
=======================================
Coverage 84.65% 84.65%
=======================================
Files 59 59
Lines 18621 18623 +2
=======================================
+ Hits 15763 15765 +2
Misses 2858 2858 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if ((ClosedRemotely && Connection->State.ClosedRemotely) || | ||
| (!ClosedRemotely && Connection->State.ClosedLocally)) { | ||
| // | ||
| // Already closed. |
There was a problem hiding this comment.
Does it make sense to set ClosedSilently if the connection is already closed? Is it required?
There was a problem hiding this comment.
Yes. We could be in "closing" state, and be sending CONN_CLOSE frames as the answer to any incoming packet, which we should stop doing when we get a second "silent" close.
I am not sure when potentially get a second close, but the code below makes it clear it can happen.
There was a problem hiding this comment.
It would be nice to provide a specific high-level example scenario where that behavior is expected or required, but I can take on face value that a connection may "close" gracefully in some context, and then decide it wants to more forcefully ("silently") close.
There was a problem hiding this comment.
I'll add that in the follow up refactor PR I am working on.
My best guess is that it could happen if we have a protocol error causing a silent close right after an application close.
* Prevent sending CONN_CLOSE when closing the connection silently * Update clog
Description
This fixes an intermittent test failure due to connection being able to send a CONN_CLOSE frame after being silently closed.
This was a regression introduce by #5107.
If a connection was closed locally with the "silent" flag, it would still send a CONN_CLOSE frame when receiving a frame from the peer.
This fix prevents this behavior in a generic way by adding a check when adding send flags.
This minimal fix needs to be backported to v2.5.
A follow up PR with a refactoring of the connection close state will follow.
Testing
Already covered.
Documentation
N/A