Fix three iPad crashes and two silent freezes - #26
Merged
Conversation
All found by the code-review pass over the merged revamp and confirmed against the tree. Crash on every host restart. The 4 Hz stats flush computed fragment deltas with wrapping subtraction, but the assembler zeroes its counters whenever it resets — which is exactly what a stream-epoch bump does. Two ~2^64 deltas were then added with a trapping +, so the app died the moment the host restarted its pipeline: supervisor auto-restart, VDD toggle, settings change, encoder crash recovery. The client killed itself on the very path M6 built to survive. It now re-baselines when a counter goes backwards. Crash on a fast swipe. TouchRelay squared Int32 deltas of coordinates spanning 0...65535; a flick across ~71% of the width overflows and Swift traps. Widened to Int64. Crash on connect and disconnect. FrameSlot.onFrameStored was a plain stored closure, written on the MainActor by the Metal view's attach and detach while the VideoToolbox decode thread called it — a race on the closure's own retain/release. It is now swapped under a lock and copied out before being called. Auto-reconnect never fired. The flush loop skipped the rest of the tick when no datagrams had arrived, and the only call to the liveness watchdog sat below that guard. A dead host sends neither media nor heartbeats, so the watchdog was silenced in precisely the situation it exists for: the bar kept reading ON AIR over a frozen frame and the reconnect cycle from #17 could never start. The watchdog now runs every tick. Large frames were dropped forever. The assembler capped frames at 1024 fragments while the host fragments against, and the wire parser accepts, 3066. A 1440p or 4K scene-change IDR lands in that gap: every fragment discarded, no sync sample, so every later frame discarded too, and the client asks for a keyframe that comes back equally large. Permanent freeze with healthy heartbeats on both ends. The cap now IS the protocol cap; maxPendingBytes remains the real memory guard. Also: the three-finger HUD gesture could only ever show the HUD, because it called scheduleHUDDismiss() which unconditionally sets showHUD = true; both tap paths now share one toggle. And Info.plist still declared Syne-Variable.ttf, deleted in #22.
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.
Findings from the
/code-reviewpass over the merged revamp, each confirmed by reading the tree.Crashes
On every host restart. The 4 Hz stats flush computed fragment deltas with wrapping subtraction (
&-), but the assembler zeroes its counters whenever it resets — which is what a stream-epoch bump does. Two ~2^64 deltas were then combined with a trapping+, so the app died the moment the host restarted its pipeline (supervisor auto-restart, VDD toggle, settings change, encoder crash recovery). The client killed itself on the exact path #18 built to survive. Now re-baselines when a counter goes backwards.On a fast swipe.
TouchRelaysquaredInt32deltas of coordinates spanning 0…65535. A flick across ~71% of the content width overflows, and Swift traps rather than wrapping. Widened toInt64.On connect and disconnect.
FrameSlot.onFrameStoredwas a plain stored closure, written on the MainActor by the Metal view's attach/detach while the VideoToolbox decode thread called it — a race on the closure's own retain/release. Now swapped under a lock and copied out before invocation.Silent freezes
Auto-reconnect never fired. The flush loop
continued when no datagrams had arrived, and the only call to the liveness watchdog sat below that guard. A dead host sends neither media nor heartbeats, so the watchdog was silenced in precisely the case it exists for: the status bar kept reading ON AIR over a frozen frame, and the reconnect cycle from #17 could never start. It now runs every tick.Large frames were dropped forever. The assembler capped frames at 1024 fragments while the host fragments against — and the wire parser accepts — 3066. A 1440p/4K scene-change IDR lands in that gap: every fragment discarded, so no sync sample, so every later frame discarded too, and the keyframe the client requests comes back equally large. Permanent freeze with healthy heartbeats on both ends. The cap is now the protocol cap;
maxPendingBytesremains the real memory guard.Smaller
The three-finger HUD gesture could only ever show the HUD (it called
scheduleHUDDismiss(), which unconditionally setsshowHUD = true); both tap paths now share one toggle.Info.pliststill declaredSyne-Variable.ttf, deleted in #22.Gates
67 iOS tests (2 new: protocol-cap frame assembly, full-screen flick), simulator E2E green.