Skip to content

Check os_event_init in obs_module_load and ExecuteOnBrowser - #5

Merged
zackees merged 1 commit into
masterfrom
fix/4-os-event-init
Oct 25, 2026
Merged

zackees merged 1 commit into
masterfrom
fix/4-os-event-init

Conversation

@zackees

@zackees zackees commented Oct 25, 2026

Copy link
Copy Markdown
Member

Fixes #4

  • obs_module_load(): if either startup event fails to init, log, destroy the one made (os_event_destroy is NULL-safe), reset both pointers and return false. libobs does not call obs_module_unload() for a module whose load failed, so the unload path never waits on a NULL event.
  • BrowserSource::ExecuteOnBrowser() (returns void): if finishedEvent fails to init, log and return without queueing the task, so nothing signals or waits on an uninitialized pointer.

Successful paths are unchanged.

RED evidence (reasoning, no harness): the existing tests/ harnesses need a full CEF build and none can inject an os_event_init() failure without stubbing libobs. Before this change, a failed init under obs-rust/obs-studio#2163 leaves the pointer NULL (module load) or uninitialized stack garbage (ExecuteOnBrowser), and the next os_event_signal/os_event_wait dereferences it. clang-format 22.1.3 clean.

Since obs-rust/obs-studio#2163 a failed os_event_init() leaves the event
NULL. obs_module_load() now fails cleanly if either startup event cannot
be created, and ExecuteOnBrowser() returns without queueing a task that
would signal a missing event.

Fixes #4
@zackees

zackees commented Oct 25, 2026

Copy link
Copy Markdown
Member Author

CI is green (clang-format, the only check). obs-browser has no merge queue: ready for review and merge by the coordinator.

@zackees zackees left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Coordinator review: LGTM.

  • obs_module_load(): only qRegisterMetaType runs before the event init, so returning false leaks nothing; os_event_destroy is NULL-safe and both globals are reset, so a retry or the cef_started_event NULL check at line 129 stays safe.
  • ExecuteOnBrowser(): returning before QueueCEFTask means no signal/wait on an uninitialized pointer; success path unchanged.
  • RED: no harness in this repo can inject an os_event_init failure without a CEF build; the reasoning-based RED in the body is acceptable under the review-checkable criterion. clang-format green.

Verdict: approve; merging (repo has no merge queue).

@zackees
zackees merged commit 7c92b29 into master Oct 25, 2026
1 check passed
chriscoveries pushed a commit to chriscoveries/obsrust-fork that referenced this pull request Oct 25, 2026
…s) (obs-rust#2378)

Moves plugins/obs-browser from 7bab463c9 to 4583599fe, with:

- obs-rust/obs-browser#3: the CEF sandbox where it can work on Linux
  (obs-rust#1092)
- obs-rust/obs-browser#5: os_event_init results checked in module load
  and ExecuteOnBrowser
- obs-rust/obs-browser#6: WebAudio resumes after the audio service dies
  (obs-rust#2328)
- obs-rust/obs-browser#7: browser audio that arrives too late to mix is
  dropped (obs-rust#2335)
- obs-rust/obs-browser#8: the fallback warning when the sandbox can't
  work (obs-rust#2218)

One bump instead of three (obs-rust#2333, obs-rust#2351 and ours), as agreed on obs-rust#2351.
docs/features.md gets the two audio lines from obs-rust#2333 and obs-rust#2351 as they
are, a line for the sandbox, and the browser on the fallback warning
line, which obs-rust#2238 merged on 2026-10-25.

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.

Check os_event_init in obs_module_load and ExecuteOnBrowser

1 participant