Skip to content

Queue navigation in browser dock to fix deadlock - #14

Merged
zackees merged 1 commit into
obs-rust:masterfrom
rdoupe:fix/queue-dock-navigation
Oct 26, 2026
Merged

zackees merged 1 commit into
obs-rust:masterfrom
rdoupe:fix/queue-dock-navigation

Conversation

@rdoupe

@rdoupe rdoupe commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Refs obs-rust/obs-studio#2527

Port of upstream obsproject/obs-browser#556 (open upstream, no review decision yet), for obsproject/obs-studio#14082. The upstream author is credited with a Co-Authored-By line.

What and why

On macOS, with a YouTube account connected, OBS can freeze for good, often seconds after launch. The upstream report has a symbolicated sample of the frozen process, and the code here is the same:

  • QCefBrowserClient::OnBeforeBrowse holds the global, non-recursive popup_whitelist_mutex while it calls QMetaObject::invokeMethod(widget, "urlChanged", ...) with the default connection type.
  • On macOS, CEF runs on Qt's main loop (ENABLE_BROWSER_QT_LOOP), so that call is direct, and every urlChanged handler runs inside the navigation event with the lock held.
  • The YouTube Live Control Panel dock's handler (YouTubeAppDock::ReloadChatDock) reloads the YouTube chat dock. That navigation runs OnBeforeBrowse again on the same thread, which waits on the same lock, and OBS freezes.
  • The fix passes Qt::QueuedConnection, so the handlers run from Qt's main event loop after OnBeforeBrowse has returned and released the lock.
  • Linux and Windows: CEF runs on its own thread there, so the call was already queued, and nothing changes.

Upstream's reviewer also suggests queueing YoutubeAuth::ReloadChat in the frontend. That would be a separate change and is not part of this port.

RED → GREEN

  • RED: none. No test is feasible here:
    • The deadlock needs CEF on Qt's main loop (macOS only) and a connected YouTube account. That means a Google login, which local and CI builds can't do.
    • On Linux and Windows the call already crosses threads, so it is queued before and after the change, and a test there would pass either way.
    • A Qt-only test would only show that Qt::QueuedConnection defers a slot. It would not touch the lock or CEF.
    • Upstream verified the fix by logging into YouTube on macOS.
  • GREEN: fdd82cc. It builds in a CEF 8078 Linux build of obs-rust. In a smoke run of that build, two custom browser docks load and render their pages, and nothing crashes.
  • Real app: not run on macOS (no macOS machine here).

Notes

  • clang-format 22.1.3 clean.

QCefBrowserClient::OnBeforeBrowse holds popup_whitelist_mutex while it
calls the widget's urlChanged signal through QMetaObject::invokeMethod
with the default connection. With CEF running on Qt's main loop
(macOS), that call is direct, so every handler of urlChanged runs
inside the browser's navigation event with the lock held. When the
YouTube app dock navigates, the frontend reloads the YouTube chat dock;
that navigation runs OnBeforeBrowse again, which waits on the same
lock, and OBS freezes.

Queue the call, so the handlers run from Qt's main event loop after
the navigation event has returned and released the lock. On Linux and
Windows, CEF runs on its own thread and the call was already queued;
nothing changes there.

Ported from upstream obs-browser PR 556 (open upstream at the time of
porting).

Refs obs-rust/obs-studio#2527

Co-Authored-By: Matt Gajownik <matt@obsproject.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.

Reviewed: queuing urlChanged releases popup_whitelist_mutex before the handlers run. On Linux and Windows the call was already cross-thread, so this is a no-op there. The reason there's no RED test is reasonable and documented. Merging.

@zackees
zackees merged commit 434cc38 into obs-rust:master Oct 26, 2026
1 check passed
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