Skip to content

Slice 3c Review

Retrospective review of slice 3c per docs/13 and docs/20, run before the slice was committed. The slice completes the introspection endpoint (/events, /stats, /memory, /log, the diagnostics bundle, fjarr-lab introspect, make introspect) and turns the docs/23 memory ladder into gates: the leaks-tracer bracketing, the soak, valgrind, heaptrack, the netem scenarios, a nightly workflow prepared for a self-hosted GPU runner. One reviewer pass on the C++; the two harness subagents (opsim soak/netem, lab tests) reported what the tooling found.

Each row is a defect the ladder or the lab surfaced within hours of existing; each is fixed and the tool that found it now guards it.

Found byDefectFix
lab /memory test, then a refcount tracewebrtcbin 1.28.2 leaks one GstRtpSession reference per get-stats call (_get_data_channel_transport_stats takes rtpbin’s get-session and never releases it): an 18 s session left 18 references and ~18 MB behind; the robot went from 44 to 300 MB over an afternoonthe stats sampler reads rtpbin’s per-source counters synchronously on the loop; get-stats is not used (docs/23); reported upstream
lab /memory test, opsim soakcensus.sources grew by one per session for ever: the RAII kit only left the census at cancel(), so a one-shot timer that fired on its own (the deferred snapshot) stayed counted while its guard sat in a mapthe trampoline decrements when a source returns REMOVE; cancel() only if it has not
opsim soakclosed sessions’ snapshot rings never expired: the deferred “closing” snapshot landed after retire() and un-retired the ring (76 closed sessions listed, the oldest 20 min old)a retired ring stays retired; retire() takes the pending snapshot immediately
heaptrackone heap allocation per delivered frame in FrameHub::deliver (the target list), against the docs/16 budgetthe list is a member reused across frames; hub keys interned so Work copies no strings
opsim soak (200 cycles)+71 MB with the census flat: snapshot retention was bounded per ring but not in total (200 closed sessions inside the 10-minute window), each snapshot held a parsed JSON tree several times its text plus a 50 KB DOT, and the producer history ramps to 64 entriessnapshots stored as compact text, DOT kept for a ring’s 8 most recent entries, at most 8 closed rings, malloc_trim at session end, and the soak’s baseline taken after a 40-cycle warm-up while the bounded caches fill (docs/24, docs/23): 200 cycles now end 1.5 MiB below the baseline
lab /memory testthe census went negative by the number of stats samples: the new stats read wrapped two transfer-full element references without the kit’s adopting helperglib::adopt_element — and the RAII gate now has a case it did not catch: a kit pointer constructed from a raw pointer (worth a grep in 3c’s follow-up)
lab /log testsix consecutive sessions logged under the same 8-character id: a UUIDv7’s first 8 hex digits are a coarse timestampsid8 is the id’s last 8 characters everywhere (logs, element names, opsim, the lab, docs/24)
valgrind146 reports from 4 contexts, all GLib/GIO/duktape-internalagent/tests/valgrind/fjarr.supp, each entry named and library-only
opsim netem lossy/badno NACK/RTX/FEC negotiated and a fixed 4 Mbit active tier: at 5 % loss a keyframe rarely survives, under 1.5 Mbit the queue floodsdeferred to slice 6 (adaptive bitrate, loss recovery); the scenarios assert transport-level arrival and record decode health; docs/25’s lossy/bad decode expectations wait for it
#SeverityFindingResolution
1highthe memory census read the tracer’s get-live-objects, whose result takes every live object’s reference (it exists for process exit): a /memory?since= on a tracer-enabled robot would have freed live elements under a running pipelinenever called; “created and still alive” is created − removed as multisets in a process-wide ledger merged on every read, so reads are idempotent and any number of readers agree
2high/events left libsoup’s body accumulation on: every frame ever written stayed in memory for the life of the connectionaccumulation off; docs/24 says so
3highexceptions from a provider, the tar writer or a malformed JSON field could unwind through libsoup’s C handler into the core loopthe handler catches std::exception and answers 500
4mediumsince() reset the tracer window, so a second read (the soak’s producer wait, the debug print) lied; several MemoryCensus instances re-started trackingthe ledger above; the test runner reads once per test
5mediumFrameHub::targets_ kept the last delivery’s sinks (and their appsrc refs) alive until the next frame on any keycleared after delivery
6mediuma slow /events reader was “ended” but neither dropped nor stopped receiving appends; a libsoup-initiated disconnect never fired finishedsocket closed, a closing flag, disconnected handled
—lowLast-Event-ID replayed one ring only; per-frame HubKey string copies beyond SSO; gzip at level 6 on the loop; the leaks listener ran after gtest’s printer; docs drift (fjarr-agent --memcheck, gst.supp, the bundle’s members, the checkpoint token)all fixed: replay across rings by timestamp, interned keys, level 1, listener before the printer, docs corrected

