From 884f0f07fc35b6f2f0d1daafa4a4f4daf30b1d10 Mon Sep 17 00:00:00 2001 From: "Jeremy [KK7GWY]" Date: Sat, 3 Oct 2026 09:06:57 -0700 Subject: [PATCH 1/2] fix(hl2): drop spectrum frames from a closed receiver's DSP. Principle VIII. A spectrum frame queued by a receiver's DSP outlives disconnect() and deleteLater(), and a receiver reopened before delivery takes the same UI number, so the frame published as the new receiver's. The forward now checks its producer is still the receiver's DSP, as the audioReady forward beside it already does. hl2_pan_create_async_test queues a frame from the old DSP, closes and reopens UI 1, then delivers it. Refs #6096 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01AADGw3HVn5FMMVrx2RQZ9g --- src/core/backends/hl2/Hl2Backend.cpp | 16 +++-- tests/hl2_pan_create_async_test.cpp | 104 +++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 6 deletions(-) diff --git a/src/core/backends/hl2/Hl2Backend.cpp b/src/core/backends/hl2/Hl2Backend.cpp index 906d884ff9..5307260a6d 100644 --- a/src/core/backends/hl2/Hl2Backend.cpp +++ b/src/core/backends/hl2/Hl2Backend.cpp @@ -834,14 +834,18 @@ bool Hl2Backend::openReceiverDsp(int ddc, std::string* error) // the UI number never changes (Hl2ReceiverMap::remove). const int ui = ids->uiNumber; - // Rule 6 holds by ordering, not by this gate: the DSP emits on m_ioThread, - // the thread that emits linkDown, so no frame is delivered after disconnected(). - // Unlike audioReady below, there is no producer check: a frame queued by a - // closed receiver's DSP publishes if its UI number was reopened before delivery. + // Rule 6 holds by ordering: the DSP emits on m_ioThread, the thread that + // emits linkDown, so no frame is delivered after disconnected(). A frame is + // published only while its producer is still the receiver behind its UI + // number: a queued frame outlives disconnect() and deleteLater(), and a + // reopened receiver can take the same number (as audioReady below). connect(dsp, &Hl2RxDsp::spectrumReady, this, - [this, ui](const std::vector& bins) { - if (!m_ids.byUi(ui)) + [this, ui, producer = QPointer(dsp)](const std::vector& bins) { + const auto* ids = m_ids.byUi(ui); + const Receiver* receiver = ids ? rx(ids->ddcIndex) : nullptr; + if (!producer || !receiver || receiver->dsp != producer.data()) { return; + } // dBFS -> dBm through the shared reference (one AD9866 behind every // DDC): derived full scale (+3 dB) minus LNA gain, so the trace holds // still across gain changes. off == 0.0 fires only when the LNA gain diff --git a/tests/hl2_pan_create_async_test.cpp b/tests/hl2_pan_create_async_test.cpp index 87b3852b65..e35f62e8ef 100644 --- a/tests/hl2_pan_create_async_test.cpp +++ b/tests/hl2_pan_create_async_test.cpp @@ -11,18 +11,21 @@ #include "TestSettingsProfile.h" #include "TestDspBuildWait.h" +#include "SeamThreadAffinityProbe.h" #include #include #include #include #include +#include #include #include #include #include #include +#include #include #include @@ -651,6 +654,105 @@ void aFailedBuildLeavesTheRolesOnALiveReceiver() } // namespace +// Swallows the retired chain's DeferredDelete so it is still alive when its +// queued frame is delivered: the producer-identity term, not the QPointer, +// must then reject it. Lives on the chain's thread, as an event filter must. +class KeepAlive : public QObject +{ +public: + bool eventFilter(QObject*, QEvent* event) override + { + return event->type() == QEvent::DeferredDelete; + } +}; + +// A frame its DSP queued before the close must not publish under the +// recycled UI number; the reuser's own frames must. +void aStaleSpectrumFrameDoesNotPublishOnTheReuser(bool retiredChainAlive) +{ + std::fprintf(stderr, "-- stale spectrum frame, retired chain %s at delivery\n", + retiredChainAlive ? "still alive" : "deleted"); + Hl2Backend backend; + AetherSDR::test::SeamThreadAffinityProbe seam(&backend); // after backend: torn down first + AetherSDR::test::attachAllSeamSignals(seam); + if (!bringUp(backend)) { + return; + } + + int framesOnUi1 = 0; + QObject::connect(&backend, &IRadioBackend::spectrumFrameReady, &backend, + [&framesOnUi1](int ui, const QByteArray&) { + if (ui == 1) + ++framesOnUi1; + }); + + check(backend.createPanadapter(), "a receiver is added"); + check(Access::lastReceiverUi(backend) == 1, "it takes UI 1"); + AetherSDR::test::spinUntil( + [&] { return Access::dspChannelForUi(backend, 1) >= 0; }); + AetherSDR::hl2::Hl2RxDsp* const oldDsp = Access::receiverDsp(backend, 1); + check(oldDsp != nullptr, "its chain is built"); + if (!oldDsp) { + return; + } + + const std::vector bins(16, -100.0f); + const auto emitFrom = [&bins](AetherSDR::hl2::Hl2RxDsp* dsp) { + QMetaObject::invokeMethod(dsp, [dsp, &bins] { emit dsp->spectrumReady(bins); }, + Qt::BlockingQueuedConnection); + }; + + emitFrom(oldDsp); + QCoreApplication::sendPostedEvents(&backend, QEvent::MetaCall); + check(framesOnUi1 == 1, "a frame from the current chain publishes on UI 1"); + + QPointer oldGuard(oldDsp); + KeepAlive* keepAlive = nullptr; + if (retiredChainAlive) { + keepAlive = new KeepAlive; + keepAlive->moveToThread(oldDsp->thread()); + QMetaObject::invokeMethod(oldDsp, [oldDsp, keepAlive] { + oldDsp->installEventFilter(keepAlive); + }, Qt::BlockingQueuedConnection); + } + + // Queued on the GUI thread and not delivered until after the reopen. + emitFrom(oldDsp); + check(backend.removePanadapter(AetherSDR::hl2::hl2PanId(1)), "UI 1 is closed"); + check(backend.createPanadapter(), "a receiver is added again"); + check(Access::lastReceiverUi(backend) == 1, "the reuser is handed UI 1 again"); + flushDeferredDeletes(Access::wire(backend)); + check(oldGuard.isNull() != retiredChainAlive, + retiredChainAlive ? "the retired chain is still alive at delivery" + : "the retired chain is deleted before delivery"); + QCoreApplication::sendPostedEvents(&backend, QEvent::MetaCall); + check(framesOnUi1 == 1, + "the closed chain's queued frame does not publish on the reused UI 1"); + if (keepAlive) { + QMetaObject::invokeMethod(keepAlive, [keepAlive, oldGuard] { + delete oldGuard.data(); // on its own thread, which owns its WDSP channel + delete keepAlive; + }, Qt::BlockingQueuedConnection); + } + + AetherSDR::test::spinUntil( + [&] { return Access::dspChannelForUi(backend, 1) >= 0; }); + AetherSDR::hl2::Hl2RxDsp* const newDsp = Access::receiverDsp(backend, 1); + check(newDsp != nullptr && newDsp != oldDsp, "the reuser has its own chain"); + const int beforeReuserFrame = framesOnUi1; + if (newDsp) { + emitFrom(newDsp); + QCoreApplication::sendPostedEvents(&backend, QEvent::MetaCall); + } + check(framesOnUi1 == beforeReuserFrame + 1, "the reuser's own frame publishes on UI 1"); + + const QStringList violations = seam.violations(); + for (const QString& v : violations) { + std::fprintf(stderr, " %s\n", v.toUtf8().constData()); + } + check(violations.isEmpty(), "every seam signal is emitted on the backend's thread"); +} + int main(int argc, char** argv) { TestSettingsProfile profile(QStringLiteral("hl2-pan-create-async")); @@ -667,6 +769,8 @@ int main(int argc, char** argv) aRetiredBuildDoesNotCompleteAgainstReconstructedReceivers(true); aFailedBuildLeavesTheRolesOnALiveReceiver(); aFailedBuildWithdrawsItsSMeter(); + aStaleSpectrumFrameDoesNotPublishOnTheReuser(false); + aStaleSpectrumFrameDoesNotPublishOnTheReuser(true); std::fprintf(stderr, "hl2_pan_create_async_test: %d failure(s)\n", g_failures); return g_failures == 0 ? 0 : 1; } From 43aefa5a24cc834146cd746d87bfc05fa23d75d0 Mon Sep 17 00:00:00 2001 From: "Jeremy [KK7GWY]" Date: Sat, 3 Oct 2026 09:22:17 -0700 Subject: [PATCH 2/2] fix(hl2): drop meter samples from a closed receiver's DSP. Principle VIII. The meter forward had the same gap as the spectrum one: a sample queued by a closed receiver's DSP reached the reopened receiver's S-meter smoother under the recycled UI number. It now checks its producer, as the spectrum and audio forwards do. Refs #6096 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01AADGw3HVn5FMMVrx2RQZ9g --- src/core/backends/hl2/Hl2Backend.cpp | 8 +++--- tests/hl2_pan_create_async_test.cpp | 39 +++++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/core/backends/hl2/Hl2Backend.cpp b/src/core/backends/hl2/Hl2Backend.cpp index 5307260a6d..411f526401 100644 --- a/src/core/backends/hl2/Hl2Backend.cpp +++ b/src/core/backends/hl2/Hl2Backend.cpp @@ -838,7 +838,8 @@ bool Hl2Backend::openReceiverDsp(int ddc, std::string* error) // emits linkDown, so no frame is delivered after disconnected(). A frame is // published only while its producer is still the receiver behind its UI // number: a queued frame outlives disconnect() and deleteLater(), and a - // reopened receiver can take the same number (as audioReady below). + // reopened receiver can take the same number. audioReady and meterUpdate + // below apply the same check. connect(dsp, &Hl2RxDsp::spectrumReady, this, [this, ui, producer = QPointer(dsp)](const std::vector& bins) { const auto* ids = m_ids.byUi(ui); @@ -884,11 +885,12 @@ bool Hl2Backend::openReceiverDsp(int ddc, std::string* error) }); connect(dsp, &Hl2RxDsp::meterUpdate, this, - [this, ui](float dbfs) { + [this, ui, producer = QPointer(dsp)](float dbfs) { const auto* ids = m_ids.byUi(ui); Receiver* r = ids ? rx(ids->ddcIndex) : nullptr; - if (!r) + if (!producer || !r || r->dsp != producer.data()) { return; + } // Same reference as the spectrum -- a meter that moved on a gain // change while the trace stayed put would be its own kind of lie. const double dbm = m_dbRef.toDbm(dbfs); diff --git a/tests/hl2_pan_create_async_test.cpp b/tests/hl2_pan_create_async_test.cpp index e35f62e8ef..a13af3c5ad 100644 --- a/tests/hl2_pan_create_async_test.cpp +++ b/tests/hl2_pan_create_async_test.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -25,6 +26,7 @@ #include #include +#include #include #include #include @@ -82,6 +84,7 @@ struct Hl2PanCreateTestAccess { QCoreApplication::sendPostedEvents(&backend, QEvent::MetaCall); } static QObject* wire(Hl2Backend& backend) { return backend.m_metis; } + static QString sliceMeterName(int uiNumber) { return Hl2Backend::sliceMeterName(uiNumber); } static QObject* buildContext(Hl2Backend& backend) { return backend.m_dspBuildContext; @@ -666,11 +669,11 @@ class KeepAlive : public QObject } }; -// A frame its DSP queued before the close must not publish under the -// recycled UI number; the reuser's own frames must. +// A frame or meter sample its DSP queued before the close must not publish +// under the recycled UI number; the reuser's own must. void aStaleSpectrumFrameDoesNotPublishOnTheReuser(bool retiredChainAlive) { - std::fprintf(stderr, "-- stale spectrum frame, retired chain %s at delivery\n", + std::fprintf(stderr, "-- stale spectrum frame and meter sample, retired chain %s at delivery\n", retiredChainAlive ? "still alive" : "deleted"); Hl2Backend backend; AetherSDR::test::SeamThreadAffinityProbe seam(&backend); // after backend: torn down first @@ -685,6 +688,13 @@ void aStaleSpectrumFrameDoesNotPublishOnTheReuser(bool retiredChainAlive) if (ui == 1) ++framesOnUi1; }); + const QString ui1Meter = Access::sliceMeterName(1); + QList metersOnUi1; + QObject::connect(&backend, &IRadioBackend::meterUpdate, &backend, + [&metersOnUi1, ui1Meter](const QString& id, double value) { + if (id == ui1Meter) + metersOnUi1 << value; + }); check(backend.createPanadapter(), "a receiver is added"); check(Access::lastReceiverUi(backend) == 1, "it takes UI 1"); @@ -702,9 +712,21 @@ void aStaleSpectrumFrameDoesNotPublishOnTheReuser(bool retiredChainAlive) Qt::BlockingQueuedConnection); }; + // A fresh smoother publishes its first reading at once, so one sample is + // enough to see a meter on either side of the reopen. + constexpr float kOldDbfs = -50.0f; + constexpr float kReuserDbfs = -90.0f; + const auto meterFrom = [](AetherSDR::hl2::Hl2RxDsp* dsp, float dbfs) { + QMetaObject::invokeMethod(dsp, [dsp, dbfs] { emit dsp->meterUpdate(dbfs); }, + Qt::BlockingQueuedConnection); + }; + emitFrom(oldDsp); + meterFrom(oldDsp, kOldDbfs); QCoreApplication::sendPostedEvents(&backend, QEvent::MetaCall); check(framesOnUi1 == 1, "a frame from the current chain publishes on UI 1"); + check(metersOnUi1.size() == 1, "a meter sample from the current chain publishes on UI 1"); + const double oldDbm = metersOnUi1.value(0); QPointer oldGuard(oldDsp); KeepAlive* keepAlive = nullptr; @@ -718,6 +740,7 @@ void aStaleSpectrumFrameDoesNotPublishOnTheReuser(bool retiredChainAlive) // Queued on the GUI thread and not delivered until after the reopen. emitFrom(oldDsp); + meterFrom(oldDsp, kOldDbfs); check(backend.removePanadapter(AetherSDR::hl2::hl2PanId(1)), "UI 1 is closed"); check(backend.createPanadapter(), "a receiver is added again"); check(Access::lastReceiverUi(backend) == 1, "the reuser is handed UI 1 again"); @@ -728,6 +751,8 @@ void aStaleSpectrumFrameDoesNotPublishOnTheReuser(bool retiredChainAlive) QCoreApplication::sendPostedEvents(&backend, QEvent::MetaCall); check(framesOnUi1 == 1, "the closed chain's queued frame does not publish on the reused UI 1"); + check(metersOnUi1.size() == 1, + "the closed chain's queued meter sample does not publish on the reused UI 1"); if (keepAlive) { QMetaObject::invokeMethod(keepAlive, [keepAlive, oldGuard] { delete oldGuard.data(); // on its own thread, which owns its WDSP channel @@ -740,11 +765,19 @@ void aStaleSpectrumFrameDoesNotPublishOnTheReuser(bool retiredChainAlive) AetherSDR::hl2::Hl2RxDsp* const newDsp = Access::receiverDsp(backend, 1); check(newDsp != nullptr && newDsp != oldDsp, "the reuser has its own chain"); const int beforeReuserFrame = framesOnUi1; + const qsizetype beforeReuserMeter = metersOnUi1.size(); if (newDsp) { emitFrom(newDsp); + meterFrom(newDsp, kReuserDbfs); QCoreApplication::sendPostedEvents(&backend, QEvent::MetaCall); } check(framesOnUi1 == beforeReuserFrame + 1, "the reuser's own frame publishes on UI 1"); + // Exactly its own reading: had the stale sample reached this smoother, the + // reading would be throttled or blended with it. + const double expectedDbm = oldDbm + (kReuserDbfs - kOldDbfs); + check(metersOnUi1.size() == beforeReuserMeter + 1 + && std::abs(metersOnUi1.last() - expectedDbm) < 1e-6, + "the reuser's own meter sample publishes on UI 1, unblended with the stale one"); const QStringList violations = seam.violations(); for (const QString& v : violations) {