Skip to content

Fix volmeter teardown deadlock between audio thread and IPC - #1718

Merged
summeroff merged 1 commit into
stagingfrom
fix-volmeter-teardown-deadlock
Jun 1, 2026
Merged

Fix volmeter teardown deadlock between audio thread and IPC#1718
summeroff merged 1 commit into
stagingfrom
fix-volmeter-teardown-deadlock

Conversation

@summeroff

@summeroff summeroff commented May 31, 2026

Copy link
Copy Markdown
Contributor

Problem

osn::Volmeter::OBSCallback is invoked by libobs while the volmeter callback_mutex is held (signal_levels_updated calls it inside that lock). It then acquires the osn global mtx and, via Manager::find, the manager's internal_mutex.

The teardown paths take those osn locks first and then callback_mutex:

  • Destroy holds mtxobs_volmeter_remove_callbackcallback_mutex
  • ClearVolmeters' for_each holds internal_mutexobs_volmeter_remove_callbackcallback_mutex

That's two AB-BA cycles:

  • callback_mutex ↔ mtx (audio thread vs Destroy)
  • callback_mutex ↔ internal_mutex (audio thread vs ClearVolmeters)

Both can hang on the shutdown path (OBS_API::destroyOBS_APIClearVolmeters) while audio capture threads are still delivering audio.

Fix (osn-only, no libobs changes)

  • OBSCallback acquires mtx with std::try_to_lock and drops the update if it can't — the audio thread never blocks on mtx while holding callback_mutex. Volmeter updates are periodic and lossy, so a dropped frame during teardown is harmless. Breaks both callback_mutex ↔ mtx cycles.
  • ClearVolmeters collects the volmeters under the manager lock, then removes callbacks / clears outside for_each, so callback_mutex is never taken while the manager lock is held. Breaks the callback_mutex ↔ internal_mutex cycle.

shared_ptr ownership already prevents use-after-free across these paths; this PR only fixes the lock ordering.

Relationship to #1717

Independent and complementary. #1717 fixes the volmeter use-after-free (lifetime); this fixes the teardown deadlock (lock ordering). They touch different functions in the same file (Attach/Detach/header member vs OBSCallback/ClearVolmeters) and merge to staging without conflict.

OBSCallback runs while libobs holds the volmeter callback_mutex, then takes
the osn global mtx (and the manager's internal_mutex via find). The teardown
paths acquire those osn locks first and then take callback_mutex:
  - Destroy holds mtx, then obs_volmeter_remove_callback -> callback_mutex
  - ClearVolmeters' for_each holds internal_mutex, then
    obs_volmeter_remove_callback -> callback_mutex
This is two AB-BA cycles (callback_mutex<->mtx, callback_mutex<->internal_mutex)
that can hang on shutdown while audio is still flowing.

- OBSCallback now try_locks mtx and drops the update if busy, so the audio
  thread never blocks on mtx while holding callback_mutex.
- ClearVolmeters collects the volmeters under the manager lock, then removes
  callbacks and clears outside for_each, so callback_mutex is never taken while
  the manager lock is held.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a shutdown/teardown deadlock involving libobs volmeter callback_mutex and osn-side locks by changing lock acquisition behavior in the volmeter callback and adjusting teardown callback removal to avoid holding manager locks while touching libobs.

Changes:

  • Update osn::Volmeter::OBSCallback to try_lock the global mtx and drop a meter update frame if it can’t be acquired, preventing the audio thread from blocking while callback_mutex is held.
  • Rework osn::Volmeter::ClearVolmeters to collect active volmeters under the manager lock, then remove callbacks outside the manager for_each to avoid lock-order inversion with libobs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@summeroff
summeroff merged commit 5abfed7 into staging Jun 1, 2026
20 checks passed
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.

3 participants