Checked and found fine: the endpoint and every provider run on the core loop; SseClient lifetime against libsoup 3.6.6’s finished/disconnected semantics; SSE framing per docs/24; the ustar header (checksum, octal fields, prefix split, padding, end marker) and the GZlib loop; the diagnostics fetch from a loop-less daemon; the log ring’s bounds and thread safety; the tracer signal names and structure fields on 1.28.2; fjarr.supp patterns never match a fjarr frame; the nightly workflow against docs/12 and docs/15.

  • docs/24 acceptance minus the viewer, through curl, fjarr-lab introspect and the lab tests: /pipelines, /stats, /memory checkpoint diff, /log, /events (a select-tracks shows as a snapshot event with the valve in its body), /diagnostics.tar.gz parsed member by member.
  • docs/15 memory rows: ASan, TSan, the leaks bracketing (32 tests, and the deliberate-leak self-check fails as it must), valgrind memcheck clean, heaptrack with no fjarr allocator on the frame path.
  • The soak: 20 cycles per commit and 200 at the gate — census identical to the post-warm-up checkpoint (sources unchanged), RSS −1.5 MiB over the 160 cycles after the 40-cycle warm-up against the 5 MiB budget (before the fixes above: +19 sources, +71 MB per 200 cycles and, over an afternoon of mixed scenarios, +250 MB).
  • Every earlier gate still green: 8 opsim scenarios twice on a quiet robot, the full lab suite, netem lossy and bad.
  • Loss recovery (NACK/RTX, FEC) and adaptive bitrate: slice 6; until then lossy asserts arrival, not decode, and bad is out of the nightly (it fails on its control channel at 15 % loss without them).
  • The toggle-silent check saw 7–11 frames decode up to 800 ms after a disable in 2 of 3 runs while netem, the soak and the lab shared one robot; it did not reproduce on a quiet robot (twice). Watch it in CI; if it returns, the suspect is send-side buffering after the valve.
  • What looked like the machine was a defect the machine exposed. After a day of builds and sanitizer runs the host’s package power limit had throttled the iGPU to its 800 MHz floor; VA-API sessions then streamed 10–15 fps (identically on the committed 3b agent) while the same encode chain without the agent did 29 and the software path did 34. With the clock excluded (intel_gpu_top: video engine idle, clock ramping) the remaining difference was the frame-stamp painter write-mapping VA-backed buffers that vapostproc had proposed upstream — a GPU round trip per frame that a fast clock hides. Fixed in the producer (system memory for stamped sources, docs/23); VA-API sessions are back at 28 fps at the floor clock and fjarr-opsim’s wire-level hot-plug continuity holds. The sim’s unthrottled glxgears was removed and docs/12 keeps the throttle diagnosis. The lab’s hot-plug test now budgets Chromium’s presentation and asserts the wire exactly; on this host Chromium’s software decode of two VA-API-encoded streams still presents with gaps (9–22 frames) that the software-encoded pair does not (3–4), so that presentation number is the browser’s, and CI’s runner is its gate.
  • The nightly’s VA-API measurement waits for the self-hosted runner (docs/12).
  • Upstream: the webrtcbin stats leak; the workaround stays until the baseline carries a fix.