-
Notifications
You must be signed in to change notification settings - Fork 2
server: cleanup pending_session_id_rotations for disconnected clients #184
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
Merged
kp-mariappan-ramasamy
merged 4 commits into
main
from
cvpn-1865-cleanup-pending-sessionid-rot
Apr 7, 2025
Merged
server: cleanup pending_session_id_rotations for disconnected clients #184
kp-mariappan-ramasamy
merged 4 commits into
main
from
cvpn-1865-cleanup-pending-sessionid-rot
Apr 7, 2025
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When we initate a session id rotation, we store the new session id along with strong pointer of lightway_server::Connection inside the HashMap. We remove this HashMap entry, when session rotation is completed. In case the session rotation does not succeed, HashMap entry is not cleared i.e. the strong pointer to Connection inside entry, is also not released and thus Connection gets leaked. There are two issues in the current approach: 1. Using strong pointer to Connection inside pending_session_id_rotations HashMap 2. Not cleaning up the pending_session_id_rotations HashMap periodically First problem is fixed in this commit. i.e Use Weak pointer to Connection in pending_session_id_rotations. This will allow the Connection getting freed when client gets disconnected. Second issue will be fixed in further commit.
When we initate a session id rotation, we store the new session id along with strong pointer of lightway_server::Connection inside the HashMap. We remove this HashMap entry, when session rotation is completed. In case the session rotation does not succeed, HashMap entry is not cleared i.e. the strong pointer to Connection inside entry, is also not released and thus Connection gets leaked. There are two issues in the current approach: 1. Using strong pointer to Connection inside pending_session_id_rotations HashMap 2. Not cleaning up the pending_session_id_rotations HashMap periodically First problem is fixed in previous commit by using Weak pointer to Connection in pending_session_id_rotations. This commit addresses the second issue by creating a task which periodically checks for connection and cleansup if connection is already invalid/disconnected.
to avoid code repetition.
979d9d0
to
8c144c7
Compare
Code coverage summary for 8c144c7:
✅ Region coverage 54% passes |
kp-max-li
approved these changes
Apr 7, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When we initiate a session id rotation, we store the new session id along with
strong pointer of lightway_server::Connection inside the HashMap. We remove this
HashMap entry, when session rotation is completed.
In case the session rotation does not succeed, HashMap entry is not cleared
i.e. the strong pointer to Connection inside entry, is also not released and thus Connection gets leaked.
There are two issues in the current approach:
1. Using strong pointer to Connection inside pending_session_id_rotations HashMap
2. Not cleaning up the pending_session_id_rotations HashMap periodically
First problem is fixed by using Weak pointer to Connection inpending_session_id_rotations.
And the second issue by creating a task which periodically checks for connection and clean-up if connection is already invalid/disconnected.
Motivation and Context
Some of the connections are not dropped even after the client has disconnected.
On debugging, found these connections are stuck in pending session id rotation state.
How Has This Been Tested?
Verified manually Connections structs are dropped while normal connection.
There is no clear way to repro client getting stucked at pending session id rotation.
Types of changes
Checklist:
main