Skip to content

Fix volmeter use-after-free on audio thread during teardown#1717

Merged
summeroff merged 2 commits into
stagingfrom
fix-volmeter-source-uaf
Jun 2, 2026
Merged

Fix volmeter use-after-free on audio thread during teardown#1717
summeroff merged 2 commits into
stagingfrom
fix-volmeter-source-uaf

Conversation

@summeroff

@summeroff summeroff commented May 31, 2026

Copy link
Copy Markdown
Contributor

Problem

Sentry crashes on the WASAPI real-time capture thread inside the volmeter chain (obs_source_output_audiosource_signal_audio_datavolmeter_source_data_received):

  • 1.20.9EXCEPTION_ACCESS_VIOLATION_EXEC in signal_levels_updated (calling a dangling cb.callback)
  • 1.21.2EXCEPTION_ACCESS_VIOLATION_READ in the peak path (get_sample_peak)

Both are a use-after-free of the obs_volmeter.

Root cause

The volmeter holds only a weak reference to its attached source. When the source is concurrently going to refcount 0, obs_volmeter_detach_source's weak→strong upgrade (obs_weak_source_get_source) returns NULL, so it skips obs_source_remove_audio_capture_callback. The volmeter is then freed, leaving a dangling {volmeter_source_data_received, freed-volmeter} entry in the source's audio_cb_list. The RT capture thread (not stopped until the deferred destroy phase) can call into the freed volmeter before obs_source_destroy frees that list.

Triggered by Volmeter::Destroy/Detach (IPC thread) racing source destruction — e.g. device removal / scene teardown. The crash logs show matching device churn (default-device change, HD60 S, Voicemod).

Fix

Serialize the lifetime on the osn side: hold a strong source reference (via findAndRef) for the duration of the attachment, released only after detach. The source can no longer reach refcount 0 while attached, so the weak-ref upgrade inside obs_volmeter_detach_source always succeeds and the audio capture callback is removed cleanly before the volmeter is freed.

No libobs changes; blast radius stays in obs-studio-server.

Tradeoff

Holding a strong ref defers a source's destruction until its volmeter is detached/destroyed. Streamlabs pairs volmeter and source teardown per audio source, so this is bounded.

The volmeter held only a weak reference to its attached source. When the
source was concurrently going to refcount 0, obs_volmeter_detach_source's
weak->strong upgrade returned NULL and it skipped
obs_source_remove_audio_capture_callback, leaving a dangling
{volmeter_source_data_received, freed-volmeter} entry in the source's
audio_cb_list. The WASAPI RT capture thread could then call into the freed
volmeter before obs_source_destroy frees that list, crashing in
signal_levels_updated (EXCEPTION_ACCESS_VIOLATION_EXEC) or the peak path
(EXCEPTION_ACCESS_VIOLATION_READ).

Hold a strong reference to the attached source for the lifetime of the
attachment (via findAndRef), released only after detach. The source can no
longer reach refcount 0 while attached, so the detach always removes the
audio capture callback before the volmeter is freed.

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

Fixes a use-after-free crash on the WASAPI audio thread by ensuring the obs_source referenced by an osn::Volmeter cannot reach refcount 0 while still attached. A strong source reference is held for the duration of the attachment and released only after obs_volmeter_detach_source runs, guaranteeing the weak→strong upgrade inside the detach path succeeds and the audio capture callback is removed before the volmeter is freed.

Changes:

  • Add an OBSSourceAutoRelease source_ref member to osn::Volmeter to keep the attached source alive.
  • In Attach, acquire the source via findAndRef and store it into source_ref.
  • In Detach, release source_ref only after obs_volmeter_detach_source completes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
obs-studio-server/source/osn-volmeter.hpp Includes <obs.hpp> and adds OBSSourceAutoRelease source_ref member with explanatory comment.
obs-studio-server/source/osn-volmeter.cpp Switches Attach to findAndRef, stores the strong reference, and clears it after detach in Detach.

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

@@ -143,6 +144,7 @@ void osn::Volmeter::Detach(void *data, const int64_t id, const std::vector<ipc::

meter->uid_source = INVALID_ID;
obs_volmeter_detach_source(meter->self);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This still calls obs_volmeter_detach_source() while holding OSN's global mtx. With the new strong source_ref, libobs detach can now reliably reach obs_source_remove_audio_capture_callback(), which takes source->audio_cb_mutex.

The audio thread can hold that mutex while dispatching volmeter_source_data_received -> signal_levels_updated -> Volmeter::OBSCallback, where OSN tries to take mtx. That gives a deadlock opportunity: detach thread holds mtx waiting for audio_cb_mutex, audio thread holds audio_cb_mutex waiting for mtx.

Can we split the lock scope here? Grab the shared_ptr<Volmeter> under mtx, clear OSN state as needed, release mtx, then call obs_volmeter_detach_source(), and only release source_ref after detach completes.

obs_volmeter_detach_source takes the source audio_cb_mutex, which the
audio thread holds before taking mtx in OBSCallback. Detaching under mtx
risked an ABBA deadlock. Split the lock scope: clear state and move out
source_ref under mtx, then detach and release the source after the lock.

Addresses review feedback.

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@summeroff summeroff merged commit 506dae6 into staging Jun 2, 2026
29 of 30 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