Fix four host faults the synthetic tests could never see - #27
Merged
Conversation
From the code-review pass over the merged revamp. Each is invisible to CI because the synthetic capture backend starts instantly, never changes resolution, and never touches the virtual display. The extended display could never come up. spawn_generation re-armed the running flag but left the watchdog heartbeat atomics holding the previous generation's timestamps, and the wedge check has no warm-up. Attaching the virtual display polls for up to 10 seconds before the capture loop stamps its first heartbeat, so at 3 seconds the watchdog judged the new generation by the old one's clock, killed it, and did the same to the next five until the storm brake parked the supervisor in Failed. The atomics are now stamped when a generation is spawned. A superseded capture thread kept running. Stage threads are detached, so one blocked in the virtual-display poll or the ACCESS_LOST retry ladder outlives its generation; it then re-read the process-wide running flag, found it true again for the NEXT generation, and carried on. Two capture loops then fight over a DXGI duplication only one can hold, and the zombie's heartbeats mask the watchdog that would have caught it. Stage loops now stop when their own generation is superseded, which is what CURRENT_GENERATION was there for. Lowering the bitrate slider did nothing. set_ceiling clamps the current rung and reports that it changed, but the caller discarded the decision and used only on_report's, which returns unchanged for a clean report. So the label moved and the encoder did not, for the life of the pipeline. Both decisions now count. A typed-in address counted as a viewer. client_connected only checked target_addr, which the GUI's Target IP field and the persisted target_ip both wrote — so with nobody connected the capture loop skipped its idle throttle and ran flat out, and with Extended display selected the virtual monitor came up for an audience of none. In v2 the session is the only truth, so client_connected asks it. The Target IP control is gone with it: media carries a session id from the handshake, so an address typed there could never have received a stream.
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
1 participant
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.
More findings from the
/code-reviewpass, each confirmed by reading the tree. They share a root cause for why CI is green: the synthetic capture backend starts instantly, never changes resolution, and never touches the virtual display.The extended display could never come up.
spawn_generationre-armed the running flag but left the watchdog heartbeat atomics holding the previous generation's timestamps, and the wedge check has no warm-up. Attaching the virtual display polls for up to 10s before the capture loop stamps its first heartbeat, so at 3s the watchdog judged the new generation by the old one's clock, killed it, and repeated until the storm brake parked the supervisor inFailed. The atomics are now stamped at spawn.A superseded capture thread kept running. Stage threads are detached, so one blocked in the VDD poll or the ACCESS_LOST ladder outlives its generation — then re-reads the process-wide running flag, finds it true again for the next generation, and carries on. Two capture loops then fight over a DXGI duplication only one can hold, and the zombie's heartbeats mask the watchdog that would have caught it. Stage loops now stop when their own generation is superseded, which is what
CURRENT_GENERATIONexisted for.Lowering the bitrate slider did nothing.
set_ceilingclamps the rung and reportschanged: true, but the caller discarded that decision and used onlyon_report's — which returns unchanged for a clean report. The label moved; the encoder didn't, for the life of the pipeline. Both decisions now count.A typed-in address counted as a viewer.
client_connectedonly checkedtarget_addr, which the GUI's Target IP field and the persistedtarget_ipboth wrote. With nobody connected the capture loop skipped its idle throttle and ran flat out, and with Extended display selected the virtual monitor came up for an audience of none. In v2 the session is the only truth, soclient_connectedasks it — pinned by a new test. The Target IP control goes with it: media carries a session id from the handshake, so an address typed there could never have received a stream.Gates
104 Rust tests (1 new), zero clippy warnings, E2E green including the crash-restart case that exercises a generation transition.