[jcmerg]

Summary

Closes #6179.

destroy_calcc() (vendored WDSP, third_party/wdsp/upstream/calcc.c) waited at most 500 ms for the PureSignal correction thread doPSCorrChange() to exit. After that it freed the CALCC regardless, and destroy_txa() then freed the iqc stage and deleted calcc.cs_update. The thread reads all of these. A thread still in calc() (the spline fit) or in correction-file I/O past the 500 ms went on using freed memory.

Reading the code turned up a second hazard that the issue does not mention: the six util splines were freed before the wait, under a thread that may still be in PSSaveCorr/PSRestoreCorr work using exactly those buffers.

The thread is created for every TX channel. Today it only idles, because nothing in src/ calls the PS API. PureSignal (planned for the HL2) would make this path live, so it needs fixing first.

Change: WDSP patch 22

• create_calcc() records whether _beginthread() started the thread (corrThreadStarted). A thread that never started is not waited for. That is the reason patches 4 and 9 bound their waits, and this keeps the unbounded wait safe from it.
• destroy_calcc() waits for hCorrChangeExited:
  • The four work semaphores are drained first, so the exit token is the next one the thread takes once its current work ends.
  • Each 1 ms pass clears iqc's busy bit, because SetTXAiqcStart()/SetTXAiqcSwap() spin on it for a DSP worker that CloseChannel() has already stopped.
  • No time limit only when patch 4's worker wait succeeded (mainExited == mainGen). Those setters take ch[].csDSP, which is free only once the worker has exited. If patch 4's wait fell through its cap, a wedged worker may still hold csDSP; a bound of 500 one-millisecond passes then stands (500 ms on POSIX, up to ~8 s at Windows' default timer tick), so that path (already undefined on main) is no worse than before: the handles stay open there, as patch 18 had them, and a is freed as on main.
  • A WAIT_FAILED return ends the wait instead of spinning, and a NULL exit event is not waited on.
• After the wait: patch 18's handle closes run only once the exit event was seen, or when the thread never started. The six util spline frees follow the wait, so on the normal path they come after the thread is gone.

What can still stall teardown: only the thread's own work. calc() is bounded CPU work. Correction-file I/O lasts as long as the file system takes. AetherSDR calls neither PSSaveCorr nor PSRestoreCorr today, and that is documented in AETHERSDR-PATCHES.md (patch 22, plus patch 18's bullet and note updated).

Evidence

New test wdsp_calcc_teardown_test:

• It holds the correction thread in PSRestoreCorr()'s fopen() on a FIFO with no writer, destroys the TX channel on another thread, and opens the writer 1 s later.
• It queues the restore through WdspChannel::restorePureSignalCorrectionForTest(), never a WDSP symbol of its own, so it links under AETHER_SHARED_CORE=ON (built and green locally in both the static and the shared-core configuration). The writer opens non-blocking: a thread that never opened the FIFO fails on ENXIO in ~1 s rather than hanging to the ctest timeout (mutated: restore call removed → 3 FAIL rows, 1.3 s).
• Red on unpatched calcc.c (origin/main): "teardown is still waiting 1 s later, past upstream's 500 ms limit". CloseChannel() returned while the thread was still blocked.
• Red with workerGone forced to 0: same row. This proves the normal path takes the unbounded branch.
• Green with the patch, five consecutive runs.
• POSIX FIFO, so it skips on Windows.
• The writer is held open for 200 ms with a junk line. An open-and-close at once can leave a BSD FIFO reader waiting on a writer generation it never sees, which hung one local run before this was added.

Also green locally (macOS 26, Qt 6.12.0): wdsp_channel_test, wdsp_allocation_scope_test, hl2_tx_gate_test, hl2_noise_blanker_test, hl2_apf_agc_off_test. Static checks check_test_registration and check_ci_test_gate are green too.

What the test does not cover: it pins the wait, not the reordering of the frees after it. With the thread parked in fopen() nothing is touched until release, so the freed-memory half is the sanitizer lanes'.

Not run: the ASan/TSan lanes locally, Windows, Linux. No ci.yml -R filter names wdsp_calcc_teardown_test, so CI's green does not include it; the red/green results above are local.

CHANGELOG.md untouched.

[aethersdr-agent]

Reviewed against the PR head checkout, #6179, and the surrounding WDSP teardown chain (channel.c, TXA.c, iqc.c, port/wdsp_port.c). Everything below is reasoned from source; I have no build and ran nothing. CI on eb55b0c is green on all five checks (build, check-macos, check-windows, Static checks, Sanitizer option configures).

1. Issue fit

Yes, on the normal path. #6179 asks that destroy_calcc() stop freeing a and its members under a correction thread that outlived the 500 ms wait, and lists two acceptable directions ("wait without a time limit, accepting a teardown stall" / "hand a to the thread to free"), explicitly not decided. The PR takes direction 1 and does it carefully: the four work semaphores are drained first, so WaitForMultipleObjects' lowest-index rule makes SemsPSCorr[4] the thread's next token; the six util spline frees — which #6179 did not name but which upstream really did free before the wait, under a thread in case 1/2 — move after it; and the ns_free / desize_calcc / DeleteCriticalSection(cs_update) / _aligned_free(a) run only after the exit event. I verified destroy_calcc precedes destroy_iqc in destroy_txa (TXA.c:478-479), so the b->busy deref is on live memory, and post_main_destroy deletes ch[].csDSP after destroy_main (channel.c:218-222), so csDSP is a valid object during the wait. corrThreadStarted is sound: malloc0 zeroes the struct, there is no early return in create_calcc, =/!= precedence gives the intended a->... = (result != -1), and the POSIX shim returns (uintptr_t)-1 on both failure paths (port/wdsp_port.c:304,319).

What is unaddressed is a ruling on the trade the PR makes — see finding 1.

2. Scope

File	What it changes	Claimed?	Verdict
third_party/wdsp/upstream/calcc.c	corrThreadStarted field + set; destroy_calcc() reordered to drain → unbounded exit wait → closes → util frees	Yes (title, body)	In scope
tests/wdsp_calcc_teardown_test.cpp	New socket-free POSIX test holding the thread in PSRestoreCorr	Yes	In scope
tests/tests.cmake	Registers the new target, TIMEOUT 120	Yes	In scope (comment placement nit below)
third_party/wdsp/AETHERSDR-PATCHES.md	Patch 22 section; patch-18 bullet and §Patch 18 updated	Yes	In scope

Nothing in the diff is unexplained by the issue. No new public surface, no settings keys, no capability changes, no CHANGELOG.md entry. §3 asks me to quote deleted symptom-comments: patch 18's block went with the guard it described, including

// On the 500 ms timeout the thread may still be waiting on
// them, so they are not freed under it. That does NOT make the timeout path
// safe: the rest of this function still frees a, which a late thread
// would use.

That removal is correct rather than a regression — the timeout the comment warned about no longer exists, so the closes can be unconditional, and the replacement comment at calcc.c:1095-1099 says why. The patch-18 bullet and §Patch 18 were both amended to point at patch 22, which is the right bookkeeping.

3. Blockers

None. One item needs a maintainer ruling rather than a change:

1. third_party/wdsp/upstream/calcc.c:1095-1104 — the comment's csDSP justification is conditional, and the failure mode it leaves is an unbounded CloseChannel() hang. The comment asserts:

   	// No time limit: with the work semaphores drained the exit token is its
   	// next one. iqc's busy bit is cleared on each pass, since SetTXAiqcStart/
   	// Swap spin on it for a DSP worker that is already gone.

   and §Patch 22 adds "ch[].csDSP, which those setters take, is free at this point (patch 9)."

   csDSP being free depends on the wdspmain worker having exited, and patch 4's own comment in channel.c:153-162 says that wait is deliberately capped and may fall through:

   // BOUNDED, because upstream ignores thread-creation failure and an
// unbounded wait would then hang CloseChannel() forever; falling through
// after the cap restores upstream's behaviour exactly, no worse.

   A worker starved past the 1000 ms cap inside dexchange() still holds csDSP. The correction thread in case 0 (SetTXAiqcEnd, iqc.c:286) or case 3 (calc() → SetTXAiqcStart/Swap, iqc.c:258) then blocks in EnterCriticalSection(&ch[channel].csDSP), hCorrChangeExited is never set, and the new while never terminates. Patch 22 therefore introduces exactly the unbounded CloseChannel() wait patch 4 declined — the busy-bit unwedge does not help, because the block is on the critical section, not on busy.

   Two things narrow it, and I want to be fair about both: that path is already undefined on main (post_main_destroy deletes csDSP under a live holder), so this converts one bad outcome into another rather than breaking a clean path; and since nothing in src/ calls PSSaveCorr/PSRestoreCorr (I grepped: the only reference outside WDSP is the new test), cases 1 and 2 are unreachable in production, leaving calc(), which is bounded CPU work as the PR says.

   So this is Needs maintainer decision, not a blocker: #6179 offered the stall-accepting direction and left it open, and the trade (a possible permanent teardown hang behind a wedged worker, versus a bounded use-after-free) is the maintainer's to ratify. If it is ratified, the csDSP sentence in both the code comment and §Patch 22 should be qualified — "free whenever patch 4's bounded wait succeeded" — rather than stated flatly, since a future reader will rely on it. If not, #6179's second direction (hand a to the thread) avoids the hang.

4. Nits (non-blocking)

• third_party/wdsp/upstream/calcc.c:1102 — a WAIT_FAILED handle hot-spins forever. The POSIX shim returns WAIT_FAILED immediately for a NULL handle (port/wdsp_port.c:207-210), and CreateEvent's result at calcc.c:1082 is unchecked (pre-existing). Under the old if (... == WAIT_OBJECT_0) that merely skipped the closes; the new loop never sleeps on that return, so it becomes a 100 %-CPU infinite spin. Cheap to close:

  	if (a->corrThreadStarted && a->hCorrChangeExited != NULL)
  	{
  		DWORD waitstat;
  		while ((waitstat = WaitForSingleObject(a->hCorrChangeExited, 1)) != WAIT_OBJECT_0)
  		{
  			if (waitstat == WAIT_FAILED) break;
  			InterlockedBitTestAndReset(&b->busy, 0);
  		}
  	}

• tests/wdsp_calcc_teardown_test.cpp:66-80 — the test covers the wait, not the reordering, which is the half that is the actual use-after-free. Its only assertion is "teardown had not returned 1 s in". Move the six util spline frees back above the wait and it still passes; keep the unbounded wait but free a before it and it still passes, because the thread is parked in fopen and touches nothing until the writer opens. Seeing the freed-memory half needs the sanitizer lane — which is also what §"Drop any local patch" already instructs ("run the lifecycle test under AddressSanitizer on every supported platform"). Canon routes lifetime behaviour there, so this is worth naming rather than withholding a merge for.
• No CI job runs this test. Every ctest step in ci.yml is -R-filtered (lines 819-820, 832, 1174, 1189) and none of the filters names it, so the "Reproduced before fixing" claim rests entirely on the author's local run. Worth stating in the body as such.
• tests/tests.cmake:985-992 — the new block orphans the comment above it. It lands between # Socket-free shared-pool admission and injected receiver lifetime tests. These / # foundations are compiled/tested even when the optional RTL USB driver is off. and wdsp_channel_reservation_test, which is what that comment describes. Moving the four new lines (and their own two-line comment) below the reservation block restores the pairing.
• tests/wdsp_calcc_teardown_test.cpp:4 and :65 say the thread is held in fopen(fifo, "rb"); ReadCorrectionFileV2 actually opens fopen(filename, "r") (nurbs_spline.c:1058). Blocking either way — the premise holds — but the quoted mode is wrong.
• third_party/wdsp/AETHERSDR-PATCHES.md:576 still reads "The ten are different shapes" above a list that now runs to twelve entries. Pre-existing staleness, one worse here.

5. What I tried to break

• The "exit token is its next one" claim. doPSCorrChange waits with bWaitAll = FALSE, so WaitForMultipleObjects returns the lowest signalled index — meaning a 0-3 token posted after the drain would starve index 4 indefinitely. I grepped every ReleaseSemaphore on SemsPSCorr: indices 0 and 3 are released only from inside calcc.c's own state machine, 1 and 2 only from PSSaveCorr/PSRestoreCorr, which nothing in src/ calls. With the DSP worker already quiesced by pre_main_destroy, I could not construct a post-drain release. The claim holds.
• corrThreadStarted being read uninitialised. create_calcc allocates with malloc0, has no early return before line 1083, and _beginthread's POSIX shim returns (uintptr_t)-1 on both the malloc and pthread_create failure paths. Nothing left stale.
• A dangling b in the new busy-clear loop. destroy_txa calls destroy_calcc before destroy_iqc, so txa[].iqc.p is live for the whole wait.
• cs_update deleted under the thread. Cases 1 and 2 take it on their error path; the DeleteCriticalSection(&txa[a->channel].calcc.cs_update) is after the wait, so that ordering is right.
• The csDSP reachability chain — this is the one that did produce a finding (1); patch 4's bounded fall-through is the gap.
• The test's premise, adversarially. If mkfifo fails, fopen returns immediately and the test fails loudly at the 1 s check rather than hanging; restore_file is char[256] against a $TMPDIR-derived path, so the unchecked while (buf[i++] = *p++) copy in PSRestoreCorr has headroom on a normal macOS/Linux temp dir. The 100 ms settle before teardown and the reader/writer rendezvous both look sound.
• Could not check: anything at runtime — no build, no ctest, no sanitizer lane, and no Windows compile of the new test (check-windows builds only AetherSDR plus four test targets, and this one is not among them; its body is entirely inside the #else, so the risk is low).

6. Recommendation

Needs maintainer decision. The code is correct on the normal path, the reordering genuinely closes what #6179 describes (and a little more — the util splines the issue missed), the documentation is thorough and honest about what can still stall, and the test does reproduce the symptom it claims. The one thing a reviewer cannot settle is the trade the PR picks: #6179 left the direction undecided, and the unbounded wait is the thing patch 4's own comment refused for CloseChannel(). Concrete next step: rule on direction 1 versus #6179's "hand a to the thread" alternative; if direction 1 stands, qualify the csDSP-is-free sentence in calcc.c:1099 and §Patch 22, and fold in the WAIT_FAILED break. The test-strength and tests.cmake items are tidy-ups either way.

Nice, surgical patch on a nasty piece of vendored teardown — thank you, and thanks especially for the patch-18 cross-references and the explicit "what can still stall teardown" paragraph; both made this much faster to audit.

---

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

[jcmerg]

Thanks. Finding 1 was right: my csDSP is free sentence held only when patch 4's wait succeeded.

Addressed in 21578206, without needing the ruling to go one way:

• Finding 1: the wait is unbounded only when mainExited == mainGen, i.e. patch 4 saw the DSP worker exit and csDSP is free. If that wait fell through its cap, upstream's 500 ms bound stands, so the path that is already undefined on main is no worse and cannot hang CloseChannel(). The code comment and §Patch 22 now say exactly this. Forcing workerGone to 0 turns the test red on the same row as unpatched main, so the normal path demonstrably takes the unbounded branch.
• WAIT_FAILED nit: taken as suggested, plus a NULL-event guard.
• Test strength: agreed, it pins the wait and not the free ordering. The body and §Patch 22 now say the freed-memory half belongs to the sanitizer lanes, and that no ci.yml filter runs this test.
• tests.cmake placement, the "rb" → "r" comment, and "The ten are": fixed.

The remaining maintainer question is narrower now. On the normal path it is direction 1 (wait for the thread); the wedged-worker path keeps today's behaviour. The "hand a to the thread" alternative would not cover txa[].iqc and cs_update, which destroy_txa() frees regardless.

[K5PTB]

Adversarial review: #6261 at 21578206

I ran this per upstream .claude/skills/pr-review against a fresh clone of aethersdr/AetherSDR. The PR head was merged onto current main (d6f6a4f0, 40 commits past this branch's base 248bccd2). I built it twice on macOS 26 / arm64 with Qt 6.12.0: once static, once with AETHER_SHARED_CORE=ON. Results below say which ones I observed and which I only reasoned out.

Issue fit

Yes, on the normal path. #6179 asks that destroy_calcc() stop freeing a under a correction thread that outlived the 500 ms wait. It leaves the direction open. The PR takes direction 1 (wait for the thread) and does it carefully: drain, then wait, then close, then free. It also catches the six util spline frees, which the issue missed. The wedged-worker fallback added in 21578206 answers the earlier review's finding 1. I reproduced the PR's own red/green claims (see "What I tried to break").

The one blocker is not in the fix. It is in how the new test reaches WDSP, and it only appears once this branch meets current main.

Scope

File	What it changes	Claimed?	Verdict
third_party/wdsp/upstream/calcc.c	corrThreadStarted; destroy_calcc() drains, waits (unbounded only when workerGone), closes, then frees util	Yes	In scope
tests/wdsp_calcc_teardown_test.cpp	New FIFO-parked teardown test	Yes	In scope; see blocker 1
tests/tests.cmake	Registers it, TIMEOUT 120	Yes	In scope
third_party/wdsp/AETHERSDR-PATCHES.md	§Patch 22; patch 18 bullet and note; "The ten are" fixed	Yes	In scope

Everything in the diff is explained by the issue. There is no new public surface and no CHANGELOG.md entry. Both commits are dated 2026-10-07 and both are on-topic. The removed patch-18 comment and its == WAIT_OBJECT_0 guard go along with the timeout they described, so that removal is not a regression.

Blockers

1. tests/wdsp_calcc_teardown_test.cpp:27,68: the test calls WDSP directly. That breaks the shared-core link, so both weekly sanitizer lanes would stop at Build.

extern "C" void PSRestoreCorr(int channel, char* filename);
...
PSRestoreCorr(channel->channelId(), fifo.data());

The target links only aethercore (tests/tests.cmake:996). Under AETHER_SHARED_CORE=ON, libaethercore keeps WDSP's symbols hidden, so the link fails. I reproduced this on the PR merged onto current main:

$ ninja -j4 -C build-shared wdsp_calcc_teardown_test wdsp_channel_test
[592/595] Linking CXX executable wdsp_calcc_teardown_test
FAILED: [code=1] wdsp_calcc_teardown_test
Undefined symbols for architecture arm64:
  "_PSRestoreCorr", referenced from:
      _main in wdsp_calcc_teardown_test.cpp.o
ld: symbol(s) not found for architecture arm64

As a control, wdsp_channel_test links in the same build dir. Both lanes in .github/workflows/sanitizers.yml set shared_core: "ON" (lines 42 and 75) and build the whole tree. This is the exact failure #6309 fixed yesterday (#6283 ASan+UBSan, #6284 TSan: "Both weekly sanitizer lanes stopped at Build, at link, so no tests ran").

The rule it breaks, in canon on main:

• CMakeLists.txt:158-161: "A test that calls WDSP directly while linking only aethercore links against the static core and fails here; go through core/dsp/WdspChannel.h."
• tools/check_wdsp_test_links.py docstring: "A test reaches WDSP only through core/dsp/WdspChannel.h … So the fix is a WdspChannel *ForTest hook, never a link."
• src/core/dsp/WdspChannel.h:482-484: "Tests reach WDSP only through this class."

Static checks will stay green regardless. The branch's checks ran on 2026-10-07 at 20:51Z, before #6309 existed. On the merged tree, python3 tools/check_wdsp_test_links.py prints WDSP test boundary OK. The checker keys on #include and documents this gap: "a hand-written extern "C" declaration gets past rule 1". Per-PR CI builds only the static core, so nothing gating this PR can catch it.

This also undercuts the body's coverage plan. The body says "the freed-memory half is the sanitizer lanes'", but the sanitizer lanes cannot build this test at all.

A fix in the shape #6309 used. PSRestoreCorr is not in aether_wdsp.h yet, so it needs a declaration there too:

third_party/wdsp/include/aether_wdsp.h, beside the other test entry points (before the closing extern "C"):

void PSRestoreCorr(int channel, char* filename);

src/core/dsp/WdspChannel.h, after synchronizeWorkerForTest():

    // Test only (#6179, WDSP patch 22): queue PSRestoreCorr(path) on this
    // TX channel's PureSignal correction thread, on the WDSP the core drives.
    void restorePureSignalCorrectionForTest(const char* path) const noexcept;

src/core/dsp/WdspChannel.cpp, after WdspChannel::synchronizeWorkerForTest():

void WdspChannel::restorePureSignalCorrectionForTest(const char* path) const noexcept
{
    // PSRestoreCorr only copies the name; WDSP's signature is just not const.
    PSRestoreCorr(m_channelId, const_cast<char*>(path));
}

tests/wdsp_calcc_teardown_test.cpp: drop the extern "C" line and call:

    channel->restorePureSignalCorrectionForTest(fifo.c_str());

Please confirm it with a -DAETHER_SHARED_CORE=ON build of wdsp_calcc_teardown_test. No per-PR job will.

Nits (non-blocking)

• third_party/wdsp/upstream/calcc.c:1110: the per-pass busy clear is the guard that keeps the unbounded wait from hanging, and nothing tests it. Mutation M4 below deletes that line and the test stays green twice. The reason is that the junk line sends the thread down ReadCorrectionFileV2's error branch, which never reaches SetTXAiqcStart()'s while (busy) Sleep(1). Reasoned, not run: without the line, a restore that succeeds after the drain spins on busy with the DSP worker gone. hCorrChangeExited never fires, and CloseChannel() hangs for good. Before this PR the same spin was capped at 500 ms. Writing a valid V2 correction file (checksum included) through the FIFO, in place of the junk line, would drive that path and pin the line. Same coverage boundary the body already names for the frees, one guard further.
• tests/wdsp_calcc_teardown_test.cpp:69,83: if the 100 ms settle loses its race, the failure is a 120 s hang, not a FAIL line. Reasoned, not observed: suppose the correction thread has not yet taken SemsPSCorr[2] when the closer runs. Then destroy_calcc() drains it, the thread exits on token 4, and the 1 s check fails. After that, ::open(fifo, O_WRONLY) blocks forever, because no reader ever opens the FIFO, and ctest kills the test at TIMEOUT 120. Opening with O_WRONLY | O_NONBLOCK and reporting ENXIO as "the correction thread never opened the FIFO" would turn that into a clear failure. It never happened in my 15 runs.
• third_party/wdsp/upstream/calcc.c:1106-1108: on Windows, the fallback cap is ~7.8 s, not "upstream's 500 ms". Reasoned: the cap counts 500 passes of WaitForSingleObject(h, 1). Under Windows' default ~15.6 ms timer resolution, each pass takes a full tick, and nothing in src/ or third_party/wdsp calls timeBeginPeriod. Patch 4's Sleep(1) × 1000 has the same shape, so this only bites on the wedged-worker path that is already undefined. But §Patch 22 and the code comment say "500 ms". Either use a time-based bound or say "500 passes".
• CI is stale. The earliest run on 21578206 was created on 2026-10-07 at 20:51Z. main is 40 commits ahead, and fix(tests): tests reach WDSP only through WdspChannel test hooks, and Static checks enforce it. Principle VIII. #6309 is one of them. main is strict: false, so nothing forces a rerun.

What I tried to break

• The PR's red/green claims. I built wdsp_calcc_teardown_test in the static build and mutated calcc.c, deleting calcc.c.o before every rebuild:

  • PR head: 5/5 green.
  • M1, merge-base calcc.c (unpatched): 2/2 red on teardown is still waiting 1 s later, past upstream's 500 ms limit.
  • M2, workerGone forced to 0: 2/2 red, same row.
  • M3, util frees moved back above the wait: 2/2 green. This is expected; the body already concedes it.
  • M4, per-pass busy clear removed: 2/2 green (nit 1).
  • Restored tree: green. ctest -R '^(wdsp_channel_test|wdsp_calcc_teardown_test)$' reports 100% of 2 passed.

  So the body's red/green statements are accurate.

• "Exit token is the next one." doPSCorrChange uses WaitForMultipleObjects(…, FALSE, …), which returns the lowest signalled index. After the drain, indices 0 and 3 can only be posted by the calcc state machine on the DSP worker, which pre_main_destroy() has already stopped. Indices 1 and 2 come only from PSSaveCorr/PSRestoreCorr, which nothing in src/ calls. I found no post-drain release. The claim holds.

• workerGone semantics. It is computed once, before the loop, from patch 4's generation counters (channel.c:163-165, main.c:43,81). A channel whose worker never started reads 0 == 0, so it is unbounded, which is correct because csDSP is free. A worker launch that failed leaves mainExited one generation behind, so it is bounded, which is harmless. post_main_destroy() deletes csDSP only after destroy_main(), so the setters' EnterCriticalSection is on a live object during the wait.

• Lifetime of what the loop touches. destroy_txa() calls destroy_calcc() before destroy_iqc() (TXA.c:478-479), so b->busy is live. cs_update, which cases 1 and 2 take on their error path, is deleted after the wait. Under the POSIX shim, a NULL hCorrChangeExited returns WAIT_FAILED (wdsp_port.c:207), and both the guard and the break cover it.

• Static checks on the merged tree: check_wdsp_test_links.py passes (blocker 1 explains why that is not enough).

• Not done. The automated code-review pass was not run; this is a manual pass only. I did not drive the app through the automation bridge: this is headless WDSP teardown, and the demo SimBackend has no WDSP TX channel or PureSignal path to exercise. I did not run the ASan/TSan lanes, Windows, or Linux. Everything ran in an isolated scratch clone; no app instance was launched.

Recommendation

Request changes. The fix itself is sound, and its red/green evidence reproduces. But the test as written breaks both weekly sanitizer lanes at link on current main, the regression #6309 just fixed. None of the checks gating this PR can see it. The WdspChannel hook above clears it. After that, I'd suggest rebasing onto main so CI covers #6309, plus a shared-core build of this one target. The nits can be taken or left.

Thanks for the careful patch-18/22 bookkeeping and the explicit "what can still stall teardown" paragraph. Both made this much quicker to audit.

[jcmerg]

Thanks. The blocker was right: the hand-written extern "C" got past the checker and would have broken both sanitizer lanes at link.

Addressed in aa80a5fb, rebased onto main (ca5e0634, includes #6309):

• Blocker 1: taken as you shaped it. PSRestoreCorr is declared in aether_wdsp.h, the test goes through WdspChannel::restorePureSignalCorrectionForTest(), and the extern line is gone. Built wdsp_calcc_teardown_test with -DAETHER_SHARED_CORE=ON and in the static build on macOS 26 / arm64, Qt 6.12.0: links in both, 3/3 green in each. check_wdsp_test_links.py passes.
• FIFO race → 120 s hang: the writer now opens with O_WRONLY | O_NONBLOCK, and ENXIO is reported as "the correction thread has the FIFO open for reading". Mutated by removing the restore call: three FAIL rows after 1.3 s, no hang.
• Windows cap: the code comment, §Patch 22 and the body now say 500 one-millisecond passes: 500 ms on POSIX, up to ~8 s at Windows' default tick.
• Busy-clear untested: not taken in this push. Driving it needs a valid V2 correction file with its checksum through the FIFO. It stays at the coverage boundary the body already names.

[Ozy311]

Review: #6261 at aa80a5fb

Issue fit (#6179): Yes, on the normal path. The two earlier reviews' blocker (the hand-written extern "C" that broke the shared-core link) and their nits are addressed in a6084f8e and aa80a5fb, and I checked each fix against the diff. One finding is left. It is not a blocker, because the path it is on already has undefined behavior on main. But it contradicts what the patch notes say, and the fix is a few lines.

This was a read-only pass: I read the diff and third_party/wdsp/port/wdsp_port.c at the PR head. Nothing was built or run.

Scope

Everything in the diff is explained by the issue. That includes the new WdspChannel::restorePureSignalCorrectionForTest() hook and the PSRestoreCorr declaration in aether_wdsp.h; both are test-only, in the shape #6309 uses.

Finding (should-fix, non-blocking): the bounded fallback closes the handles under a live thread, which patch 18 specifically avoided

third_party/wdsp/upstream/calcc.c:1101-1116

When workerGone is false and 500 passes elapse, or when the wait returns WAIT_FAILED, the loop breaks with the correction thread still alive. Execution then falls through to the closes, which now run unconditionally:

	for (int i = 0; i < 5; i++)
		CloseHandle(a->SemsPSCorr[i]);
	CloseHandle(a->hCorrChangeExited);

Under the POSIX shim, CloseHandle() runs pthread_cond_destroy, then pthread_mutex_destroy, then free(handle) (port/wdsp_port.c:275-284). The surviving thread is either blocked in WaitForMultipleObjects(…, a->SemsPSCorr, …, INFINITE) (calcc.c:1769) or heading for SetEvent(a->hCorrChangeExited) (calcc.c:1868), so both are now on freed handles. Patch 18 guarded the closes with == WAIT_OBJECT_0 precisely to avoid "adding a second hazard on the timeout path". On this path the PR brings that hazard back.

That differs from what the docs now say:

• AETHERSDR-PATCHES.md, patch 18 note: "Patch 22 closes the timeout path by removing it." The timeout path still exists whenever !workerGone.
• Patch 18 bullet: "the closes now follow the unbounded exit wait." They also follow the bounded one.
• §Patch 22: "this patch does not make it worse." It does, slightly: before this PR, only a was freed under the thread; now the handles are as well.

Keeping patch 18's guard fixes this and leaves the normal path unchanged:

	int corrThreadGone = !a->corrThreadStarted;
	if (a->corrThreadStarted && a->hCorrChangeExited != NULL)
	{
		const int workerGone = _InterlockedAnd(&ch[a->channel].mainExited, ~0L)
			== _InterlockedAnd(&ch[a->channel].mainGen, ~0L);
		int waited = 0;
		DWORD waitstat;
		while ((waitstat = WaitForSingleObject(a->hCorrChangeExited, 1)) != WAIT_OBJECT_0)
		{
			if (waitstat == WAIT_FAILED || (!workerGone && ++waited >= 500))
				break;
			InterlockedBitTestAndReset(&b->busy, 0);
		}
		corrThreadGone = waitstat == WAIT_OBJECT_0;
	}
	// AetherSDR patch 18: close only once the thread is gone; on the bounded
	// fallback it may still be waiting on them.
	if (corrThreadGone)
	{
		for (int i = 0; i < 5; i++)
			CloseHandle(a->SemsPSCorr[i]);
		CloseHandle(a->hCorrChangeExited);
	}

(Written, not compiled.) With this change, the doc sentences above could say: the timeout path remains only behind a wedged worker; there the handles are left open, as patch 18 had them, and a is still freed, as on main.

Nit (non-blocking)

• tests/wdsp_calcc_teardown_test.cpp:46: on Windows the test prints [SKIP] and returns 0, so ctest reports a pass. Returning 77 with SKIP_RETURN_CODE 77 in tests/tests.cmake would report it as skipped. No CI job builds it on Windows today, so this only affects how local runs read.

What I tried to break that held

• Lifetime ordering on the normal path. The drain comes first, the exit wait is unbounded only when patch 4 saw the worker exit, and the closes, the util frees, cs_update and iqc all come after the thread's SetEvent. That matches the two earlier reviews.
• The new test hook. restorePureSignalCorrectionForTest() goes through WdspChannel, the test no longer declares any WDSP symbol, and PSRestoreCorr is now declared in aether_wdsp.h. That is the #6309 pattern.
• The FIFO-race fix. O_WRONLY | O_NONBLOCK with an ENXIO check turns a lost race into a FAIL line instead of a hang until the ctest timeout.

Not tested: I built and ran nothing, including the sanitizer lanes. CI on aa80a5fb is green, and the run was created after #6309 merged, but no ci.yml filter runs wdsp_calcc_teardown_test.

73, K6OZY

[jcmerg]

Thanks, you're right: my a6084f8e dropped patch 18's guard, and on the bounded fallback (and on WAIT_FAILED) the closes freed the handles under the live thread.

Addressed in 9ffa9ace:

• Finding: taken in your shape. corrThreadGone starts as !corrThreadStarted and becomes waitstat == WAIT_OBJECT_0 after the wait. The five semaphores and the exit event are closed only when it is set. The normal path is unchanged. The patch 18 bullet, the patch 18 note and §Patch 22 now say the timeout path remains only behind a wedged worker: the handles stay open there, as patch 18 had them, and a is freed as on main. The body says the same.
• Nit: on Windows the test returns 77, and tests.cmake registers it with SKIP_RETURN_CODE 77.

Built on macOS 26 / arm64, Qt 6.12.0, both static and with AETHER_SHARED_CORE=ON. wdsp_calcc_teardown_test is 3/3 green in each, and ctest -R '^(wdsp_channel_test|wdsp_calcc_teardown_test)$' passes 2/2 in each. With workerGone forced to 0 the test still goes red on the 1 s row, so the normal path takes the unbounded branch. The guarded closes themselves are still not observable without a sanitizer lane, and I did not compile on Windows.

[jensenpat]

Review: #6261 at 9ffa9ace728bb9f9c646cd9a6ac6594d5c47c963

Issue fit (#6179): Yes. On the normal path destroy_calcc() now waits until doPSCorrChange() signals hCorrChangeExited before it frees the CALCC, the six util splines, and (via the existing destroy_txa() order) iqc / cs_update. The wait is unbounded only when patch 4 saw the DSP worker exit (mainExited == mainGen). If that wait fell through, 500 one-millisecond passes still bound it, the semaphore and exit-event handles stay open (patch 18), and a is freed as on main. 9ffa9ace is the handle-close guard from the review at aa80a5fb.

RadioModel seam: This change does not cross it. The diff does not touch RadioModel, RadioSession, IRadioBackend, src/gui/, or any radio-family wire header. restorePureSignalCorrectionForTest() lives on WdspChannel (src/core/dsp/), which HL2 transmit already owns inside Hl2TxDsp behind the backend. Its only caller is tests/wdsp_calcc_teardown_test.cpp. src/models/ and src/gui/ do not include WdspChannel.h or aether_wdsp.h. PSRestoreCorr is declared on the WDSP facade so WdspChannel.cpp can call the copy the core links; under AETHER_SHARED_CORE=ON it is not exported from libaethercore (hidden visibility). No new family-string branch, vendor include above the seam, dynamic_cast to a concrete backend, capability boolean, or keying path around the TX gate.

Affected runtime is a WDSP TX channel teardown. That is the HL2 TXA channel today. Flex does not run this thread. ANAN remains RX-only, so it does not create a TXA CALCC.

Scope

File	What it changes	Claimed?	Verdict
third_party/wdsp/upstream/calcc.c	corrThreadStarted; drain, wait, close handles only once the thread is gone, then free util	Yes	In scope
src/core/dsp/WdspChannel.h / .cpp	Test-only restorePureSignalCorrectionForTest()	Yes	In scope; not a RadioModel API
third_party/wdsp/include/aether_wdsp.h	Declares PSRestoreCorr for that hook	Yes	In scope; symbol stays inside the core
tests/wdsp_calcc_teardown_test.cpp	FIFO-parked teardown	Yes	In scope; no socket
tests/tests.cmake	Registers it, SKIP_RETURN_CODE 77, timeout 120	Yes	In scope
third_party/wdsp/AETHERSDR-PATCHES.md	Patch 22; patch 18 note updated	Yes	In scope

No CHANGELOG.md entry. Four commits, all on #6179. The earlier shared-core link blocker (a test extern "C" to PSRestoreCorr) is gone.

Verification

• Required CI on this SHA is green: build, check-windows, check-macos, Static checks, Sanitizer option configures (run 38041591758 / 38041591792).
• Those jobs do not execute this test. It is not in .github/ci-test-gate.txt.
• Local shared-core build of wdsp_calcc_teardown_test at this SHA (Qt 6.12.0, AETHER_SHARED_CORE=ON, RelWithDebInfo). Link succeeded. nm shows PSRestoreCorr is not an undefined symbol in the test and is not exported from libaethercore.dylib. ctest -R '^wdsp_calcc_teardown_test$': 1/1 passed, 2.04 s.
• The test pins the wait (teardown still blocked at 1 s, then completes when the FIFO writer opens). It does not pin the free ordering; that remains a sanitizer-lane question, and those lanes are weekly.

No blockers.

Non-blocking follow-up

The freed-memory half (util splines and a not touched until the thread has left PSRestoreCorr) is not asserted by wdsp_calcc_teardown_test. The weekly ASan/TSan lanes are the useful run, on the HL2/WDSP TX teardown seam. Not a merge prerequisite.