fix(spectrum): free the waterfall stream when closing a panafall (#3843) by jensenpat · Pull Request #3855 · aethersdr/AetherSDR

jensenpat · 2026-06-27T03:16:59Z

What

Closing a panafall-created panadapter never freed its waterfall stream on the radio. The FlexLib-correct teardown is the pair Panadapter.Close() + Waterfall.Close() — i.e. both display pan remove 0x<panId> and display panafall remove 0x<wfStreamId> (FlexLib v4.2.18) — but the close path sent only the first.

Two defects from the issue, both addressed:

Dead + bogus public API. RadioModel::removePanadapter issued display pan close, which is not a FlexLib command at all, and had zero callers — a latent trap.
Live close path omitted the waterfall removal. The GUI X-button and the layout-shrink path each sent an inline display pan remove only, so the panafall's waterfall stream was left allocated on the radio.

The client-side cleanup was already complete (the display pan <id> removed echo unregisters both streams), so get pans always read clean — which is precisely why this radio-side leak went unnoticed.

Fixes #3843.

The fix

RadioModel::removePanadapter now captures the waterfall id before sending (the removal echo deletes the PanadapterModel, which would otherwise race the read) and sends display pan remove plus display panafall remove when a waterfall is present.
Single source of truth: the X-button handler (MainWindow_Wiring) and the layout-shrink path (MainWindow::applyPanLayout) both route through removePanadapter instead of inlining display pan remove.
The automation bridge pan close verb now also drives removePanadapter, so it exercises the exact production teardown (and DRYs up the duplicated command pair).

Tightly scoped to the close-path bug. A radio-side stream-inventory diagnostic to make this class of leak directly observable (and to catch resource-level lingering / duplicate pans) is split out into its own feature request so this fix stays minimal and low-risk.

How the agent automation bridge proved it

Driven against a live FLEX-8400M (firmware 4.2.18). A leak-repro build (panafall-remove omitted) and the fixed build were each driven through the production close path (pan close → removePanadapter), capturing the radio command stream:

Build	`display pan remove`	`display panafall remove`
Broken baseline	✅ `display pan remove 0x40000001`	❌ absent
Fixed	✅ `display pan remove 0x40000001`	✅ `display panafall remove 0x42000001`

Broken-build capture:

RadioModel::removePanadapter: "0x40000001" waterfall: "0x42000001"
RadioModel::sendCommand: "display pan remove 0x40000001"
(no display panafall remove)

Fixed-build capture:

RadioModel::removePanadapter: "0x40000001" waterfall: "0x42000001"
RadioModel::sendCommand: "display pan remove 0x40000001"
RadioModel::sendCommand: "display panafall remove 0x42000001"

Firmware note: on this radio the waterfall UDP stops on pan-removal regardless, so the leak here is the waterfall object left allocated rather than continued UDP — the command-level capture above is the decisive evidence that the radio is now correctly told to free it. Continued-UDP waterfall leaks do occur on older (#268-class) firmware.

Test plan

Builds clean (Ninja, RelWithDebInfo, arm64).
Live FLEX-8400M: production close path sends the FlexLib-correct command pair (before/after capture above).
Maintainer review (label present on the issue).

Multi-panadapter: 2nd panadapter loses FFT spectrum (waterfall continues), cannot be closed — persistent across reboot and sole-client reconnect #268 — distinct glitch-corruption Multi-Flex case (Pan 2 FFT blank, stuck); different root cause.
On restart, AetherSDR restores persisted broken panadapter state instead of defaulting to single-pan safe mode #269 — restored broken pan state on restart.
Additional PAN added to session after ASDR and Radio startup. Pulling strange sessions, upon station startup. #3212 — duplicate-panadapter on reconnect (fixed via SliceRecreatePolicy).
PR feat(automation): observability fields + close/drag/showMenu/pan verbs, grab pan, token-anchored TX guard (#3646) #3842 — automation-bridge pan close verb that first proved the correct teardown.
Follow-up: radio-side stream-inventory / leak-detection feature request (filed separately).

💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat

jensenpat · 2026-06-27T03:38:52Z

Adversarial self-review pass

Ran a multi-agent review over this diff (4 dimensions — thread-safety, close-ordering, bridge semantics, detector soundness — each finding refute-verified). The production close-path fix drew zero findings. Two real soundness defects were found in the new streams leak detector and fixed in 5b0723a:

False negative — the orphan check was gated on !known*Streams.isEmpty(), which goes blind after pan close all (the live set empties), so a genuinely leaking waterfall would read orphanCount:0 — defeating the verb's own canonical reset → close all → read workflow.
False positive — creating a 2nd pan briefly flagged the new pan's waterfall during its sub-second registration-lag window (small age_ms, the "live leak" signature).

Both fixed by keying the detector off ever-registered AND not-currently-registered instead of live-set-non-empty: a leak is a stream we once owned and let go of (stays detectable after the set empties), while a never-yet-registered stream can't be mis-flagged during its lag window. Ever-registered sets reset on disconnect.

Live re-verified (FLEX-8400M): add-2nd-pan now reads orphanCount:0 before close (the lag false positive is gone), clean through close. Finding 1's positive catch needs firmware that keeps streaming a removed pan's waterfall (#268 class) to live-demonstrate; this firmware stops the UDP either way, so the command-level capture remains the decisive proof.

…hersdr#3843) Closing a panafall-created panadapter left its waterfall stream allocated on the radio. The FlexLib-correct teardown is the pair Panadapter.Close() + Waterfall.Close() — `display pan remove` AND `display panafall remove` (FlexLib v4.2.18) — but the close path sent only the first. The dead `RadioModel::removePanadapter` was worse still: it issued `display pan close`, which is not a command at all. The client-side cleanup was already complete (the `display pan <id> removed` echo unregisters both streams), so `get pans` always read clean — which is exactly why the radio-side leak went unnoticed. Fix: - RadioModel::removePanadapter now captures the waterfall id (before the removal echo deletes the model) and sends `display pan remove` plus `display panafall remove` when a waterfall is present. - The GUI X-button (MainWindow_Wiring) and the layout-shrink path (MainWindow::applyPanLayout) both route through removePanadapter as the single source of truth, instead of an inline `display pan remove`. - The automation bridge `pan close` verb now also drives removePanadapter, so it exercises the real production teardown (and DRYs up the duplicated command pair). Proven on a live FLEX-8400M (firmware 4.2.18) via the agent automation bridge, capturing the radio command stream through the production close path (`pan close` -> removePanadapter) on a leak-repro build vs the fixed build: - broken: `display pan remove 0x40000001` sent; `display panafall remove` ABSENT. - fixed: `display pan remove 0x40000001` AND `display panafall remove 0x42000001` both sent. A radio-side stream-inventory diagnostic to make this class of leak directly observable is split out into its own feature request, so this fix stays minimal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

jensenpat · 2026-06-27T03:53:25Z

Scope trimmed. Squashed to a single commit containing only the panafall close-path fix (removePanadapter sends the display pan remove + display panafall remove pair; X-button, layout-shrink, and the bridge pan close all route through it). The radio-side stream-inventory / leak-detection diagnostic I'd prototyped here (the streams verb + orphan detector, and the two soundness fixes from the review above) is split out into #3856 so it gets a proper full-feature design — including the resource-level-lingering case that needs a radio-authoritative display query, not just UDP-orphan detection. This PR now stays minimal and low-risk; the command-level before/after proof of the fix is unchanged.

aethersdr-agent

Reviewed the diff against main. This is a clean, tightly-scoped fix and the reasoning checks out. Thanks for the thorough writeup and the live-hardware before/after capture, @jensenpat.

Verified the key claims:

RadioModel::removePanadapter was genuinely dead on main — the only other removePanadapter references are PanadapterStack::removePanadapter (a different method) and the stale comments. No callers, so re-pointing it at the real teardown is safe.
display pan close was indeed wrong: the status path keys off display pan <id> removed, which display pan remove produces. Good catch on the bogus command.
The waterfall-id capture-before-send is correct and necessary — the removed echo does m_panadapters.take(panId) → deleteLater() (RadioModel.cpp:4446), so reading waterfallId() afterward would dereference a torn-down model.
sendCommand just wraps sendCmd with a log line, so swapping sendCmd → sendCommand is behavior-preserving (and adds useful tracing).
waterfallId() returns the string stream id, which is the right form for display panafall remove.
Null-pan fallback is handled: an explicit 0x… id not in the model still sends display pan remove <id> and just skips the waterfall command. Robust at the boundary.

Consolidating the X-button, layout-shrink, and pan close verb onto a single removePanadapter is the right call — no more duplicated command pairs to drift.

One minor, non-blocking note: in AutomationServer::doPan the close loop now reads waterfallId() once for the JSON response and again inside removePanadapter. Harmless (a cheap hash lookup, and the value is stable until the async echo arrives), just slightly redundant — fine to leave as-is.

CI is green across all six checks. LGTM.

_{🤖 aethersdr-agent · cost: $3.3487 · model: claude-opus-4-8}

ten9876

Approved. Verified the fix end-to-end:

removePanadapter captures the waterfall id before sending — necessary, since the display pan <id> removed echo deletes the PanadapterModel, so reading waterfallId() afterward would race the teardown. Then sends the FlexLib-correct pair display pan remove + display panafall remove.
Null-pan fallback is safe (explicit id not in the model still removes the pan, skips the waterfall command).
Single source of truth: X-button, layout-shrink, and the bridge pan close verb all route through removePanadapter — no duplicated command pairs to drift.
Removed the bogus, call-free display pan close.

Not a keying path (Principle VI N/A). Excellent live-hardware before/after capture (FLEX-8400M, fw 4.2.18) proving display panafall remove now fires. CI green on all 6. Thanks @jensenpat — clean fix for a quiet radio-side leak.

aethersdr#3856) Adds a `streams` bridge verb that makes leaked/duplicate panadapter and waterfall streams on the radio directly observable — a class of bug `get pans` can never surface, because the client always cleans up its own view on the radio's "removed" echo. Two independent radio-authoritative layers: Layer A — `streams` (VITA-49 UDP truth). PanadapterStream records any FFT/waterfall packet for a stream id that was EVER registered this session but is no longer registered — a stream we once owned and let go of that the radio keeps transmitting. Keyed off "ever-registered AND not-now-registered" so it stays detectable after `pan close all` and never mis-flags a freshly created stream's registration-lag window. Catches continued-stream leaks (the aethersdr#268 class). `streams reset` re-baselines the tally. Layer B — `streams radio` (status-bookkeeping truth). RadioModel now accumulates the radio-authoritative display-object set (m_radioDisplayPans / m_radioDisplayWaterfalls) from ALL "display pan" / "display waterfall" status — ours, foreign, and orphan — pruned on the matching "removed", independent of m_panadapters (which only holds objects WE own). The pure DisplayInventoryPolicy::classify() then labels each object ours/foreign/orphan and flags leaked waterfalls (parent panadapter gone) — the aethersdr#3843 fingerprint. This catches resource-level lingering that emits no UDP, which Layer A cannot see. DisplayInventoryPolicy is a pure, header-only function (mirroring RadioStatusOwnership / SliceRecreatePolicy) with a unit test (display_inventory_policy_test) covering clean, leaked-parent-gone, foreign- vs-orphan, no-parent-reported, and unknown-own-handle cases. Proven on a live FLEX-8400M (firmware 4.2.18) via the bridge: - `streams radio` tracked the inventory exactly across add/close (1 pan/1 wf → 2/2 → 1/1), every object ownership=ours, no false-positive leaks. - Key firmware finding: on this radio `display pan remove` ALONE makes the radio also remove the waterfall — it echoes `display waterfall <id> removed`. So neither aethersdr#3843 symptom (continued UDP nor lingering resource) reproduces on 8000-series/4.2.18; the radio auto-cascades. The aethersdr#3855 close-path fix remains correct per FlexLib and matters for firmware that does NOT cascade (the aethersdr#268 class). Because this firmware never leaves an orphan, Layer B's positive leak detection is covered by the unit test; live it correctly reads clean with no false positives. All `streams` actions are RX/observe only — none sends a radio command or keys the transmitter. Docs in docs/automation-bridge.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@jensenpat

…n (streams verb) (#3856) (#3857) ## What Implements the radio-side display-stream **inventory + leak detection** feature requested in #3856. It makes leaked / duplicate / lingering panadapter & waterfall streams *on the radio* directly observable — a class of bug `get pans` can never surface, because the client always cleans up its own view on the radio's `removed` echo. Two independent radio-authoritative layers behind one `streams` bridge verb: ### Layer A — `streams` (VITA-49 UDP truth) `PanadapterStream` records any FFT/waterfall packet for a stream id that was **ever registered this session but is no longer registered** — a stream we once owned and let go of that the radio keeps transmitting. Keyed off *ever-registered ∧ not-now-registered* (not "live set non-empty"), so it stays detectable after `pan close all` and never mis-flags a freshly-created stream's registration-lag window. Catches **continued-stream** leaks (the #268 class). `streams reset` re-baselines. ### Layer B — `streams radio` (status-bookkeeping truth) `RadioModel` now accumulates the radio-authoritative display-object set (`m_radioDisplayPans` / `m_radioDisplayWaterfalls`) from **all** `display pan` / `display waterfall` status — ours, foreign, and orphan — pruned on the matching `removed`, independent of `m_panadapters` (which only holds objects *we* own). The pure `DisplayInventory::classify()` labels each object `ours` / `foreign` / `orphan` and flags **leaked waterfalls** (parent panadapter gone) — the #3843 fingerprint. This catches **resource-level lingering that emits no UDP**, which Layer A cannot see. `DisplayInventoryPolicy` is a pure, header-only function (mirroring `RadioStatusOwnership` / `SliceRecreatePolicy`), with a unit test (`display_inventory_policy_test`, 12 assertions) covering clean, leaked-parent-gone, foreign-vs-orphan, no-parent-reported, and unknown-own-handle cases. Closes #3856. ## Bridge surface | action | layer | effect | |---|---|---| | `streams` | A | `registeredPanStreams`, `registeredWfStreams`, `orphanStreams` (`streamId`, `kind`, `packets`, `age_ms`) | | `streams radio` (alias `inventory`) | B | radio display-object set classified ours/foreign/orphan + `leakedWaterfalls` | | `streams reset` | A | clear the orphan tally | All `streams` actions are RX/observe only — none sends a radio command or keys TX. Docs in `docs/automation-bridge.md`. ## Proven on a live FLEX-8400M (firmware 4.2.18) `streams radio` tracked the inventory **exactly** across an add/close cycle, every object `ownership: ours`, no false-positive leaks: ``` baseline: radioPanCount=1 radioWaterfallCount=1 leakCount=0 after add: radioPanCount=2 radioWaterfallCount=2 leakCount=0 (both ours, parentMissing=false) after close:radioPanCount=1 radioWaterfallCount=1 leakCount=0 ``` **Key firmware finding (answers the open #3843 question):** on this radio, `display pan remove` **alone** makes the radio *also* remove the waterfall — it echoes `display waterfall <id> removed`. A leak-repro build that deliberately omitted `display panafall remove` produced the **same clean result** (`leakedWaterfalls=[]`, waterfall count 2→1, removal echo observed). So **neither #3843 symptom — continued UDP nor lingering resource — reproduces on 8000-series / 4.2.18**: the radio auto-cascades. The #3855 close-path fix remains correct per FlexLib and matters for firmware that does **not** cascade (the #268 class). Because this firmware never leaves an orphan, **Layer B's positive leak detection is covered by the unit test** (synthetic parent-gone waterfall → flagged); live, it correctly reads clean with no false positives. Layer A likewise reads `orphanCount:0` here (this firmware stops the UDP), and is the guard for firmware that keeps streaming. ## Test plan - [x] `display_inventory_policy_test` — 12/12 pass (pure classification logic, incl. the leaked-waterfall case). - [x] Full app builds clean (Ninja, RelWithDebInfo, arm64). - [x] Live FLEX-8400M: `streams` + `streams radio` inventory accurate across add/close; no false-positive leaks; firmware cascade behavior characterized. - [ ] Maintainer review. ## Notes / follow-ups - The empirical cascade finding means Layer B's live positive-catch can only be demonstrated on #268-class firmware (which leaves the orphan). The detection logic is unit-tested. - Layer B is positioned to plug into reconnect/duplicate-pan reconciliation (#3429) — surfacing a radio-side pan the client doesn't own — as a future step. ## Related - #3856 — this feature request. - #3855 / #3843 — the panafall close-path fix this observability complements (the *send* side; this is the *observe* side). - #268 — continued-UDP waterfall leak on older firmware (the case Layer A guards). - #3429 — reconnect reconciliation (future Layer B integration point). 💻 Generated with Claude Code (Opus 4.8) with architecture by @jensenpat --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Jeremy [KK7GWY] <kk7gwy@aethersdr.com>

jensenpat marked this pull request as ready for review June 27, 2026 03:26

jensenpat requested review from a team as code owners June 27, 2026 03:26

jensenpat force-pushed the fix/3843-panafall-close branch from 5b0723a to a0b2c9c Compare June 27, 2026 03:51

jensenpat mentioned this pull request Jun 27, 2026

Agent automation bridge: radio-side display-stream inventory + leak detection (streams verb) — prevent leaked/duplicate pan & waterfall streams #3856

Closed

5 tasks

jensenpat mentioned this pull request Jun 27, 2026

feat(automation): radio-side display-stream inventory + leak detection (streams verb) (#3856) #3857

Merged

4 tasks

aethersdr-agent Bot reviewed Jun 27, 2026

View reviewed changes

ten9876 approved these changes Jun 27, 2026

View reviewed changes

ten9876 merged commit 40768ff into aethersdr:main Jun 27, 2026
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(spectrum): free the waterfall stream when closing a panafall (#3843)#3855

fix(spectrum): free the waterfall stream when closing a panafall (#3843)#3855
ten9876 merged 1 commit into
aethersdr:mainfrom
jensenpat:fix/3843-panafall-close

jensenpat commented Jun 27, 2026 •

edited

Loading

Uh oh!

jensenpat commented Jun 27, 2026

Uh oh!

jensenpat commented Jun 27, 2026

Uh oh!

aethersdr-agent Bot left a comment

Uh oh!

ten9876 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jensenpat commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

The fix

How the agent automation bridge proved it

Test plan

Related

Uh oh!

jensenpat commented Jun 27, 2026

Adversarial self-review pass

Uh oh!

jensenpat commented Jun 27, 2026

Uh oh!

aethersdr-agent Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ten9876 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jensenpat commented Jun 27, 2026 •

edited

Loading