fix(wdsp): tearing down a TX channel waits for the PureSignal correction thread. Principle VIII.

src/core/dsp/WdspChannel.cpp

@@ -1273,6 +1273,12 @@ void WdspChannel::synchronizeWorkerForTest() const noexcept
     SynchronizeChannelWorkerForTest(m_channelId);
 }
 
+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));
+}
+
 std::unique_lock<std::mutex> WdspChannel::fftwSetupLock()
 {
     // Forwards, and keeps its name so Hl2Spectrum, AnanPanAnalyzer and

src/core/dsp/WdspChannel.h

@@ -515,6 +515,9 @@ class WdspChannel final
     // After output-count readiness, wait for the last hook and DSP body before
     // arming another hold. Test/control only; NEVER call while a hold is armed.
     void synchronizeWorkerForTest() const noexcept;
+    // 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;
     // Test only: the next `count` control operations on THIS channel are
     // refused exactly as a racing processIq() callback would refuse them, so
     // a caller's refused-then-converges path can be driven deterministically.

tests/tests.cmake

@@ -1016,6 +1016,13 @@ target_link_libraries(wdsp_channel_reservation_test PRIVATE aethercore)
 add_test(NAME wdsp_channel_reservation_test COMMAND wdsp_channel_reservation_test)
 set_tests_properties(wdsp_channel_reservation_test PROPERTIES TIMEOUT 120)
 
+# Tearing down a TX channel waits for the PureSignal correction thread before
+# freeing what it uses (#6179, WDSP patch 22). POSIX FIFO; skips on Windows.
+add_executable(wdsp_calcc_teardown_test tests/wdsp_calcc_teardown_test.cpp)
+target_link_libraries(wdsp_calcc_teardown_test PRIVATE aethercore)
+add_test(NAME wdsp_calcc_teardown_test COMMAND wdsp_calcc_teardown_test)
+set_tests_properties(wdsp_calcc_teardown_test PROPERTIES SKIP_RETURN_CODE 77 TIMEOUT 120)
+
 # Compiles src/core/NnrControls.h so its static_asserts are real, and pins the
 # default markers the NNR tab draws. Header-only: the WDSP cross-check needs the
 # facade that arrives with NnrFilter (RFC #5684 step 2).

tests/wdsp_calcc_teardown_test.cpp

@@ -0,0 +1,102 @@
+// Tearing down a TX channel must wait for WDSP's PureSignal correction thread
+// (doPSCorrChange) before freeing what it uses (#6179, WDSP patch 22).
+//
+// The thread is held in PSRestoreCorr()'s fopen() on a FIFO that has no writer
+// yet, which no fixed timeout can outlast. The channel is destroyed on another
+// thread; the writer opens 1 s later. CloseChannel() must still be waiting
+// then, and return once the thread is released. Upstream gave up after 500 ms
+// and freed the CALCC under the blocked thread.
+
+#include "core/dsp/WdspChannel.h"
+
+#include <atomic>
+#include <chrono>
+#include <cerrno>
+#include <cstdio>
+#include <cstdlib>
+#include <iostream>
+#include <memory>
+#include <string>
+#include <thread>
+
+#ifndef _WIN32
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#endif
+
+namespace {
+
+int failures = 0;
+
+void check(bool value, const char* message)
+{
+    std::cout << (value ? "[ OK ] " : "[FAIL] ") << message << '\n';
+    if (!value) {
+        ++failures;
+    }
+}
+
+}   // namespace
+
+int main()
+{
+#ifdef _WIN32
+    std::cout << "[SKIP] needs a POSIX FIFO\n";
+    return 77;
+#else
+    WdspChannel::Config tx;
+    tx.direction = WdspChannel::Direction::Transmit;
+    tx.inputSampleRate = 48000;
+    tx.dspSampleRate = 48000;
+    tx.outputSampleRate = 48000;
+    std::string error;
+    std::unique_ptr<WdspChannel> channel = WdspChannel::create(tx, &error);
+    check(channel != nullptr, "a transmit channel opens");
+    if (!channel) {
+        std::cerr << error << '\n';
+        return 1;
+    }
+
+    std::string fifo = std::string(std::getenv("TMPDIR") ? std::getenv("TMPDIR") : "/tmp")
+        + "/wdsp-calcc-teardown-" + std::to_string(::getpid());
+    ::unlink(fifo.c_str());
+    check(::mkfifo(fifo.c_str(), 0600) == 0, "premise: the FIFO is created");
+
+    // The correction thread now blocks in fopen(fifo, "r") until a writer opens.
+    channel->restorePureSignalCorrectionForTest(fifo.c_str());
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    std::atomic<bool> closed{false};
+    std::thread closer([&] {
+        channel.reset();
+        closed.store(true);
+    });
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(1000));
+    check(!closed.load(),
+          "teardown is still waiting 1 s later, past upstream's 500 ms limit");
+
+    // Release the thread: a line that is no correction file, so the restore
+    // fails and it moves on. Held open briefly so the reader sees a writer.
+    // Non-blocking, so a thread that never opened the FIFO fails here (ENXIO)
+    // instead of hanging until the ctest timeout.
+    const int writer = ::open(fifo.c_str(), O_WRONLY | O_NONBLOCK);
+    if (writer < 0 && errno == ENXIO) {
+        check(false, "premise: the correction thread has the FIFO open for reading");
+    }
+    check(writer >= 0, "premise: the writer end opens");
+    if (writer >= 0) {
+        const char junk[] = "not-a-correction-file x\n";
+        check(::write(writer, junk, sizeof(junk) - 1) > 0, "premise: the junk line is written");
+        std::this_thread::sleep_for(std::chrono::milliseconds(200));
+        ::close(writer);
+    }
+    closer.join();
+    check(closed.load(), "teardown completes once the correction thread is released");
+    ::unlink(fifo.c_str());
+
+    std::cout << failures << " failure(s)\n";
+    return failures == 0 ? 0 : 1;
+#endif
+}

third_party/wdsp/AETHERSDR-PATCHES.md

@@ -573,7 +573,7 @@ number, so the grep above does not find them; it is findable from this file
 and from `ensure_minphase` in `upstream/firmin.c`.
 
 When refreshing WDSP, first check whether upstream has made each change
-itself. The ten are different shapes, so grep for the shape, not for a free:
+itself. They are different shapes, so grep for the shape, not for a free:
 
 - **patches 1-2** -- a trailing `_aligned_free()` of the object itself at the
   end of `destroy_notchdb()` / `destroy_nurbs()`.
@@ -616,8 +616,14 @@ itself. The ten are different shapes, so grep for the shape, not for a free:
   `stitch(disp)`, not before it; and `sendbuf()`'s `IQO_idx` hand-off and
   `IQout_index` advance inside its existing `BufferControlSection`.
 - **patch 18** -- in `destroy_calcc()`, the `SemsPSCorr` and
-  `hCorrChangeExited` closes inside an `== WAIT_OBJECT_0` check on the
-  thread-exit wait.
+  `hCorrChangeExited` closes, still only after the thread's exit event was
+  seen (`corrThreadGone`, patch 22). A thread that was never started has
+  nothing to wait for, so its handles are closed directly.
+- **patch 22** -- in `calcc.c`: the `corrThreadStarted` field set from
+  `_beginthread()`'s result in `create_calcc()`, and in `destroy_calcc()` the
+  `hCorrChangeExited` wait (clearing iqc's `busy` bit each pass, unbounded
+  once patch 4 saw the worker exit) with the six `util` spline frees moved
+  **after** it, not before.
 
 Drop any local patch upstream now carries. Otherwise reapply only these minimal
 changes and run the lifecycle test under AddressSanitizer on every supported
@@ -1003,7 +1009,9 @@ wait, `destroy_calcc()` goes on to free `a` and its members, which a correction
 thread still running past the timeout would use. That use-after-free predates
 this patch (upstream frees `a` on the same path) and is tracked separately
 (#6179). Patch 18 only stops the normal, successful-exit path from
-leaking and avoids adding a second hazard on the timeout path.
+leaking and avoids adding a second hazard on the timeout path. Patch 22 leaves
+the timeout path only behind a wedged DSP worker; there the handles stay open
+as above, and `a` is still freed, as on `main`.
 
 **Upstream status.** Not reported.
 
@@ -1238,3 +1246,60 @@ freshness and retired observations. These tests are added validation obligations
 their results must come from a subsequent exact-source Nobara run. On upstream
 refresh retain this extension unless equivalent typed, bounded, race-free
 semantics replace it, and do not label INDY as a PLL.
+
+## Patch 22 — `destroy_calcc()` frees nothing the correction thread still uses (#6179)
+
+`upstream/calcc.c`. `destroy_calcc()` waited at most 500 ms for the
+`doPSCorrChange()` thread to signal `hCorrChangeExited`, then freed `a` and its
+members anyway. The thread reads `a`, the six `util` splines, `txa[].iqc` and
+`txa[].calcc.cs_update`, all freed by `destroy_calcc()` / `destroy_txa()`
+right after. A thread in `calc()` or in correction-file I/O past the 500 ms
+went on using freed memory. Upstream also freed the six `util` splines
+**before** the wait, under a thread in `PSSaveCorr` / `PSRestoreCorr` work.
+
+Now:
+
+- `create_calcc()` records whether `_beginthread()` started the thread
+  (`corrThreadStarted`). A thread that never started is not waited for, which
+  is the reason patches 4 and 9 bound their waits.
+- `destroy_calcc()` waits for `hCorrChangeExited`, clearing iqc's `busy` bit
+  on each 1 ms pass, because `SetTXAiqcStart()` / `SetTXAiqcSwap()` spin on
+  it for a DSP worker that `CloseChannel()` has already stopped. With the four
+  work semaphores drained first, the exit token is the next one the thread
+  takes once its current work ends.
+- The wait has **no time limit only when patch 4's worker wait succeeded**
+  (`mainExited == mainGen`). Those iqc setters take `ch[].csDSP`, which is
+  free only once the worker has exited (patch 9). If patch 4's wait fell
+  through its cap, a wedged worker may still hold `csDSP`, an unbounded wait
+  could then never end, and a bound of 500 one-millisecond passes stands
+  instead: 500 ms on POSIX, up to ~8 s under Windows' default 15.6 ms timer
+  tick, since nothing in AetherSDR calls `timeBeginPeriod`. That path
+  is already undefined on `main` (`post_main_destroy` deletes `csDSP` under a
+  live holder); there the handles stay open as patch 18 had them, and `a` is
+  still freed as on `main`, so this patch does not make it worse.
+- A `WAIT_FAILED` return (an exit event `CreateEvent` failed to make) ends
+  the wait rather than spinning.
+- The `SemsPSCorr` / exit-event closes (patch 18) run only once the exit
+  event was seen, or when the thread never started. The six `util` spline
+  frees follow the wait.
+
+**What can still stall teardown.** The thread's own work, on the normal path:
+`calc()` is bounded CPU work. Correction-file I/O can block as long as the file system does.
+AetherSDR calls neither `PSSaveCorr` nor `PSRestoreCorr` today; a caller that
+adds them owns that stall.
+
+**Reproduced before fixing**, locally: no `ci.yml` filter runs the test, and
+it covers the wait, not the reordering of the frees after it (that half is the
+sanitizer lanes'). `wdsp_calcc_teardown_test` holds the thread in
+`PSRestoreCorr()` on a FIFO with no writer, closes the channel on another
+thread, and opens the writer 1 s later. Before this patch `CloseChannel()`
+returned after ~500 ms while the thread was still blocked. With it,
+`CloseChannel()` returns only after the writer opens. POSIX only (`mkfifo`);
+skipped on Windows. The test queues the restore through
+`WdspChannel::restorePureSignalCorrectionForTest()`, so it links under
+`AETHER_SHARED_CORE` and the sanitizer lanes build it.
+
+**Upstream status.** Not reported.
+
+When updating WDSP, keep this unless upstream's `destroy_calcc()` joins the
+correction thread before freeing.

third_party/wdsp/include/aether_wdsp.h

@@ -527,6 +527,9 @@ int GetChannelOutputSamplesForTest(int channel);
 // After count readiness, wait for the previous handoff hook/DSP to finish.
 // Test/control only; never call with either worker hold armed or entered.
 void SynchronizeChannelWorkerForTest(int channel);
+// Queues a correction-file restore on a TX channel's PureSignal correction
+// thread. AetherSDR calls it only from wdsp_calcc_teardown_test (patch 22).
+void PSRestoreCorr(int channel, char* filename);
 
 #ifdef __cplusplus
 }

third_party/wdsp/upstream/calcc.c

@@ -199,6 +199,7 @@ typedef struct _calcc
 		CurveEMA      m_calavg_save, c_calavg_save, s_calavg_save;
 	} util;
 	HANDLE hCorrChangeExited;
