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

Take lock before accessing sessions collection #6629

Merged
merged 2 commits into from
Nov 12, 2024

Conversation

eddyashton
Copy link
Member

Another fix pulled out of #6616.

We take RPCSessions::lock before accessing RPCSessions::sessions, except in these 2 handlers. TSAN noticed. This meant that (potentially) while we're processing an inbound message, we could close and delete the Session object. We now take take shared ownership while processing further.

@eddyashton eddyashton added 5.x-todo PRs which should be backported to 5.x run-long-test Run Long Test job labels Nov 11, 2024
@eddyashton eddyashton requested a review from a team November 11, 2024 17:21
@achamayou achamayou added this pull request to the merge queue Nov 12, 2024
@achamayou achamayou added the auto-backport Automatically backport this PR to LTS branch label Nov 12, 2024
Merged via the queue into microsoft:main with commit 944fef2 Nov 12, 2024
21 checks passed
@achamayou achamayou deleted the rpc_sessions_locks branch November 12, 2024 07:32
ghost pushed a commit that referenced this pull request Nov 12, 2024
Co-authored-by: Amaury Chamayou <[email protected]>
(cherry picked from commit 944fef2)
@ghost ghost added the backported This PR was successfully backported to LTS branch label Nov 12, 2024
achamayou pushed a commit that referenced this pull request Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.x-todo PRs which should be backported to 5.x auto-backport Automatically backport this PR to LTS branch backported This PR was successfully backported to LTS branch run-long-test Run Long Test job
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants