ci(sanitizers): let a leak fail its test in the ASan lane. Principle VIII. by ten9876 · Pull Request #6176 · aethersdr/AetherSDR

ten9876 · 2026-10-04T23:02:29Z

Summary

Refs #6154. Land after #6173.

The ASan lane's ASAN_OPTIONS include halt_on_error=0. With that option, LeakSanitizer still prints a leak, but the process exits 0. The test "passes", ctest --output-on-failure throws its output away, and the leak is never seen. As a result the lane has never failed a test on a leak. #6173 had to find the three real ones by hand, with a full local sweep:

disconnectClientHandlesThen's self-owning lambda;
WDSP's calcc semaphores, across 17 tests;
a test's leaked QFocusEvent.

This PR drops halt_on_error=0 from the ASan entry, so a leak fails its test and lands in the sticky issue like any other finding.

Measured on a leaking binary (asr_whisper_smoke_test locally, which leaks the ggml Vulkan registry on a GPU machine):

`ASAN_OPTIONS`	exit	leak printed
`detect_leaks=1`	1	yes
`detect_leaks=1:halt_on_error=0`	0	yes
`detect_leaks=1:abort_on_error=0`	1	yes
CI's full string (before this PR)	0	yes

Nothing is lost. Without -fsanitize-recover=address, the first ASan error aborts regardless of halt_on_error, so the option only ever affected leaks. UBSan's own halt_on_error=0 is left alone: UBSan is built with -fno-sanitize-recover, so it aborts anyway.

Why after #6173: this makes the lane strictly stricter. Landing #6173 first means the first run that can see leaks starts from a tree whose leaks have already been fixed. In the local full sweep, the only leak left after #6173 is ggml's Vulkan backend registry, which needs a Vulkan GPU, and the runner has none.

Constitution principle honored

Principle VIII: a sanitizer lane that can't fail on a class of finding can't vouch for it.

Test plan

Exit-code behaviour measured locally (table above)
Workflow YAML parses, and the matrix runtime_env resolves to the new string
Leak-visible lane run before merge: 37243769261, fix(sanitizers): clear every ASan/UBSan/TSan report on the full-tree lane, incl. two WDSP races. Principle VIII. #6173's final head plus this PR, confirmed in the log running with ASAN_OPTIONS=detect_leaks=1:abort_on_error=0:strict_string_checks=1:… (no halt_on_error=0). No LeakSanitizer (or ASan/UBSan) report on the runner, including no fontconfig leak under offscreen Qt, so no lsan.supp is needed. ASan's only failures are the two known sanitizer-slowdown timeouts.
After fix(sanitizers): clear every ASan/UBSan/TSan report on the full-tree lane, incl. two WDSP races. Principle VIII. #6173 and this PR merge, the next weekly run on main confirms the same

Checklist

Commits are signed
CI-only change; no code, settings or meter UI touched
No CHANGELOG.md

🤖 Generated with Claude Code

…VIII. The ASan entry's ASAN_OPTIONS carried halt_on_error=0. With it, LeakSanitizer still prints the leak report but the process exits 0, so the test "passes" and ctest --output-on-failure discards the output. The lane has never failed a test on a leak; #6173 had to find three of them (disconnectClientHandlesThen's self-owning lambda, WDSP's calcc semaphores across 17 tests, a test's QFocusEvent) with a local sweep. Measured on a leaking binary: detect_leaks=1 exits 1; adding halt_on_error=0 exits 0 with the same report printed; abort_on_error=0 alone does not change the exit code. Dropping halt_on_error=0 loses nothing for ASan errors: without -fsanitize-recover=address the first one aborts regardless. UBSan's own halt_on_error=0 is unchanged (UBSan is built -fno-sanitize-recover, so it aborts anyway). Land after #6173, so the lane's first leak-aware run starts from the leak-free tree. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

ten9876

Issue fit: Yes — this makes LeakSanitizer findings fail CTest instead of disappearing from --output-on-failure. Reviewed 097372424138dbf3f750fc98130d4d8c996f54b1 against #6154 and its consolidated maintainer diagnosis. It repairs the reporting gap, not the underlying leaks; the disclosed dependency on #6173 remains a landing-order constraint.

Scope

File/group	Claimed?	Verdict
`.github/workflows/sanitizers.yml:60-68`	Yes: remove ASan `halt_on_error=0`, explain why	Entire diff is explained; ASan compiler flags, UBSan/TSan settings, test selection and reporting chain are unchanged

One authored commit (2026-10-04), no unrelated files, new dependencies, public protocol, settings, UI preference, or changelog entry. The scope checklist holds. This is an infrastructure bug fix; no RFC or engine-boundary manifest change is warranted.

Blockers

None in this change.

Non-blocking nits

