Skip to content

fix: recording shutdown waits for inactive after deactivate - #1392

Merged
zackees merged 2 commits into
masterfrom
fix/recording-wait-inactive-ship
Oct 18, 2026
Merged

zackees merged 2 commits into
masterfrom
fix/recording-wait-inactive-ship

Conversation

@zackees

@zackees zackees commented Oct 18, 2026 •

Copy link
Copy Markdown
Member

Refs #1391, #1283; follows merged #1342.

Problem

WaitForOutputToStop() treated the deactivate signal as proof that the output was inactive. libobs emits that signal immediately before storing inactive, so the UI thread could return success during that interval. This matches the first unchanged recording-wait assertion failure seen in #1333 CI run 39095486998; this does not establish the cause of every subsequent failure.

Fix

Require active() to be false before reporting success. After deactivate, coalesced 1 ms Qt timer callbacks recheck the state within the existing timeout. Checks run on the UI thread, and the receiver lifetime cancels pending callbacks. Existing stop ordering, timeout and user-input exclusion remain intact.

RED and GREEN

  • RED c23757e03: new real QtCore/libobs-signal-handler regression fails both cases: deactivate before a later inactive store returns while active; deactivate without an inactive store reports success instead of timing out. Synchronization uses a condition variable and queued events, without sleeps. Assertions run after thread joins and handler cleanup.
  • GREEN 6a5f8b68e: both new cases pass. The unchanged test_recording_shutdown_wait.cpp passes all four cases. Both executables pass against each existing local C and Rust-linked libobs variant: 12 cases total.
  • The actual new and existing tests.d CMake fragments configure, compile and pass CTest in a focused wrapper importing the real local libobs, Qt 6.11.1 and cmocka 2.0.1. This validates their registration, not a full root CMake or GUI build.
  • clang-format 22.1.3, gersemi 0.25.0 and git diff --check pass.
  • New regression repeatability: CTest --repeat until-fail:100 passes 100 consecutive invocations against each local core (200 executions / 400 cases, 15.05 s and 17.81 s).
  • Read-only scoped review: no blocking findings (clud-review: clean, one existing reviewer reused).

No existing C/C++ tests or public headers were changed. The production fix is in the private frontend helper. This is a C++ correctness fix, not a Rust port; Rust unit/oracle tiers do not apply. A real app quit-while-recording run has not been performed for this standalone fix; PR CI remains the merge gate.

@zackees
zackees marked this pull request as ready for review October 18, 2026 04:47
@zackees
zackees added this pull request to the merge queue Oct 18, 2026
Merged via the queue into master with commit 84aac88 Oct 18, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant