Skip to content

fix(apple): drain capture queue in destroy to avoid UAF on macOS 26#54

Merged
wysaid merged 3 commits intowysaid:mainfrom
acida:drain_capture_queue_on_destroy_on_mac
Apr 30, 2026
Merged

fix(apple): drain capture queue in destroy to avoid UAF on macOS 26#54
wysaid merged 3 commits intowysaid:mainfrom
acida:drain_capture_queue_on_destroy_on_mac

Conversation

@acida
Copy link
Copy Markdown
Contributor

@acida acida commented Apr 30, 2026

Summary

On macOS 26 we observed reproducible crashes inside ccap::ProviderImp::getFreeFrame() at std::mutex::lock(), throwing
std::system_error from an AVCaptureVideoDataOutput_Tundra callback on ccap.queue:

  ccap::ProviderImp::getFreeFrame()
  __56-[AVCaptureVideoDataOutput_Tundra _render:sampleBuffer:]_block_invoke
  _dispatch_lane_serial_drain

Root cause

In -[CameraCaptureObjc destroy] we call:

[_videoOutput setSampleBufferDelegate:nil queue:dispatch_get_main_queue()];

setSampleBufferDelegate:queue: does not guarantee that captureOutput: blocks already enqueued on the previous capture
queue have finished by the time it returns, especially when the new queue differs from the old one. On macOS 26's
reworked capture pipeline, a frame callback can still be in flight on the original _captureQueue while we tear down
ProviderImp (and its mutexes), producing a use-after-free that surfaces as a mutex::lock() system error.

Fix

After detaching the delegate, explicitly drain the original capture queue with a dispatch_sync barrier before
destroying session state. This guarantees no captureOutput: callback is still running against a soon-to-be-freed
ProviderImp.

  if (_captureQueue) {
      dispatch_sync(_captureQueue, ^{ });
  }

Impact

  • One-line semantic change, no API change.
  • Fixes a reproducible crash on macOS 26 (also a latent UAF on earlier versions).
  • Negligible cost: a single sync round-trip on a queue we are about to release anyway.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed crashes during camera shutdown when capture operations were still in progress.
    • Improved teardown sequencing to ensure in-flight capture callbacks complete without causing races or deadlocks, increasing camera stability during shutdown.

Copilot AI review requested due to automatic review settings April 30, 2026 07:40
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ad026969-51e7-423e-be64-e08d82cc2833

📥 Commits

Reviewing files that changed from the base of the PR and between 655fae3 and eed8817.

📒 Files selected for processing (1)
  • src/ccap_imp_apple.mm

Walkthrough

open stores a queue-specific key for _captureQueue; destroy removes the sample buffer delegate then conditionally performs a dispatch_sync no-op on the previous capture queue to drain pending captureOutput callbacks, skipping the drain when already executing on that queue.

Changes

Cohort / File(s) Summary
Capture queue sync and teardown
src/ccap_imp_apple.mm
open associates _captureQueue with kCcapCaptureQueueKey; destroy clears the sample buffer delegate and conditionally runs a dispatch_sync no-op on the previous capture queue to drain in-flight captureOutput callbacks, skipping the sync if called from the capture queue to avoid deadlock.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped along the capture lane, set a key to mark my trail,
Then waited gentle, synced a tap, so callbacks could prevail.
No tangled locks, no hurried end—just tidy, calm, and bright,
A rabbit's nod to code that waits, and tidies up at night. 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(apple): drain capture queue in destroy to avoid UAF on macOS 26' is clear, specific, and directly describes the main change: fixing a use-after-free (UAF) bug by draining the capture queue during destroy on macOS 26. It accurately summarizes the primary goal of the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

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 macOS 26 crash/UAF risk in the Apple camera backend teardown path by ensuring outstanding frame callbacks on the capture queue have completed before session/provider state is destroyed.

Changes:

  • Detach the AVCaptureVideoDataOutput sample buffer delegate and then synchronously drain the original _captureQueue during -[CameraCaptureObjc destroy].
  • Add detailed rationale comments documenting the macOS 26 capture pipeline behavior that can leave callbacks in-flight after delegate detachment.

Comment thread src/ccap_imp_apple.mm Outdated
If user code invokes close() from inside the new-frame callback, the
existing dispatch_sync drain would deadlock on the capture queue.
Tag the queue with dispatch_queue_set_specific and skip the drain when
destroy is re-entered on it — the in-flight callback that called us is
the only one that could touch the provider, and it is about to return.
Copy link
Copy Markdown

Copilot AI left a comment

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-time use-after-free on macOS 26 by ensuring no in-flight AVCaptureVideoDataOutput callbacks can continue running on the old capture queue while ProviderImp is being torn down.

Changes:

  • Tag the _captureQueue with a queue-specific key to detect re-entrant teardown calls originating from the capture callback.
  • After detaching the sample buffer delegate, explicitly drain the original capture queue via dispatch_sync (when safe) before continuing teardown.

@wysaid
Copy link
Copy Markdown
Owner

wysaid commented Apr 30, 2026

@Auggie review

@wysaid wysaid merged commit 1f2a916 into wysaid:main Apr 30, 2026
45 of 46 checks passed
@wysaid
Copy link
Copy Markdown
Owner

wysaid commented Apr 30, 2026

Thanks for the fix~

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