.github/workflows/sanitizers.yml:60-65: trim the incident narrative and attribution (“blind ... until #6173 found them by hand”) per AGENTS.md's Comments guidance. The PR body already records that history. A sufficient replacement is:
```
          # Keep ASan's default halt_on_error so LeakSanitizer returns nonzero.
          # CTest then retains leak reports with --output-on-failure.
```
.github/workflows/sanitizers.yml:67-70: retain the stated “land after #6173” sequence and dispatch the sanitizer lane on the integrated main tree. This review verifies leak visibility, not the separate claim that all application/vendor leaks have disappeared. My review of #6173 also identifies an outstanding widget-lifetime finding; that belongs to #6173 rather than being a defect in this one-line CI fix.

Verification and attacks

Parsed the exact-head YAML and used its ASan entry's extra_cxx flags and runtime_env verbatim to build/run a socket-free allocation probe with GCC 16.2. The same binary has a deliberately leaking path and a clean path. Under head options, ctest --no-tests=error --output-on-failure returned 8, reported 123 byte(s) leaked in 1 allocation(s), and retained the diagnostic; the clean path passed. Restoring only the removed halt_on_error=0 option made both tests pass and CTest return 0, hiding the leak report. This directly reproduces the claimed before/after failure mode without application or GPU dependencies.
Inspected the complete reporting path: the quoted matrix heredoc exports the options, CTest's nonzero pipeline result is captured despite continue-on-error, and the existing report/artifact/sticky-issue/final-failure steps consume that exit value. No change disables leak detection or broadens suppressions. The existing UBSan and TSan entries and the frozen per-PR test gate are untouched.
Test-boundary preflight: no test source or socket-owning target is added or modified. Only the local socket-free probe ran; no full application suite, live radio, GitHub workflow dispatch, or GUI bridge session was needed for this CI-only change. Actual hosted artifact/issue publication and the author's machine-specific whole-tree leak sweep were not reproduced. No model-invocable code-review skill was available; this was a manual audit plus empirical validation.

CI and recommendation

The PR CI runs were created at 2026-10-04 23:02:32 UTC (the dynamic Code Quality run at 23:02:30), after current main 85f2a1249d655026adc017d00819c17b9b29ed3e at 22:40:53 UTC. There is no timestamp evidence of a stale main merge at this review. Protection remains strict: false, so reassess after main moves. The changed Sanitizers workflow itself runs on schedule/manual dispatch, not this PR event; ordinary green checks do not validate its runtime semantics.

Checks at submission: build: SUCCESS; Analyze (javascript-typescript): SUCCESS; Sanitizer option configures: SUCCESS; Analyze (python): SUCCESS; check-windows: SUCCESS; Static checks: SUCCESS; check-macos: SUCCESS.

Recommendation: Approve with nits, subject to normal CI completion and the disclosed dependency/landing sequence. The measured leak-versus-clean control supports the fix; no code blocker was found here. Follow with a sanitizer dispatch on the integrated tree.

— Jeremy KK7GWY + Codex

aethersdr-agent

Issue fit

No fixes: link — the body says Refs #6154 and this is a CI-lane change with a clear root cause, so GOVERNANCE.md's RFC-first rule doesn't bite (bug fixes with a clear root cause explicitly don't need one). Against its own stated intent it lands: .github/workflows/sanitizers.yml:67 drops halt_on_error=0 from the ASan entry's ASAN_OPTIONS, which is exactly the one option the diagnosis blames, and nothing else changes. The diagnosis itself is corroborated independently of the table in this body — #6173 reports the same exit-0 behaviour on a second binary (phone_tx_filter_numeric_entry_test), with abort_on_error=0 as the control that still exits 1. Good find; a lane that silently can't fail on a whole class of report is the worst kind of green.

Scope

File	What it changes	Claimed by title/body?	Verdict
`.github/workflows/sanitizers.yml:60-67`	Removes `halt_on_error=0` from the ASan/UBSan entry's `runtime_env`; adds a 6-line comment explaining why it must not come back	Yes — this is the entire PR	In scope

One commit (09737242) on top of 85f2a124, one file, +7/−1. No code, no settings keys, no capability surface, no UI, no CHANGELOG.md entry (correct — don't add one). The TSan entry's halt_on_error=0 and UBSan's are untouched, and the body explains why UBSan's is left alone. Everything in the diff is explained by the stated fix. No sibling lane left half-converted: grep for halt_on_error/LSAN_OPTIONS/detect_leaks across the head checkout returns only this file (lines 60, 67, 68, 69, 84), so there is no second place carrying the same option.

Blockers

None.

Nits (non-blocking)

.github/workflows/sanitizers.yml:67 — the strict configuration has never actually been run in CI. Both runs the body and #6173 cite (37208806037, and the #6173 branch dispatch 37230031178) ran with halt_on_error=0 in place, so neither exercised leak-as-failure. The evidence that the tree is leak-clean comes from the author's local full sweep — and #6173 says of that sweep: "Leaks from this machine only (fontconfig/pango/GTK, NVIDIA cuInit, ggml Vulkan) were suppressed locally, not in the repo." tests/sanitizers/ contains only tsan.supp; there is no lsan.supp, and LSAN_OPTIONS=print_suppressions=0 at line 69 wires no suppressions path. GTK/pango and the GPU leaks won't appear on a GPU-less runner under QT_QPA_PLATFORM=offscreen, but fontconfig/freetype can — the offscreen plugin still brings up the Unix font database. So the first run that can fail on a leak is the run after merge, which the test plan's own unchecked box concedes. Concrete step, cheaper than finding out on main: rebase this branch onto #6173's head and workflow_dispatch Sanitizers on it, so the leak-visible evidence exists before the merge rather than after. I'd rather not see a preemptive lsan.supp — if that dispatch turns up environment-only leaks, then it's worth wiring one the way tsan.supp is (readability guard plus the path appended in-step, since ${{ github.workspace }} is empty at matrix-expansion time, per the comment at line ~238).
Merge ordering is disclosed but not enforceable. #6173 is still open, and this branch is based on main, not on it. If this lands first, the next weekly run fails #6173's known leaks (86 create_calcc blocks across 17 tests, plus two more) and opens a fresh sticky — the body's "Land after #6173" is a maintainer sequencing instruction, not a finding against the PR. Worth restating in the merge queue, not in the code.

What I tried to break

The load-bearing exit-code claim. I tried to reproduce the body's table and could not: I built a leaking ASan binary locally and every variant printed AddressSanitizer: detect_leaks is not supported on this platform — arm64 macOS has no LeakSanitizer, so the 134 → 1 flip I got is just abort_on_error acting on the unsupported-platform die, not a leak path. No docker/podman on this box either, so no Linux container to retry in. What I can say from source rather than from a run: the behaviour matches compiler-rt's asan_rtl.cpp, where the atexit handler is selected on flags()->halt_on_error — __lsan::DoLeakCheck (fatal) when set, DoRecoverableLeakCheckVoid when clear, and the recoverable path reports the leak but never reaches HandleLeaks()/Die(), hence exit 0. That is consistent with the table, including the abort_on_error=0 control row. Reasoned from source, not reproduced here — the author's measurement plus #6173's second binary is the only empirical evidence, and it's a different binary each time with the right control, which is why I'm not treating it as unverified-and-therefore-suspect.
"Nothing is lost" for ASan errors. Holds by the same mechanism: without -fsanitize-recover=address, instrumentation calls the fatal report entry points (__asan_report_load4, not the _noabort variants), which Die() irrespective of halt_on_error. Removing the option also makes that death a controlled internal__exit rather than falling through to the trap, since abort_on_error=0 is retained at line 67 — so ctest sees exit 1, not a SIGABRT/SIGILL, which is the better shape for the report chain. Same argument retires UBSan's halt_on_error=0 as inert under -fno-sanitize-recover=undefined (line 57), so leaving it in place at line 68 costs nothing.
Whether a leak survives the report chain once it does fail a test. LSan's summary is SUMMARY: AddressSanitizer: N byte(s) leaked in M allocation(s) at column 0, so the extract step's grep -q '^SUMMARY:' / grep -B 80 '^SUMMARY:' catches it and the issue body won't fall back to the 50 KB tail. The fingerprint is the sorted failed-test name set, so a single leak spanning 17 tests dedupes into one sticky rather than churning — this does not reintroduce the per-run-duplicate problem the fingerprint rewrite fixed.
Scope and deletions. Read the - line as carefully as the +: the only removal is the option itself, no guard or early return. Checked the file for a second lane or script carrying the same option (none), and confirmed the diff is one commit on current main with nothing cherry-picked alongside.
No socket tests, fake peers, or test files are added or removed by this PR — the diff is a single workflow entry, so there is nothing to record under that heading. CodeGuard and Copilot returned nothing on the changed file; I'm not reading that as a clean bill, only as no leads to verify.
All seven checks on 09737242 are green (build, check-macos, check-windows, Static checks, Sanitizer option configures, both CodeQL analyses). None of them run the Sanitizers workflow, so green here says the YAML parses, not that the lane behaves.

Recommendation

Approve with nits. The change is one option removal with the mechanism correctly identified, a comment that will stop it being reinstated, and no collateral scope — this is the right fix at the right seam. The only real caveat is sequencing: merge #6173 first, and ideally rebase this onto #6173's head and dispatch Sanitizers once before merging, so the first leak-visible run isn't on main. If that dispatch does surface an environment-only leak (fontconfig is the candidate), handle it then with a wired-up lsan.supp rather than by putting halt_on_error=0 back. Thanks for chasing this one down to the flag — "the lane was blind to an entire finding class and nobody could tell" is exactly the thing worth a standalone PR.

_{🤖 aethersdr-agent · cost: $2.9764 · model: claude-opus-5}

…ple VIII. Review of #6176: trim the comment's incident narrative and attribution per AGENTS.md's Comments guidance; the PR body keeps the history. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

ten9876 · 2026-10-04T23:27:30Z

Both nits are addressed on head aeefd9b5c.

Comment trimmed to your suggested two lines. The history stays in the PR body.
Leak-visible evidence before merge. Rather than rebase this PR onto fix(sanitizers): clear every ASan/UBSan/TSan report on the full-tree lane, incl. two WDSP races. Principle VIII. #6173 (it stays based on main, so it can land independently once fix(sanitizers): clear every ASan/UBSan/TSan report on the full-tree lane, incl. two WDSP races. Principle VIII. #6173 is in), I pushed a temporary integration branch, ci/integration-6173-6176. It is fix(sanitizers): clear every ASan/UBSan/TSan report on the full-tree lane, incl. two WDSP races. Principle VIII. #6173's final head 7f37cf045 with this PR's two commits cherry-picked, and I dispatched Sanitizers on it: 37243769261. That is the first run of the lane where a leak fails its test, on the tree this PR will actually meet.
What happens next depends on that run. If it surfaces an environment-only leak on the runner (fontconfig is the likely candidate), I'll wire up an lsan.supp the way tsan.supp is wired, with the readability guard and the path appended in the step, rather than restore halt_on_error=0. If it's clean, no suppression file is needed. I'll delete the integration branch afterwards.
Sequencing is unchanged: land after fix(sanitizers): clear every ASan/UBSan/TSan report on the full-tree lane, incl. two WDSP races. Principle VIII. #6173.

ten9876

Issue fit: No closing issue; it references #6154, whose leak findings are the motivation. Against its own intent, which is to make a leak fail its test in the ASan lane, the change does what it says. I reproduced both the claim and its "nothing is lost" corollary on a real build.

This review is at head aeefd9b5. The comment trim there addresses the history nit I would otherwise have raised. No blockers, so I'm posting this as COMMENT.

Scope

File	Change	Claimed?	Verdict
`.github/workflows/sanitizers.yml:60-63`	Drop `halt_on_error=0` from the ASan entry's `ASAN_OPTIONS`, plus a 2-line comment	Yes	In scope

Two commits, both authored 2026-10-04, one file. UBSan and TSan options are unchanged. No other halt_on_error/detect_leaks site exists in the tree: I checked .github, tests, cmake and docs. No CHANGELOG entry. Socket preflight: no test sources touched.

Blockers

None.

Nits (non-blocking)

Let the integration dispatch finish before merging. 37243769261 is the stacked #6173+#6176 run, and it was still in progress when I posted. The lane has no LSan suppressions file, so an image-only system-library leak would turn every GUI test red at once. My local runs (below) found none, but that's Arch Linux/Qt 6.12 offscreen, not ghcr.io/aethersdr/aethersdr-ci:latest. The author's stated fallback, an lsan.supp wired the way tsan.supp is, is the right one if it does.

Verified empirically

The headline claim. I built a minimal leaking binary with CI's ASan flags (-fsanitize=address,undefined -fno-sanitize-recover=undefined):

old ASAN_OPTIONS this PR's

leak only exit 0, SUMMARY printed exit 1

heap-buffer-overflow exit 1 exit 1

So halt_on_error=0 only ever masked leaks.

On real tests. I built #6173 with CI's exact ASan+UBSan flags (Debug, AETHER_SHARED_CORE=ON), reverted #6173's three leak fixes one at a time, and ran the affected tests under both option strings:

Mutation	old options	this PR's options
`disconnectClientHandlesThen` strong self-capture → `tx_operation_integration_test`	exit 0, "216 bytes leaked in 5 allocations" printed and discarded	exit 1
`new QFocusEvent` → `phone_tx_filter_numeric_entry_test`	exit 0, 24 bytes leaked	exit 1
patch 18 reverted → `wdsp_channel_test`	leak printed	exit 1

With #6173's fixes in place, the 10 targeted tests (including gui_nested_lifetime_test at 7f37cf04) exit 0 with no SUMMARY: line under this PR's options. No fontconfig or other system-library leak showed up in the five Qt-widget tests among them.

What I tried to break.
- Whether variable leak sizes (35,840 vs 62,160 bytes across two runs of one mutation) would fork the sticky issue. They won't: the fingerprint is the sorted failing-test names, with the SUMMARY hash only as fallback (sanitizers.yml:306-322).
- Whether the comment's premise holds. With the flag dropped, ASan's default halt_on_error=1 applies; a leak at exit then returns exitcode (1), as the table shows.
CI. Runs on aeefd9b5 were created 23:26Z, after main's 22:40Z tip, so they are current. None of them execute sanitizers.yml.

Recommendation: Approve with nits after #6173 lands and 37243769261 comes back clean, or with an lsan.supp if it doesn't.

— Jeremy KK7GWY + Claude

ten9876 · 2026-10-05T01:51:30Z

Evidence is in: 37243769261, #6173's final head plus this PR, ran with the new ASAN_OPTIONS (verified in the log) and produced no LeakSanitizer, ASan or UBSan report. That includes no fontconfig leak under offscreen Qt, so no lsan.supp is needed. The only failures are the two known sanitizer-slowdown timeouts. The temporary integration branch is deleted, and its stickies (#6189, #6191) are closed into #6154 / #6156. Landing order is unchanged: after #6173.

…lane, incl. two WDSP races. Principle VIII. (#6173) ## Summary Refs #6154, #6156. This doesn't close them: the sanitizer-slowdown timeouts are still open (see below). The weekly Sanitizers lane recently started building the full test tree again (#6136, #6152). Its first complete run on `main`, [37208806037](https://github.com/aethersdr/AetherSDR/actions/runs/37208806037), failed 6 tests under ASan + UBSan and 11 under TSan. This PR fixes every one of those failures that was an actual sanitizer report. **In the branch run [37230031178](https://github.com/aethersdr/AetherSDR/actions/runs/37230031178), neither lane reports a single sanitizer error.** What still fails there is timeouts, plus one plain assertion in `rtl_capture_worker_test` (below). Two of the eight commits change shipped DSP behaviour. They are vendored WDSP patches, documented in `third_party/wdsp/AETHERSDR-PATCHES.md`: - **Patch 16, transmit path.** `SetTXABandpassFreqs()` was the only one of four `TXASetupBPFilters()` callers that didn't take `csDSP`. `TXASetupBPFilters()` clears `bp1`/`bp2 run` first and re-enables them only after the FIR redesign. So a TX filter change while transmitting with COMP (and CESSB) on could run blocks with the post-compressor bandpass bypassed, which is out-of-band splatter. Fix: take the lock, as `SetTXAMode()` does. It adds no new lock order, and costs one FIR redesign stall per TX filter change, which upstream already accepts for a mode change. - **Patch 17, panadapter analyzer.** 1. `stop` and `end_dispatcher` are now atomic. 2. `input_busy` is released *after* `stitch()`, so the next frame's FFT workers can no longer overwrite results while the stitch is still copying them (a torn spectrum frame). 3. `sendbuf()` now hands off `IQout_index` inside `BufferControlSection`, closing a lost update. Only part 2 changes timing: the next frame's FFTs start one stitch later, tens of microseconds. | Commit | Lane | Finding | Fix | |---|---|---|---| | `1f9c4ed66` | ASan | `offline_health_registry_test`: stack-use-after-scope. Section 6 reuses a factory that captured section 5's block-scoped `RecorderState` by reference | `rec` moves to `main()` scope (test only) | | `cacf9fe30` + `49fa9a75a` | UBSan | `rx_applet_squelch_reconciliation`, `vfo_dsp_launcher_width`, `fm_filter_controls`: invalid `QPointer<QLabel>` downcast in `VfoWidget::eventFilter` during the collapsed label's teardown | Remove the filter before `~VfoWidget` deletes the label, **and** test the event type before the guarded-pointer comparison. The second covers the label dying *first*, after `beginDirectEntry()` `raise()`s the VFO (found in review). Regression: `gui_nested_lifetime_test` `vfoCollapsedLabelTeardownInEitherOrder`, covering both child orders | | `01d06c6d1` | TSan | `receive_intent_routing`, `tci_automation`, `tci_server_review`: lock-order inversion between two of Qt's own TLS-backend mutexes, both taken on the main thread | Anchored `deadlock:QSslSocketPrivate::tlsBackendInUse` suppression | | `f04a9ffb2` | TSan | `tci_server_review`: unlocked global `QStringList` log sink written from worker threads | One mutex for the sink and its lists (test only) | | `6f6e4a5fd` | TSan | `tx_operation_integration`: the command sink appends on the `RadioConnection` thread to a list destroyed before that thread is joined | Declare the list before `radio` (test only) | | `fbbd24087` | TSan | `wdsp_channel`: WDSP patch 16 (above) | Lock in `SetTXABandpassFreqs` | | `af9c15224` | TSan | `anan_rxdsp_handedness`, `spectrum_sequence_gap`, `wdsp_process_tally`: WDSP patch 17 (above) | Three analyzer hand-offs synchronised | | `31e249c3e` | TSan | `weather_radar_loading`: Qt's `QHttpThreadDelegate` teardown vs a main-thread reply delete. Three map tests were fetching live OSM tiles | Stub network in those three tests, plus an anchored `race:QHttpThreadDelegate::~QHttpThreadDelegate` suppression | Each suppression in `tests/sanitizers/tsan.supp` carries its reasoning and a caveat. Neither is anchored on a frame AetherSDR code runs inside. The file's DO-NOT-ADD list is untouched. Note that a suppression hides **every** stack it matches, not only the reported cycle or teardown. So zero TSan reports with these entries means no *unsuppressed* race was seen. It is not proof that every Qt TLS or HTTP-teardown interaction is race-free. **Still failing after this PR, tracked in #6154 / #6156:** - **Timeouts under the sanitizers' slowdown, with no sanitizer report.** ASan: `rtl_spectrum_averaging`, `weather_radar_loading`. TSan: the same two plus `audio_engine_rates`, `nr_stereo_independence`, `peripheral_auth_dialog`. - **`rtl_capture_worker_test`, TSan lane only:** fails the assertion "DC mode change discards old estimates and shows the actual first corrected window", with no race reported. This branch touches no RTL code, and the test passed in the same run's ASan lane. It passed in both lanes this morning, before `main` gained #5919's RTL changes. **Leak fixes (added after the first push, found by a full local LeakSanitizer sweep):** | Commit | Leak | Fix | |---|---|---| | `6121f6ad6` | `RadioModel::disconnectClientHandlesThen`: the `step` lambda captured its own `shared_ptr`, a cycle that leaked on every client-disconnect chain | Capture a `weak_ptr`; the in-flight `sendCmd` callback holds the strong reference | | `7577bab41` + `7f37cf045` | WDSP **patch 18**: `destroy_calcc()` never closed the five PureSignal `SemsPSCorr` semaphores. 86 leak blocks across 17 tests (every TX channel destroy) | Close them, and the exit event, only once the correction thread has signalled exit; on the 500 ms timeout they are not freed under a live thread. **This does not make the timeout path safe:** `destroy_calcc()` still frees `a`, which a late thread would use. That hazard predates this PR and is now #6179 | | `192e8f09a` | `phone_tx_filter_numeric_entry_test`: `sendEvent(…, new QFocusEvent)` leaked, since `sendEvent` never takes ownership | Stack event (test only) | **Why CI never reported these leaks:** the ASan lane's `ASAN_OPTIONS` include `halt_on_error=0`, and with that, a leak doesn't change a test's exit code. The test "passes" and `ctest --output-on-failure` throws its output away. Verified with `phone_tx_filter_numeric_entry_test`: it exits 0 with CI's options and 1 with plain `detect_leaks=1`. Fixing that is a `sanitizers.yml` change and is **not** in this PR. **Full local sweep (all 653 tests, leaks counted as failures):** 26 failed before these three commits. After them, the only leak left anywhere is whisper.cpp's ggml Vulkan backend registry, a process-lifetime static that only appears on a machine with a Vulkan GPU. Not ours, and CI has no GPU. The other non-leak failures are: the two known timeouts; `connection_panel_size` and `local_control_server`, which fail only on this machine with no sanitizer report and pass in CI; and `rtl_receive_pipeline` and `cw_pcm_consumer`, which fail intermittently (1 run in 3 serially, and under parallel load, respectively) with no sanitizer report. Those are flakes already on `main`, out of scope here. ## Constitution principle honored Principle VIII: every finding was reproduced before it was fixed. Each fix that could be checked locally was shown to fail without it and pass with it; the TSan-only fixes are proven by the branch dispatch. The two WDSP changes follow the vendoring rules: minimal, commented in place, and documented in `AETHERSDR-PATCHES.md` with what to keep on the next WDSP update. ## Test plan - [x] Local build passes: full tree, Debug, with CI's ASan + UBSan flags and `AETHER_SHARED_CORE=ON` - [x] **Red/green for both ASan fixes.** Reverted, all 4 tests reproduce CI's exact reports (stack-use-after-scope at `:95`, downcast at `qpointer.h:76`). With the fixes they pass 3/3. - [x] 71 WDSP, ANAN, TX, HL2 DSP, map and edited tests under ASan + UBSan. All pass apart from the 2 known 30 s timeouts. `weather_radar_loading_test` run directly: 44 passed, 0 failed, 26 s. - [x] **Red/green for the leak fixes.** In a full-suite LSan sweep on the same tree, before vs after the three leak commits: 86 `create_calcc` blocks, 5 `disconnectClientHandlesThen` blocks and the `QFocusEvent` leak, down to none. All the affected tests pass with leak detection on, `tx_operation_integration` with 421/421 assertions. Leaks from this machine only (fontconfig/pango/GTK, NVIDIA `cuInit`, ggml Vulkan) were suppressed locally, not in the repo. - [x] Sanitizers `workflow_dispatch` [37230031178](https://github.com/aethersdr/AetherSDR/actions/runs/37230031178) tested **`31e249c3e`**, the head before the leak fixes and the review fixes, **not** the final head. Zero sanitizer `SUMMARY:` reports in either lane, so every originally targeted test is green. The run still went red on the timeouts and on `rtl_capture_worker_test`. Its duplicate stickies (#6165, #6170) are closed into #6154 / #6156. - [x] Sanitizers `workflow_dispatch` on the **final head `7f37cf045`**, [37243767521](https://github.com/aethersdr/AetherSDR/actions/runs/37243767521): **zero** sanitizer `SUMMARY:` reports in either lane. ASan failed 2/656 and TSan 5/656, every one a known sanitizer-slowdown timeout. `gui_nested_lifetime_test` (the new VFO regression), `phone_tx_filter_numeric_entry_test`, `wdsp_channel_test` and `rtl_capture_worker_test` passed in both lanes. - [x] The same head **plus #6176**, so a leak fails its test, [37243769261](https://github.com/aethersdr/AetherSDR/actions/runs/37243769261), on the temporary branch `ci/integration-6173-6176` (since deleted): **no ASan, UBSan or LeakSanitizer report**, and the same timeouts only. Its stickies #6189 and #6191 are closed into #6154 / #6156. - [x] **VFO teardown red/green.** The new `vfoCollapsedLabelTeardownInEitherOrder` aborts on the `qpointer.h:76` downcast in its "label-dies-first" row with only the destructor fix. With the event-type-first check, all 10 cases pass, 3 of 3 runs. The 26 vfo/rx_applet/fm_filter/slice/lifetime tests pass. - [ ] Behaviour on a real radio: patch 16 is a TX-path change. A COMP-on filter-width change while keyed should be checked on hardware and a dummy load before it ships. - [ ] Existing tests pass (CI) ## Checklist - [x] Commits are signed (`docs/COMMIT-SIGNING.md`) - [x] No new flat-key `AppSettings` calls - [x] Code is clean-room - [x] No meter UI touched - [x] Documentation updated: `third_party/wdsp/AETHERSDR-PATCHES.md` (patches 16, 17) and the in-file rationale in `tests/sanitizers/tsan.supp`. No `CHANGELOG.md`. - [x] Not security-sensitive 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…d rtl_capture_worker_test in lockstep. Principle VIII. (#6208) ## Summary Closes #6201. Refs #6202, which stays open for the intermittent `DecoderAudioModel` race only. **With both changes, the Sanitizers lane is fully green for the first time:** [37341135089](https://github.com/aethersdr/AetherSDR/actions/runs/37341135089), run on integration commit `88314cb9a`, passed **ASan + UBSan 658/658 and TSan 658/658, with zero sanitizer reports**. Relation to this head (`c29509c34`): every test and harness file is identical to that run except `tests/tests.cmake`. Its one later change (review N1) only *widens* which builds count as instrumented, so the sanitizer lane, which builds with global flags, configures identically: ×4, the same 658 tests. The only other difference is the `BUILD-OPTIONS.md` row. ### 1. `bbd2649fb`: scale test timeouts in sanitizer builds After #6173 and #6176, the weekly Sanitizers lane had **no sanitizer reports**, but it stayed red on seven **timeouts**: tests finishing past limits sized for ordinary builds. None of them was a hang. `rtl_spectrum_averaging_test`, the only suspect because it printed nothing before timing out, passes cleanly in 37 s under ASan when run alone; it just prints everything at the end. `aether_retrofit_tests()` already gives every test a limit (its own, or the 300 s default). It now also multiplies that limit by **`AETHER_SANITIZER_TIMEOUT_SCALE`** (default **4**) when the build is instrumented: `AETHERSDR_SANITIZER` is set, or the C/C++ flags contain `-fsanitize=`, which is how `sanitizers.yml` builds. Ordinary builds keep every limit exactly as written, so a slow test still fails fast where it should. A non-integer scale, or a non-integer `TIMEOUT` in an instrumented build, fails the configure step. Review follow-ups: `f1bcbf103` adds the `BUILD-OPTIONS.md` Setting row for `AETHER_SANITIZER_TIMEOUT_SCALE`. `c29509c34` also detects per-configuration flags (`CMAKE_<LANG>_FLAGS_<CONFIG>`), so a build instrumented only through `CMAKE_CXX_FLAGS_DEBUG` scales too; a TIMEOUT isn't per-configuration, so a multi-config generator scales when any configuration is instrumented. It also trims the code comment to the rule itself. No workflow file is touched anywhere in this PR. ### 2. `a0594d5cf`: feed `rtl_capture_worker_test`'s device in lockstep The last TSan failure, **intermittent** (2 of 4 TSan runs, never under ASan): *"DC mode change discards old estimates and shows the actual first corrected window"*. The product is correct here: adoption resets the spectrum and DC history, and the backend drops frames from an unpublished revision. The test assumed a fast device thread. - `InjectedDevice::block()` only *queues* a block. When the device thread lags, which is what TSan's slowdown of the DSP does, queued blocks spill into the post-adoption window. The first corrected window then completes early, before the backend publishes the revision (so it is dropped), or window 2 lands first. - Two more checks made the same assumption: the queue-saturation precondition (200 callbacks within 3 s, which fails deterministically on a single core) and the parked-spectrum frame count. - **Fix, test-only:** the adoption steps feed one *consumed* block at a time and take the first accepted frame. The parked checks drain the queue first. The saturation precondition is bounded at 30 s, a failure bound only. - **Guard:** `RtlInjectedDevice` reads `AETHER_TEST_RTL_CALLBACK_DELAY_MS`, and a new registered `rtl_capture_worker_slow_device_test` runs the same binary with every callback 30 ms slow. A fast device thread can no longer be assumed by accident. ## Why ×4 These are the times on `main` (run 37280204924) against each test's original limit. TSan runs 1.1× slower than ASan at the median and up to about 9× for GUI tests. | Test | ASan | TSan | Limit | |---|---|---|---| | `rtl_spectrum_averaging` | >30 (timeout) | >30 (timeout) | 30 | | `weather_radar_loading` | >30 (timeout) | >30 (timeout) | 30 | | `audio_engine_rates` | 94 | >120 (timeout) | 120 | | `nr_stereo_independence` | 169 | >300 (timeout) | 300 | | `peripheral_auth_dialog` | 18 | >60 (timeout) | 60 | With ×4, the slowest of these in the branch run used 65% of its new limit (below). ## Constitution principle honored Principle VIII: a lane that's red for reasons unrelated to the code hides the next real finding. This makes the ASan lane green, so a new failure stands out. ## Test plan **Commit 2 (`rtl_capture_worker_test`):** - [x] **Red:** the *original* test source under `rtl_capture_worker_slow_device_test` fails with exactly the CI message, plus the saturation check. A per-callback delay sweep fails from 15 ms (saturation) and from 30 ms (DC). - [x] **Green:** the fixed source passes at 0, 5, 15, 30, 60 and 100 ms. Both RTL capture entries pass, along with the other four RTL tests that share the harness. 48 concurrent runs: 0 failures (was 15). 6 runs pinned to one core: 0 failures (was 6). - [x] **Full lane:** [37341135089](https://github.com/aethersdr/AetherSDR/actions/runs/37341135089). Both `rtl_capture_worker_test` and `_slow_device_test` passed under ASan and under TSan. **Commit 1 (timeouts):** - [x] **Per-configuration detection (`c29509c34`)**, by configure matrix (`rtl_capture_worker_test` / default TIMEOUT): Debug + `CMAKE_CXX_FLAGS_DEBUG=-fsanitize=address` → 80 / 1200 (was 20 / 300); plain Debug → 20 / 300; Release with only the Debug flags instrumented → 20 / 300; global flags → 80 / 1200; `AETHERSDR_SANITIZER=address` → 80 / 1200. - [x] **Configure-time:** read back each test's `TIMEOUT` with `ctest --show-only=json-v1`. In the ASan build all 654 tests are scaled ×4 (30→120, 60→240, 120→480, default 300→1200). A plain configure leaves all 654 unchanged. `-DAETHERSDR_SANITIZER=address` also triggers the scaling, and `AETHER_SANITIZER_TIMEOUT_SCALE=1.5` is rejected. Every test still has a `TIMEOUT`, so the existing retrofit assertion holds. - [x] **Full local ASan + UBSan suite at `-j16`**, with leaks failing tests: **no timeouts**. `rtl_spectrum_averaging` (47.5 s) and `weather_radar_loading` (35.4 s) would have failed their old 30 s limits under that load. That is the red/green. - [x] **Sanitizers `workflow_dispatch` on `bbd2649fb`** (commit 1 alone), [37319217237](https://github.com/aethersdr/AetherSDR/actions/runs/37319217237). Both lanes logged `test TIMEOUTs scaled x4`, and neither had a sanitizer report. - **ASan + UBSan: 657/657 passed.** This is the first green ASan lane. - **TSan: 656/657 passed.** The one failure is the known intermittent `rtl_capture_worker_test` assertion, tracked in #6202. It is not a timeout and not something this change can fix. - Every former timeout passed, in seconds, new limit in brackets: | Test | ASan | TSan | |---|---|---| | `rtl_spectrum_averaging` | 64 (120) | 32 (120) | | `weather_radar_loading` | 32 (120) | 78 (120) | | `audio_engine_rates` | 106 (480) | 199 (480) | | `nr_stereo_independence` | 204 (1200) | 313 (1200) | | `peripheral_auth_dialog` | 24 (240) | 123 (240) | - The duplicate sticky that run filed (#6207) is closed into #6202. - [ ] Existing tests pass (PR CI) ## Checklist - [x] Commits are signed - [x] Test infrastructure only; no product code, settings or UI touched - [x] No `CHANGELOG.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

ten9876 requested a review from a team as a code owner October 4, 2026 23:02

ten9876 self-assigned this Oct 4, 2026

ten9876 commented Oct 4, 2026

View reviewed changes

ten9876 removed their assignment Oct 4, 2026

aethersdr-agent Bot reviewed Oct 4, 2026

View reviewed changes

ten9876 mentioned this pull request Oct 4, 2026

fix(sanitizers): clear every ASan/UBSan/TSan report on the full-tree lane, incl. two WDSP races. Principle VIII. #6173

Merged

14 of 16 tasks

ten9876 commented Oct 4, 2026

View reviewed changes

aethersdr-agent Bot mentioned this pull request Oct 5, 2026

[sanitizer] ASan + UBSan weekly run failure (fp 033d48555465) #6189

Closed

ten9876 mentioned this pull request Oct 5, 2026

[sanitizer] TSan weekly run failure (fp eaf76ff70558) #6191

Closed

ten9876 merged commit 3342761 into main Oct 5, 2026
7 checks passed

ten9876 deleted the ci/asan-leaks-fail-tests branch October 5, 2026 07:51

ten9876 mentioned this pull request Oct 6, 2026

[sanitizer] TSan weekly run failure (fp 1869b913d6f3) #6202

Open

Navigation Menu

Repository navigation

ci(sanitizers): let a leak fail its test in the ASan lane. Principle VIII. - #6176

ci(sanitizers): let a leak fail its test in the ASan lane. Principle VIII.#6176
ten9876 merged 2 commits into
mainfrom
ci/asan-leaks-fail-tests

ten9876 commented Oct 4, 2026 •

edited

Loading

Uh oh!

ten9876 left a comment

Uh oh!

aethersdr-agent Bot left a comment

Uh oh!

ten9876 commented Oct 4, 2026

Uh oh!

ten9876 left a comment

Uh oh!

ten9876 commented Oct 5, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	old `ASAN_OPTIONS`	this PR's
leak only	exit 0, SUMMARY printed	exit 1
heap-buffer-overflow	exit 1	exit 1

Conversation

ten9876 commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Constitution principle honored

Test plan

Checklist

Uh oh!

ten9876 left a comment

Choose a reason for hiding this comment

Scope

Blockers

Non-blocking nits

Verification and attacks

CI and recommendation

Uh oh!

aethersdr-agent Bot left a comment

Choose a reason for hiding this comment

Issue fit

Scope

Blockers

Nits (non-blocking)

What I tried to break

Recommendation

Uh oh!

ten9876 commented Oct 4, 2026

Uh oh!

ten9876 left a comment

Choose a reason for hiding this comment

Scope

Blockers

Nits (non-blocking)

Verified empirically

Uh oh!

ten9876 commented Oct 5, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ten9876 commented Oct 4, 2026 •

edited

Loading