gh-obsproject/obs-browser

Watch

Fork 0

Do not try to create the browser before init #519

Merged

hoshinolina merged 1 commit from fix-init-race into master

2026-05-26 13:49:14 -05:00

hoshinolina commented

2026-04-14 04:35:24 -05:00

(Migrated from github.com)

Description

Should fix the random asserts on startup (not related to the recent consistent startup failures), I think.

Motivation and Context

Fixes a race.

How Has This Been Tested?

Quick smoke test.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

I have read the contributing document.
My code has been run through clang-format.
My code follows the project's style guidelines
My code is not on the master branch.
My code has been tested.
All commit messages are properly formatted and commits squashed where appropriate.
I have included updates to all appropriate documentation.

### Description Should fix the random asserts on startup (not related to the recent consistent startup failures), I think. ### Motivation and Context Fixes a race. ### How Has This Been Tested? Quick smoke test. ### Types of changes  - Bug fix (non-breaking change which fixes an issue)       ### Checklist:   - [x] I have read the [**contributing** document](https://github.com/obsproject/obs-studio/blob/master/CONTRIBUTING.md). - [x] My code has been run through [clang-format](https://github.com/obsproject/obs-studio/blob/master/.clang-format). - [x] My code follows the project's [**style guidelines**](https://github.com/obsproject/obs-studio/blob/master/CODESTYLE.md) - [x] My code is not on the master branch. - [x] My code has been tested. - [x] All commit messages are properly formatted and commits squashed where appropriate. - [ ] I have included updates to all appropriate documentation.

👍 2 ❤️ 1

notr1ch (Migrated from github.com) approved these changes

2026-04-14 21:30:25 -05:00

PatTheMav commented

2026-04-27 07:47:32 -05:00

(Migrated from github.com)

On macOS BrowserInit always finishes before obs_browser_initialize returns, so I guess we don't face the same issue there?

What's the actual root cause of the race? At first glance the "browser manager thread" seems like a culprit, because it decouples the actual initialisation of CEF from the main thread on Windows and Linux (but not on macOS), so CEF will only be initialised "eventually". On macOS `BrowserInit` always finishes before `obs_browser_initialize` returns, so I guess we don't face the same issue there?

hoshinolina commented

2026-04-27 08:08:00 -05:00

(Migrated from github.com)

Yes, it's that. In particular, the graphics thread will call tick on the source once it's added to the scene, and if CEF init isn't complete yet by then bad things happen.

👍 1

PatTheMav (Migrated from github.com) reviewed

2026-04-27 12:01:19 -05:00

PatTheMav (Migrated from github.com) left a comment

Annoying that it's necessary, but seems like the most elegant fix for this race condition.

Please amend the commit description per our contribution guidelines to explain the "why" of the fix (explaining the race condition, it's root cause, and why this fixes it).

Annoying that it's necessary, but seems like the most elegant fix for this race condition. Please amend the commit description per our contribution guidelines to explain the "why" of the fix (explaining the race condition, it's root cause, and why this fixes it).

hoshinolina commented

2026-04-28 03:46:23 -05:00

(Migrated from github.com)

Done.

👍 1

PatTheMav (Migrated from github.com) reviewed

2026-04-30 11:58:59 -05:00

obs-browser-source.cpp

	if (os_event_try(cef_started_event) != 0) {
		return;
	}

Sorry, should've picked up on this in my earlier preview. Per our code style guidelines, except for C code, we prefer all conditional and loop blocks to be explicitly enclosed with curlies.

```suggestion if (os_event_try(cef_started_event) != 0) { return; } ``` Sorry, should've picked up on this in my earlier preview. Per our code style guidelines, except for C code, we prefer all conditional and loop blocks to be explicitly enclosed with curlies.

hoshinolina (Migrated from github.com) reviewed

2026-05-03 04:33:17 -05:00

obs-browser-source.cpp

I mean... sure but... nothing else in the file does this, including the rest of the function.

I think if this is desired, someone should add InsertBraces: true to .clang-format and fix the whole codebase. Enforcing things like this in review is essentially a waste of everyone's time when clang-format can do it for you.

I mean... sure but... nothing else in the file does this, including the rest of the function. I think if this is desired, someone should add `InsertBraces: true` to .clang-format and fix the whole codebase. Enforcing things like this in review is essentially a waste of everyone's time when clang-format can do it for you.

RytoEX (Migrated from github.com) reviewed

2026-05-06 13:08:41 -05:00

obs-browser-source.cpp

Historically, we have avoided this because it would be disruptive to all open PRs. However, we have decided that there is no good time for this, so we will be enforcing braces via clang-format in the near future.