From 761615c2fe5b45434b10e7ef36bf80666cdeff8c Mon Sep 17 00:00:00 2001 From: Martino Ferrari Date: Wed, 14 Oct 2026 15:38:00 +0200 Subject: [PATCH] fix: a restarted client is a new stream, so its history and its monotonic mark go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Going live from a file view left the display frozen for seconds: the per-second line showed `ring=[2.5200..2.5200] n=567000 -> n=1131000 -> n=1689000` — the sample count climbing at the stream's rate while the retained span was one value, the file view's last stamp. The status bar was green because everything about the *connection* was fine. It is the poisoning `invariants-data-path.md` already describes, entered through a different door. The new client's stream clock sits *behind* the stamps the old stream left in `Acquisition::last_stamp_` (the file view's samples are still in the store), so the advance test rejects every packet of a perfectly good stream, four rejections abandon the declared time signal, and the arrival-clock fallback derives its epoch from the surviving mark — every sample stamped flat at the old value. The guard did exactly what it was built to do; the *stream* was the thing that had changed and nothing had told it. So `Acquisition::start()` — which is the one place a client (re)starts — now says so: `ScopeModel::reset_stream()` drops the previous stream's history (`store_->reset()`, which the data-path invariants require to leave nothing behind) and moves `plan_generation_`, which is what the producer already watches to drop the state tied to the old store; and `last_stamp_` is re-based to `-infinity` so the monotonic guard re-bases on the new clock instead of pinning it. The declaration is kept: the first packet of the new stream passes, and monotonicity holds from there. One self-inflicted crash on the way, fixed before this: `reset_stream()` dereferenced `store_`, which is **null until the first CONFIG has planned anything** — at start-up there is none — so the app died with SIGSEGV and no output at all. Caught by running it plainly rather than trusting the change. Verified with the events probe (a replay and a group, then *Live*): live restored (after replay): … pending=0 trig engine: … history=0.4377/0.4000 s now_t=0.298 ring=[-0.1392..0.2985] n=569000 trig engine: … history=0.8723/0.4000 s now_t=0.733 ring=[-0.1392..0.7331] n=1134000 (pending=1: it latched) trig engine: … history=1.2954/0.4000 s now_t=1.156 ring=[-0.1392..1.1562] n=1684000 `ring` moves within a round-trip, `history` fills, and the trigger latches the frame after the restore; `make test-ui` 14 files, zero `Gtk-CRITICAL`, and the plain run is clean. Docs: the restart's own door into the poisoning in `invariants-data-path.md`, and the `ring=` reading's entry in `probe-readings.md` now says what it must do after *Go live*. --- docs/claude/invariants-data-path.md | 12 ++++++++++++ docs/claude/probe-readings.md | 6 +++++- src/acquisition.cpp | 8 ++++++++ src/model.hpp | 14 ++++++++++++++ 4 files changed, 39 insertions(+), 1 deletion(-) diff --git a/docs/claude/invariants-data-path.md b/docs/claude/invariants-data-path.md index 9336929..0da7431 100644 --- a/docs/claude/invariants-data-path.md +++ b/docs/claude/invariants-data-path.md @@ -94,6 +94,18 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear comparison is off by 1e9, so every packet would read as an implausible jump. The guard is `tools/mock_udpstreamer.py --time-glitch-at`/`--time-glitch-by` (see [verification.md](verification.md)). +- **A *restarted* client is the other way into the same poisoning, and it is not a glitch.** Going live + from a file view starts a new `udps_client`, and the new stream's stamps can sit **behind** the ones the + old stream left in `last_stamp_` — the file view's samples are still in the store. The advance test then + rejects every packet of a perfectly good stream, four rejections abandon the declared time signal, and + the arrival clock derives its epoch from the stale mark: every sample is stamped **flat at the old + value**. `trig engine: … ring=[2.5200..2.5200] n=567000 → n=1131000 → n=1689000` is what that looks + like — the count climbing at the stream's rate while the retained span is one value — until some later + rebuild resets the store, which is where the "few seconds of nothing" came from. + `Acquisition::start()` therefore calls `ScopeModel::reset_stream()` and re-bases `last_stamp_` to + `-infinity`: a started or restarted client is a *new stream*, so the previous one's history goes (which + is what `reset()` is for) and the monotonic guard re-bases on the new clock instead of pinning it. + `ring` moving within a stream round-trip of *Go live* is the assertion. - **The arrival-clock fallback does not drop history.** `arrival_epoch_` is derived from `last_stamp_`, so the arrival clock *continues* from the monotonic mark and the two timelines share a zero by construction — the recorded stamps are therefore still on one diff --git a/docs/claude/probe-readings.md b/docs/claude/probe-readings.md index 2da6d81..da1c39a 100644 --- a/docs/claude/probe-readings.md +++ b/docs/claude/probe-readings.md @@ -1500,7 +1500,11 @@ which of two failures a stalled display is.** `n` counts every stored sample, so rate whenever data arrives at all: - **`ring` standing still while `n` climbs** is the *clock*: samples are stored but every one carries the - same stamp, so the axis cannot advance and the trigger's `history` never fills. Measured after going live + same stamp, so the axis cannot advance and the trigger's `history` never fills. The stamps are pinned at + the file view's last value because a restarted client's stream clock sits behind it: the advance test + rejects the new packets, four rejections abandon the declared time signal, and the arrival-clock fallback + derives its epoch from the surviving mark. `Acquisition::start()` now resets the store and re-bases that + mark, so `ring` must **move** within a stream round-trip of *Go live*. Measured after going live from an event view: `ring=[2.5200..2.5200] n=567000 → n=1131000 → n=1689000`, one second apart, with `now_t` and `holdoff_until` both pinned at `2.520` — the file view's last stamp. The producer's clock was frozen, not lagging, and the display stayed blank until something re-planned the store. diff --git a/src/acquisition.cpp b/src/acquisition.cpp index 162776f..560c3e4 100644 --- a/src/acquisition.cpp +++ b/src/acquisition.cpp @@ -3,6 +3,8 @@ #include "acquisition.hpp" +#include + #include #include "layout_store.hpp" @@ -50,6 +52,12 @@ Acquisition::Acquisition(ScopeModel& model, const Config& cfg) : model_(model), Acquisition::~Acquisition() { stop(); } void Acquisition::start() { + // A started -- or *restarted* -- client is a new stream. Going live from a file view is the restart that + // matters: the store still holds the event's samples, and the new stream's clock can sit behind their + // stamps, so the guard below has to re-base rather than clamp. `ring=[2.5200..2.5200]` with `n` climbing + // was the symptom -- every packet pinned flat at the file view's last stamp. + model_.reset_stream(); + last_stamp_ = -std::numeric_limits::infinity(); udps_client_config_t c; udps_client_config_init(&c); c.server_addr = cfg_.server.c_str(); diff --git a/src/model.hpp b/src/model.hpp index cd995fe..0dbca70 100644 --- a/src/model.hpp +++ b/src/model.hpp @@ -632,6 +632,20 @@ class ScopeModel { /// `mutex()` and the producer holds that lock for all of `on_data`, so a plain read under the /// lock is correct, and a reconnect with an identical CONFIG is a re-plan this sees and the /// stride/channel-count comparison does not. + /// A restarted client is a **new stream**, and the retained history belongs to the old one: its clock may + /// sit behind the stamps the previous stream wrote, and the producer's monotonic guard would then pin + /// every later packet flat at the old value -- the `ring=[x..x]` a display that never moves shows. + /// `reset()` is what the data-path invariants require of a rebuild (nothing of the old history behind), + /// and `plan_generation_` moving is what tells the producer to drop the state tied to the old store. + void reset_stream() { + std::lock_guard lock(mutex_); + // Null before the first CONFIG has planned anything: `Acquisition::start()` calls this, and at start-up + // there is no store yet. Dereferencing it there was a SIGSEGV with no output at all. + if (store_ != nullptr) store_->reset(); + ++plan_generation_; + ++generation_; + } + unsigned plan_generation() const noexcept { return plan_generation_; } private: