fix(hl2): drop spectrum and meter output from a closed receiver's DSP. Principle VIII.

src/core/backends/hl2/Hl2Backend.cpp

@@ -838,14 +838,19 @@ 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. audioReady and meterUpdate
+    // below apply the same check.
     connect(dsp, &Hl2RxDsp::spectrumReady, this,
-            [this, ui](const std::vector<float>& bins) {
-        if (!m_ids.byUi(ui))
+            [this, ui, producer = QPointer<Hl2RxDsp>(dsp)](const std::vector<float>& 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
@@ -884,11 +889,12 @@ bool Hl2Backend::openReceiverDsp(int ddc, std::string* error)
     });
 
     connect(dsp, &Hl2RxDsp::meterUpdate, this,
-            [this, ui](float dbfs) {
+            [this, ui, producer = QPointer<Hl2RxDsp>(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);

tests/hl2_pan_create_async_test.cpp

@@ -11,18 +11,23 @@
 
 #include "TestSettingsProfile.h"
 #include "TestDspBuildWait.h"
+#include "SeamThreadAffinityProbe.h"
 
 #include <QCoreApplication>
 #include <QElapsedTimer>
 #include <QEvent>
 #include <QEventLoop>
 #include <QHash>
+#include <QList>
+#include <QPointer>
 #include <QSemaphore>
 #include <QStringList>
 #include <QThread>
 #include <QTimer>
 
 #include <atomic>
+#include <cmath>
+#include <vector>
 #include <cstdio>
 #include <tuple>
 
@@ -79,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;
@@ -651,6 +657,135 @@ 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 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 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
+    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;
+                     });
+    const QString ui1Meter = Access::sliceMeterName(1);
+    QList<double> 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");
+    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<float> bins(16, -100.0f);
+    const auto emitFrom = [&bins](AetherSDR::hl2::Hl2RxDsp* dsp) {
+        QMetaObject::invokeMethod(dsp, [dsp, &bins] { emit dsp->spectrumReady(bins); },
+                                  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<AetherSDR::hl2::Hl2RxDsp> 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);
+    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");
+    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");
+    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
+            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;
+    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) {
+        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 +802,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;
 }