+	int corrThreadStarted;	// AetherSDR patch 22: destroy_calcc() waits only for a thread that exists
 } calcc, * CALCC;
 
 void __cdecl doPSCorrChange(void* arg);
@@ -1079,40 +1080,53 @@ CALCC create_calcc (int channel, int runcal, int size, int rate, double hw_scale
 		a->SemsPSCorr[i] = CreateSemaphoreW(0, 0, 1, 0);
 	}
 	a->hCorrChangeExited = CreateEvent(NULL, FALSE, FALSE, NULL);
-	_beginthread(doPSCorrChange, 0, (void*)a);
+	a->corrThreadStarted = _beginthread(doPSCorrChange, 0, (void*)a) != (uintptr_t)-1;
 	return a;
 }
 
 void destroy_calcc (CALCC a)
 {
 	IQC b = txa[a->channel].iqc.p;
 
-	ns_free(a->util.m_spline_restore); a->util.m_spline_restore = NULL;
-	ns_free(a->util.c_spline_restore); a->util.c_spline_restore = NULL;
-	ns_free(a->util.s_spline_restore); a->util.s_spline_restore = NULL;
-	ns_free(a->util.m_spline_save);    a->util.m_spline_save = NULL;
-	ns_free(a->util.c_spline_save);    a->util.c_spline_save = NULL;
-	ns_free(a->util.s_spline_save);    a->util.s_spline_save = NULL;
-
 	for (int i = 0; i < 4; i++)
 		while (WaitForSingleObject(a->SemsPSCorr[i], 0) == WAIT_OBJECT_0);
 	InterlockedBitTestAndReset(&b->busy, 0);
 	ReleaseSemaphore(a->SemsPSCorr[4], 1, 0);
-	// AetherSDR patch 18: close the five SemsPSCorr semaphores, which upstream
-	// never closed (leaked on every TXA channel destroy), and close them and the
-	// exit event ONLY once doPSCorrChange() has signalled it is gone. Its last
-	// act is SetEvent(hCorrChangeExited) then return, so after that nothing
-	// touches either. 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 hazard predates this patch and is tracked separately
-	// (#6179; see AETHERSDR-PATCHES.md, patch 18).
-	if (WaitForSingleObject(a->hCorrChangeExited, 500) == WAIT_OBJECT_0)
+	// AetherSDR patch 22 (#6179): nothing the correction thread touches (`a`,
+	// its util splines, the iqc stage, cs_update) is freed until it has exited.
+	// No time limit once patch 4 saw the DSP worker exit: csDSP, which the
+	// thread's iqc setters take, is then free, and iqc's busy bit they spin on
+	// is cleared each pass. If that wait fell through, 500 passes bound it
+	// (500 ms on POSIX, longer at Windows' default timer tick).
+	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: closed only once the thread is gone; past the
+	// bounded fallback it may still wait on them or set the event.
+	if (corrThreadGone)
 	{
 		for (int i = 0; i < 5; i++)
 			CloseHandle(a->SemsPSCorr[i]);
 		CloseHandle(a->hCorrChangeExited);
 	}
+	ns_free(a->util.m_spline_restore); a->util.m_spline_restore = NULL;
+	ns_free(a->util.c_spline_restore); a->util.c_spline_restore = NULL;
+	ns_free(a->util.s_spline_restore); a->util.s_spline_restore = NULL;
+	ns_free(a->util.m_spline_save);    a->util.m_spline_save = NULL;
+	ns_free(a->util.c_spline_save);    a->util.c_spline_save = NULL;
+	ns_free(a->util.s_spline_save);    a->util.s_spline_save = NULL;
 
 	ns_free(a->m_spline); a->m_spline = NULL;
 	ns_free(a->c_spline); a->c_spline = NULL;