Skip to content

Fix possible deadlock in conference (audio & video)#4243

Merged
nanangizz merged 4 commits intomasterfrom
conf-handle-op-fix
Jan 7, 2025
Merged

Fix possible deadlock in conference (audio & video)#4243
nanangizz merged 4 commits intomasterfrom
conf-handle-op-fix

Conversation

@nanangizz
Copy link
Copy Markdown
Member

@nanangizz nanangizz commented Jan 6, 2025

As reported by @LeonidGoltsblat here:

The OP_REMOVE_PORT function calls the group lock handler while holding the conf->mutex. As with any callback invoked under a lock, there is a potential risk of deadlock. We cannot predict what actions the user’s application may take in the callback, for instance, it might acquire the SIP dialog mutex, which could lead to a lock ordering issue similar to the one described in bug report #3183.
I don't think it's necessary to call handle_op_queue under the mutex lock. Mutex protection is only required to protect conf->op_queue, not the handle_op_queue() function itself. This way, we can invoke the callback outside the conference mutex lock.

This PR is trying to fix that using the suggested solution, with a couple of additional touches:

  • Limit the op processing number, as the op processing is done in media processing thread and the op queue may grow (theoritically indefinitely), a limit will reduce/avoid starvation.
  • Protect conf->ports[port] = NULL in OP_REMOVE_PORT with mutex to avoid race conditions with other APIs that access conf->ports[] (with mutex).

@nanangizz nanangizz merged commit 709ecdc into master Jan 7, 2025
@nanangizz nanangizz deleted the conf-handle-op-fix branch January 7, 2025 02:22
BarryYin pushed a commit to BarryYin/pjproject that referenced this pull request Feb 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants