Skip to content

Do not hand over to the Qt main thread once its loop has stopped - #11

Merged
zackees merged 2 commits into
obs-rust:masterfrom
vpacher:fix/2391-qt-shutdown-handover
Oct 25, 2026
Merged

zackees merged 2 commits into
obs-rust:masterfrom
vpacher:fix/2391-qt-shutdown-handover

Conversation

@vpacher

@vpacher vpacher commented Oct 25, 2026

Copy link
Copy Markdown
Member

Refs obs-rust/obs-studio#2391

What and why

From the review of #10: obs_browser_initialize() hands BrowserInit() to the Qt main thread with a blocking queued call when it is called from another thread (macOS). Once Qt's main loop has finished, nothing runs that call, so a first browser source created off the main thread while OBS quits would hang it on quit.

QCoreApplication::closingDown() alone does not cover it: it is only true while the application object is destroyed, not between the end of exec() and that. So:

  • The hand-over moves into RunOnQtMainThread() (browser-qt-main-thread.hpp, no CEF).
  • WatchQtMainLoop(), called from obs_module_load() on the main thread, sets a flag on QCoreApplication::aboutToQuit.
  • RunOnQtMainThread() does not hand over when that flag or closingDown() is set. It queues the call without blocking and waits for it in 50 ms steps; if the loop stops before the call has started, it cancels it (a queued call that runs later does nothing) and returns false. obs_browser_initialize() then leaves CEF uninitialized, as for a failed initialization.

On the main thread and without a Qt application it still calls BrowserInit() directly. Linux and Windows (their own CEF thread) are unchanged.

RED → GREEN

tests/qt_main_thread.py compiles tests/qt-main-thread.cpp against Qt Core only (no CEF, no display; --qt-lib-dir on macOS, pkg-config Qt6Core on Linux):

  • RED 643f00f (the hand-over moved unchanged into the helper): while the loop runs, a worker thread's call runs on the main thread (passes); after the loop has finished, the call is still waiting after 2 s (fails).
  • GREEN c01896a: both pass, 20 of 20 runs.
  • tests/first_source_off_main_thread.py on macOS 26.7.1 with this branch: websocket-first and collection both render.

AI note: Claude Code (Claude Opus 5.5) wrote the change, the tests and this text under the submitter's direction, and ran the tests above on the submitter's machine.

vpacher and others added 2 commits October 25, 2026 13:22
obs_browser_initialize() hands BrowserInit() to the Qt main thread with a
blocking queued call when it is called from another thread (obs-rust#10). Once
Qt's main loop has finished, nothing runs that call, so a first browser
source created off the main thread while OBS quits would hang it
(obs-rust/obs-studio#2391, from the review of obs-rust#10).

The hand-over moves unchanged into RunOnQtMainThread()
(browser-qt-main-thread.hpp), with WatchQtMainLoop() called from
obs_module_load() (a no-op for now). qt-main-thread.cpp, run by
qt_main_thread.py against Qt Core only: while the loop runs, a worker
thread's call runs on the main thread (passes); after the loop has
finished, the call must return within 2 s without running (fails: it
waits forever).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RunOnQtMainThread() waited on a blocking queued call; once Qt's main
loop has finished (OBS quits), nothing runs it, so a first browser
source created off the main thread then hung OBS on quit
(obs-rust/obs-studio#2391, from the review of obs-rust#10).

WatchQtMainLoop(), called from obs_module_load() on the main thread, sets
a flag on QCoreApplication::aboutToQuit. RunOnQtMainThread() no longer
hands over when that flag or QCoreApplication::closingDown() is set: it
queues the call without blocking and waits for it in 50 ms steps, and if
the loop stops before the call has started, it cancels the call (a
queued call that runs later does nothing) and returns false. The caller
then leaves CEF uninitialized.

tests/qt_main_thread.py: both cases pass, 20 of 20 runs.
tests/first_source_off_main_thread.py on macOS: both scenarios render.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@zackees zackees left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Coordinator review: approved, and thanks for picking up the note from #10. You're right that closingDown() alone isn't enough: it's false between the end of exec() and the application's destruction, so aboutToQuit is the right signal.

  • The handshake is sound. State changes all happen under handover->mutex: the caller cancels only while the call is still Queued, and the main-thread lambda checks for Cancelled before it runs. So func runs at most once, and the caller never returns while it's running. A call that runs after cancellation is a no-op.
  • WatchQtMainLoop() runs at module load on the main thread, while the app exists. obs_browser_initialize() treats a refused hand-over as a failed initialization, which the callers already handle.
  • The Qt Core-only test covers both cases (the loop running, and the loop finished).
    Note: the 50 ms poll doesn't change the other case from #10. If the main thread were blocked waiting on the caller, this still waits (now polling rather than deadlocked). No current path does that.

@zackees
zackees merged commit 68f1526 into obs-rust:master Oct 25, 2026
1 check passed
@vpacher
vpacher deleted the fix/2391-qt-shutdown-handover branch October 25, 2026 12:50
MCPVOT pushed a commit to MCPVOT/obs-studio that referenced this pull request Oct 25, 2026
…rust#2402)

obs-rust/obs-browser#11: on macOS, do not hand BrowserInit() to the Qt
main thread once its loop has stopped, so a first browser source created
off the main thread while OBS quits no longer hangs it (obs-rust#2391).

obs-rust/obs-browser#12: browser sources and panels use the CEF sandbox
on macOS, with libcef_sandbox.dylib from the CEF framework; without it,
the helpers run unsandboxed and OBS shows the fallback warning (obs-rust#1092).

docs/features.md: the sandbox line covers Linux and macOS, the obs-rust#2387
line names the quit case.

Fixes obs-rust#2391
Refs obs-rust#1092

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants