Repository navigation
fix(ci): fit the full test tree on the hosted runner again. Principle VIII. - #6152
Merged
Merged
Conversation
… VIII. Full Suite died at link 4934/4976 with "No space left on device" on a325fb9. ci.yml's build job builds the same tree and only survives because #5924 gave it a step that frees 18 GB of host toolchains. Underneath both, ~250 test executables each link a static aethercore and carry their own copy of its debug info: ~325 MB apiece at -g. - ci.yml build + full-suite.yml: RelWithDebInfo at -g1 (line tables only). Measured locally on core-linking tests: 325 MB -> 120 MB each, libaethercore.a 496 -> 114 MB. Backtraces keep file:line. Both jobs change together, because Full Suite restores the build job's ccache. - full-suite.yml: the same host-toolchain mounts and Free runner disk step as ci.yml's build job, plus a df after the build. - full-suite.yml: a Configure or Build failure now files the tracking issue. ctest's skipped output reads as null, which `exit != '0'` treats as 0, so the red run filed nothing. Same pattern as #6136. - sanitizers.yml: TSan moves to the shared core. With the Qt 6.12 :tsan image it now builds, and its -g3 static tree filled the runner (100%, at link 4438/4854, run 37176807431). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ten9876
commented
Oct 4, 2026
ten9876
left a comment
ten9876
left a comment
Collaborator
Author
There was a problem hiding this comment.
Issue fit: the diff addresses the three stated failures: oversized ordinary CI builds, TSan's duplicated static core, and missing Full Suite reports when Configure/Build fails. There is no closing issue; #6148 supplies the TSan failure context. I verified the original ENOSPC failures in runs 37181657060 and 37176807431. The workflow logic checks out; the hosted-runner disk-capacity claim remains provisional while the new runs are building. Reviewed head bdd9ae860598770c77a5a569b1ccc0c3ac0deb84.
Scope
| File | Change | Claimed in body? | Verdict |
|---|---|---|---|
.github/workflows/ci.yml |
Linux RelWithDebInfo C/C++ flags use -g1 |
Yes | In scope |
.github/workflows/full-suite.yml |
Matching flags, host-toolchain cleanup, post-build disk reading, Configure/Build failure reporting | Yes | In scope |
.github/workflows/sanitizers.yml |
Enable existing shared-core option for TSan | Yes | In scope |
Everything in the diff is explained by the stated problem. One commit, authored 2026-10-04; no unrelated commit, public protocol surface, application default, UI preference, test deregistration, or gate expansion. These are CI fixes permitted by GOVERNANCE.md's AI Contributors section; the shared-core build option already exists specifically for sanitizer CI. No scope-checklist contradiction found.
Blockers
None found.
Nits — non-blocking
- Complete the hosted-runner evidence before treating the capacity fix as demonstrated —
.github/workflows/ci.yml:271,.github/workflows/full-suite.yml:112,.github/workflows/sanitizers.yml:73. At review time CI's Linux build, Full Suite run 37208804048, and both sanitizer jobs in 37208806037 were still building. Please update Validation with the completed build results and Full Suite/TSan post-build disk readings. I have not independently reproduced the exact 325 MB → 120 MB / approximately 50 GB measurements, or verified the complete TSan shared-core link/runtime. A later assertion failure should be distinguished from failure of this PR's disk/link fix.
Verification and attacks that held up
- Preflight: only the three workflows change. No added/modified test sources, socket-owning targets, or synthetic peers. No test suite or GUI instance was launched.
- Executed the actual Full Suite Build shell body under
sh -e, injectingcmakeexit codes 0, 2, and 137. Every code survived unchanged anddfran on every path. Executed both configure shell bodies with a recordingcmakestub: identical argv, correctly quoted C/C++ flags; failing configure remains nonzero. - Executed the actual new report shell body for Configure, Build, and repeated Build failures. Outputs were
fingerprint=8c86ae9408ddfor Configure andfingerprint=b83f0666e782for Build; repeated Build reused its fingerprint. The report and artifact log matched. Executed the tracking-issue JavaScript with a mock GitHub API: the first report creates an issue and the repeat appends to the matching issue. No API writes were made by these tests. Actions scheduling/condition evaluation was inspected, not reproduced by this shell harness; the explicit failure/status checks preserve the report path after an upstream failure. - A separate GCC smoke binary built with
-O2 -g1 -DNDEBUGretained function/file/line resolution viaaddr2lineand was smaller than the same source with-g(22,696 vs 44,264 bytes). This validates the compiler-flag premise, not the application's exact size estimates. Read the existing shared-core CMake path: PIC is enabled and sanitizer flags remain supplied for configure and build. python tools/check_ci_test_gate.pypassed (7 frozen selected tests);python tools/check_test_registration.py --strictpassed. Prior Full Suite log confirms ENOSPC near 4934/4976; prior TSan log confirms ENOSPC andoverlay 145G 145G 163M 100% /near 4438/4854.
CI freshness: current main was f4e463c2 (2026-10-04 09:02:58Z), older than all head-run creation times (earliest 14:18:56Z; CI pull_request run 14:18:58Z). No stale-main timing gap observed. Windows, macOS, Static checks, and sanitizer configure check passed; Linux build was pending. CI tests the PR merge result, whereas the two manually dispatched validation workflows test the branch head.
The separate automated code-review skill and local actionlint executable were unavailable; this was a manual audit plus the focused checks above. I did not independently rerun the body's actionlint claim. No automation-bridge session: this is a CI-only change, so driving the demo would not test its runner-capacity or workflow-reporting claims. No operator application or radio was touched.
Recommendation: Approve with nits once the outstanding runner evidence and required checks complete successfully. No corrective code change identified. This is a COMMENT review, not an approval or authorization to merge before validation finishes.
Collaborator
Author
|
On the review's nit (complete the runner evidence):
The Validation section of the body now has these numbers. |
This was referenced Oct 4, 2026
14 of 16 tasks
ten9876
added a commit
that referenced
this pull request
Oct 5, 2026
…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>
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
1 participant
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.
Problem
Full Suite on
mainis red. Run 37181657060 ona325fb99e(#5924) died at link 4934/4976 withfinal link failed: No space left on device. The previous commit's run was green, so the tree is right at the runner's limit.buildjob builds the same tree. It only survives because RTL M1: confirmed receive state, free capture browsing and finer zoom #5924 gave it a step that frees 18 GB of host toolchains (84 → 102 GB free). That's about 50 more core-linking tests of headroom before the PR gate hits the same wall.:tsanimage from fix(ci): build TSan QtMultimedia with uninstrumented host tools #6123 it got far enough to build for the first time, then filled the runner:overlay 145G 145G 163M 100%at link 4438/4854 (run 37176807431, [sanitizer] TSan weekly run failure (fp b83f0666e782) #6148).null. GitHub's expressionexit != '0'coercesnullto 0, so every report step was skipped.Underneath all three: ~250 test executables each link a static
aethercoreand carry their own copy of its debug info.Fix
ci.ymlbuild job +full-suite.yml: configure RelWithDebInfo at-g1(line tables only), viaCMAKE_{C,CXX}_FLAGS_RELWITHDEBINFO. Backtraces keep file:line; only debugger-level locals and types are dropped. Both jobs change together because Full Suite restores the build job's ccache, so the firstmainrun after merge builds cold and the next is warm again.full-suite.yml: the same host-toolchain mounts and "Free runner disk" step as ci.yml's build job, and adfafter the build, including when it fails.full-suite.yml: a "Report a suite that did not build" step runs only when Configure or Build failed. It writes the report with a stable per-step fingerprint, and the artifact and tracking-issue steps also run on that path. Same pattern as ci(sanitizers): link the ASan lane against a shared core so it fits on disk. Principle VIII. #6136.sanitizers.yml: TSan'sshared_coregoes to"ON", the change the ASan lane proved in ci(sanitizers): link the ASan lane against a shared core so it fits on disk. Principle VIII. #6136.Validation
Measured locally (GCC, this tree,
aethercoreplus four core-linking tests):-g(today)-gsplit-dwarf-g1libaethercore.aAt ~250 core-linking executables, that's roughly 50 GB off a tree that needs a little over 84 GB.
actionlintis clean onfull-suite.ymlandsanitizers.yml. It reports two errors inci.yml(runner.workspace, lines 451 and 841), which were already onmainand are untouched here.On the runner:
-g1objects exist yet. The firstmainbuild after merge saves them, and later runs are warm again.build, the required PR check on this head: green, so the-g1configure works on the build job.buildfixes proven above.🤖 Generated with Claude Code