diff --git a/src/core/backends/hl2/Hl2Backend.cpp b/src/core/backends/hl2/Hl2Backend.cpp index 2cca872465..9a2495fae5 100644 --- a/src/core/backends/hl2/Hl2Backend.cpp +++ b/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& 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 @@ -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(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 87b3852b65..a13af3c5ad 100644 --- a/tests/hl2_pan_create_async_test.cpp +++ b/tests/hl2_pan_create_async_test.cpp @@ -11,18 +11,23 @@ #include "TestSettingsProfile.h" #include "TestDspBuildWait.h" +#include "SeamThreadAffinityProbe.h" #include #include #include #include #include +#include +#include #include #include #include #include #include +#include +#include #include #include @@ -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 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 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 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; }