Repository navigation
fix: recording shutdown waits for inactive after deactivate - #1392
Merged
Merged
Conversation
zackees
marked this pull request as ready for review
October 18, 2026 04:47
This was referenced Oct 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #1391, #1283; follows merged #1342.
Problem
WaitForOutputToStop()treated thedeactivatesignal 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
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.6a5f8b68e: both new cases pass. The unchangedtest_recording_shutdown_wait.cpppasses all four cases. Both executables pass against each existing local C and Rust-linked libobs variant: 12 cases total.tests.dCMake 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.git diff --checkpass.--repeat until-fail:100passes 100 consecutive invocations against each local core (200 executions / 400 cases, 15.05 s and 17.81 s).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.