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

[Backport 4.9] OF-2921: Prevent deadlock by not broadcasting synchronously #2647

Merged
merged 3 commits into from
Jan 4, 2025

Conversation

github-actions[bot]
Copy link

@github-actions github-actions bot commented Jan 4, 2025

Description

Backport of #2635 to 4.9.

guusdk added 3 commits January 4, 2025 20:55
When a client drops offline, a presence update is broadcast on behalf of that client. We've observed deadlocks occurring when this happens (as the broadcast itself, especially when it occurs in context in of MUC rooms, will acquire mutexes).

There appears to be little reason for this broadcast to happen synchronous to the process of session termination process. To reduce the likelihood of deadlocks, this commit makes the broadcast happen in an asynchronous process.

(cherry picked from commit b98a83d)
When stream management is enabled, deliver and record stanzas under a mutex. If it's not enabled, don't acquire the lock to reduce lock contention.

(cherry picked from commit debf053)
When a websocket error is raised while holding another mutex (eg: when processing a MUC stanza) then having _another_ mutex introduces a deadlock risk. This has been observed to happen in the wild.

In this commit, the mutex held when processing a websocket error is removed.

(cherry picked from commit 6428f38)
@akrherz akrherz closed this Jan 4, 2025
@akrherz akrherz reopened this Jan 4, 2025
@akrherz akrherz merged commit 2214420 into 4.9 Jan 4, 2025
18 checks passed
@akrherz akrherz deleted the backport-2635-to-4.9 branch January 4, 2025 21:15
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