-
Notifications
You must be signed in to change notification settings - Fork 29
fix(macos): avoid camera-permission deadlock when opening off the main thread #56
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
Merged
+39
−27
Merged
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
fc618ea
fix(macos): avoid camera-permission deadlock when opening off the mai…
wysaid bb7f44b
test(macos): harden permission deadlock test against hangs and UAF
wysaid bd4e03c
ci(windows): fix VS builds after windows-latest moved to the VS 2026 …
wysaid d21f8d5
ci(windows): fix VS2026 shared-link test vcvars path
wysaid 7f30f19
test(playback): make GetCurrentTimeProgression robust to CI timing
wysaid dda3119
ci(windows): make VS2026 configure idempotent for build-cache hits
wysaid 150f212
fix(macos): notify condition_variable under lock; address review find…
wysaid d78dbe8
refactor(macos): drop the C++ wait helper, keep the minimal GCD fix
wysaid File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| /** | ||
| * @file ccap_apple_async.h | ||
| * @author wysaid (this@wysaid.org) | ||
| * @brief Run a callback-based asynchronous request and block until it completes, | ||
| * without bouncing the request onto the main dispatch queue. | ||
| * | ||
| * macOS callback APIs such as `AVCaptureDevice requestAccessForMediaType:` deliver | ||
| * their completion on an internal queue, not on the caller's run loop. ccap used to | ||
| * dispatch the permission request onto the main queue for non-main-thread callers, | ||
| * which deadlocks whenever nothing is servicing that queue -- e.g. a ccap::Provider | ||
| * opened from a worker thread in a process that has no CFRunLoop on its main thread | ||
| * (a Node.js / Electron addon, a head-less multi-threaded service, ...). | ||
| * | ||
| * runBlockingAsyncRequest() starts the request on the *calling* thread and blocks on a | ||
| * portable condition variable until the supplied continuation is invoked, so it is | ||
| * safe to call from any thread regardless of run-loop state. | ||
| * | ||
| * Covered by tests/test_apple_permission.cpp. | ||
| */ | ||
|
|
||
| #pragma once | ||
|
|
||
| #if defined(__APPLE__) | ||
|
|
||
| #include <condition_variable> | ||
| #include <functional> | ||
| #include <mutex> | ||
|
|
||
| namespace ccap | ||
| { | ||
|
|
||
| /** | ||
| * Invoke @p start on the current thread and block until the continuation that | ||
| * @p start receives (its `done` argument) is called. @p start may invoke `done` from | ||
| * any thread or queue. The request is never dispatched to the main queue, so this | ||
| * cannot deadlock when no run loop is servicing it. | ||
| */ | ||
| inline void runBlockingAsyncRequest(const std::function<void(const std::function<void()>& done)>& start) | ||
| { | ||
| std::mutex mutex; | ||
| std::condition_variable cv; | ||
| bool finished = false; | ||
|
|
||
| start([&mutex, &cv, &finished]() { | ||
| { | ||
| std::lock_guard<std::mutex> lock(mutex); | ||
| finished = true; | ||
| } | ||
| cv.notify_one(); | ||
| }); | ||
|
|
||
| std::unique_lock<std::mutex> lock(mutex); | ||
| cv.wait(lock, [&finished]() { return finished; }); | ||
| } | ||
|
|
||
| } // namespace ccap | ||
|
|
||
| #endif // __APPLE__ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| /** | ||
| * @file test_apple_permission.cpp | ||
| * @brief Regression test for the macOS camera-permission request deadlock. | ||
| * | ||
| * ccap::runBlockingAsyncRequest() (used by ProviderApple::open) must run the | ||
| * permission request on the calling thread and must NOT bounce it onto the main | ||
| * dispatch queue. Otherwise Provider::open() hangs forever when called from a worker | ||
| * thread in a process whose main thread is not running a run loop -- exactly the | ||
| * situation a Node.js / Electron addon or any head-less multi-threaded embedder | ||
| * creates. | ||
| * | ||
| * We exercise the real helper with a *simulated* asynchronous request: a short | ||
| * countdown that fires the completion from a background thread, just like | ||
| * AVCaptureDevice requestAccessForMediaType: delivers its completion off the caller's | ||
| * run loop. No camera is required, so this runs deterministically in CI. | ||
| * | ||
| * On non-Apple platforms this file compiles to an empty translation unit. | ||
| */ | ||
|
|
||
| #if defined(__APPLE__) | ||
|
|
||
| #include <gtest/gtest.h> | ||
|
|
||
| #include <chrono> | ||
| #include <functional> | ||
| #include <future> | ||
| #include <thread> | ||
|
|
||
| #include "ccap_apple_async.h" | ||
|
|
||
| namespace | ||
| { | ||
|
|
||
| // Stand-in for AVCaptureDevice requestAccessForMediaType:completionHandler:: it fires | ||
| // the completion asynchronously from a *background* thread after a short countdown, | ||
| // never touching the caller's main run loop. | ||
| void simulateAsyncPermissionRequest(const std::function<void()>& done) | ||
| { | ||
| std::function<void()> completion = done; // must outlive this call | ||
| std::thread([completion]() { | ||
| std::this_thread::sleep_for(std::chrono::milliseconds(50)); // countdown | ||
| completion(); | ||
| }).detach(); | ||
| } | ||
|
|
||
| // Runs runBlockingAsyncRequest (optionally on a worker thread) and reports whether it | ||
| // returned within the timeout. A timeout means it deadlocked. | ||
| bool completesWithoutDeadlock(bool onWorkerThread, std::chrono::milliseconds timeout) | ||
| { | ||
| std::promise<void> donePromise; | ||
| std::future<void> doneFuture = donePromise.get_future(); | ||
|
|
||
| auto body = [&donePromise]() { | ||
| ccap::runBlockingAsyncRequest(&simulateAsyncPermissionRequest); | ||
| donePromise.set_value(); | ||
| }; | ||
|
|
||
| std::thread worker; | ||
| if (onWorkerThread) { | ||
| worker = std::thread(body); | ||
| } else { | ||
| body(); | ||
| } | ||
|
|
||
| const bool completed = doneFuture.wait_for(timeout) == std::future_status::ready; | ||
| if (worker.joinable()) { | ||
| if (completed) { | ||
| worker.join(); | ||
| } else { | ||
| worker.detach(); // leave the hung thread; the process exits regardless | ||
| } | ||
| } | ||
| return completed; | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| // The regression: open() called off the main thread with no run loop servicing the | ||
| // main queue. This deadlocked with the old dispatch-to-main-queue implementation. | ||
| TEST(AppleCameraPermission, OffMainThreadWithoutRunLoopDoesNotDeadlock) | ||
| { | ||
| EXPECT_TRUE(completesWithoutDeadlock(/*onWorkerThread=*/true, std::chrono::seconds(5))) | ||
| << "runBlockingAsyncRequest() deadlocked off the main thread -- the request was " | ||
| "likely bounced onto an unserviced main dispatch queue."; | ||
| } | ||
|
|
||
| // Sanity: the common main-thread path must also complete promptly. | ||
| TEST(AppleCameraPermission, MainThreadDoesNotDeadlock) | ||
| { | ||
| EXPECT_TRUE(completesWithoutDeadlock(/*onWorkerThread=*/false, std::chrono::seconds(5))) | ||
| << "runBlockingAsyncRequest() deadlocked on the main thread."; | ||
| } | ||
|
|
||
| #endif // __APPLE__ | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.