From 7c7a7d7c1eae756c1516f1d7ee0a99e1204b8d89 Mon Sep 17 00:00:00 2001 From: srcejon Date: Thu, 14 Nov 2024 11:40:46 +0000 Subject: [PATCH 1/2] FreeDV: Fix memory allocation/free issues. Part of #2315. --- plugins/channelrx/demodfreedv/freedvdemod.cpp | 1 + plugins/channelrx/demodfreedv/freedvdemodsink.cpp | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/channelrx/demodfreedv/freedvdemod.cpp b/plugins/channelrx/demodfreedv/freedvdemod.cpp index 1d6dd4a31..3b996ffcf 100644 --- a/plugins/channelrx/demodfreedv/freedvdemod.cpp +++ b/plugins/channelrx/demodfreedv/freedvdemod.cpp @@ -79,6 +79,7 @@ FreeDVDemod::FreeDVDemod(DeviceAPI *deviceAPI) : FreeDVDemod::~FreeDVDemod() { + stop(); QObject::disconnect( m_networkManager, &QNetworkAccessManager::finished, diff --git a/plugins/channelrx/demodfreedv/freedvdemodsink.cpp b/plugins/channelrx/demodfreedv/freedvdemodsink.cpp index 070ed4048..dc05bdd99 100644 --- a/plugins/channelrx/demodfreedv/freedvdemodsink.cpp +++ b/plugins/channelrx/demodfreedv/freedvdemodsink.cpp @@ -176,6 +176,8 @@ FreeDVDemodSink::~FreeDVDemodSink() { delete SSBFilter; delete[] m_SSBFilterBuffer; + delete[] m_speechOut; + delete[] m_modIn; } void FreeDVDemodSink::feed(const SampleVector::const_iterator& begin, const SampleVector::const_iterator& end) @@ -449,7 +451,7 @@ void FreeDVDemodSink::applyFreeDVMode(FreeDVDemodSettings::FreeDVMode mode) freedv_set_ext_vco(m_freeDV, 0); freedv_set_sync(m_freeDV, FREEDV_SYNC_MANUAL); - int nSpeechSamples = freedv_get_n_speech_samples(m_freeDV); + int nSpeechSamples = freedv_get_n_max_speech_samples(m_freeDV); int nMaxModemSamples = freedv_get_n_max_modem_samples(m_freeDV); int Fs = freedv_get_modem_sample_rate(m_freeDV); int Rs = freedv_get_modem_symbol_rate(m_freeDV); From 6dcc342c10d1b2095a04a516953e6cc684c6b778 Mon Sep 17 00:00:00 2001 From: srcejon Date: Thu, 14 Nov 2024 11:42:53 +0000 Subject: [PATCH 2/2] Fix memory leak and race condition relating to DSP*Engines. Part of #2159 --- sdrbase/dsp/dspengine.cpp | 22 ++++++++++++---------- sdrbase/dsp/dspengine.h | 3 ++- sdrgui/mainwindow.cpp | 29 +++++++++++++++++++++++------ 3 files changed, 37 insertions(+), 17 deletions(-) diff --git a/sdrbase/dsp/dspengine.cpp b/sdrbase/dsp/dspengine.cpp index 47726ea53..da86975a8 100644 --- a/sdrbase/dsp/dspengine.cpp +++ b/sdrbase/dsp/dspengine.cpp @@ -187,39 +187,41 @@ void DSPEngine::removeLastDeviceMIMOEngine() } } -QThread * DSPEngine::removeDeviceEngineAt(int deviceIndex) +QThread *DSPEngine::getDeviceEngineThread(int deviceIndex) { if (deviceIndex >= m_deviceEngineReferences.size()) { return nullptr; + } else { + return m_deviceEngineReferences[deviceIndex].m_thread; } +} - QThread *deviceThread = nullptr; +void DSPEngine::removeDeviceEngineAt(int deviceIndex) +{ + if (deviceIndex >= m_deviceEngineReferences.size()) { + return; + } if (m_deviceEngineReferences[deviceIndex].m_deviceEngineType == 0) // source { DSPDeviceSourceEngine *deviceEngine = m_deviceEngineReferences[deviceIndex].m_deviceSourceEngine; - deviceThread = m_deviceEngineReferences[deviceIndex].m_thread; - deviceThread->exit(); + m_deviceEngineReferences[deviceIndex].m_thread->exit(); m_deviceSourceEngines.removeAll(deviceEngine); } else if (m_deviceEngineReferences[deviceIndex].m_deviceEngineType == 1) // sink { DSPDeviceSinkEngine *deviceEngine = m_deviceEngineReferences[deviceIndex].m_deviceSinkEngine; - deviceThread = m_deviceEngineReferences[deviceIndex].m_thread; - deviceThread->exit(); + m_deviceEngineReferences[deviceIndex].m_thread->exit(); m_deviceSinkEngines.removeAll(deviceEngine); } else if (m_deviceEngineReferences[deviceIndex].m_deviceEngineType == 2) // MIMO { DSPDeviceMIMOEngine *deviceEngine = m_deviceEngineReferences[deviceIndex].m_deviceMIMOEngine; - deviceThread = m_deviceEngineReferences[deviceIndex].m_thread; - deviceThread->exit(); + m_deviceEngineReferences[deviceIndex].m_thread->exit(); m_deviceMIMOEngines.removeAll(deviceEngine); } m_deviceEngineReferences.removeAt(deviceIndex); - - return deviceThread; } void DSPEngine::createFFTFactory(const QString& fftWisdomFileName) diff --git a/sdrbase/dsp/dspengine.h b/sdrbase/dsp/dspengine.h index 3171c4cd9..a3948c092 100644 --- a/sdrbase/dsp/dspengine.h +++ b/sdrbase/dsp/dspengine.h @@ -53,7 +53,8 @@ public: DSPDeviceMIMOEngine *addDeviceMIMOEngine(); void removeLastDeviceMIMOEngine(); - QThread *removeDeviceEngineAt(int deviceIndex); + QThread *getDeviceEngineThread(int deviceIndex); + void removeDeviceEngineAt(int deviceIndex); AudioDeviceManager *getAudioDeviceManager() { return &m_audioDeviceManager; } diff --git a/sdrgui/mainwindow.cpp b/sdrgui/mainwindow.cpp index 1b265f560..15d6e538c 100644 --- a/sdrgui/mainwindow.cpp +++ b/sdrgui/mainwindow.cpp @@ -778,10 +778,19 @@ void RemoveDeviceSetFSM::removeUI() void RemoveDeviceSetFSM::stopEngine() { qDebug() << "RemoveDeviceSetFSM::stopEngine"; - QThread *thread = m_mainWindow->m_dspEngine->removeDeviceEngineAt(m_deviceSetIndex); - if (thread && !thread->isFinished()) // FIXME: Is there a race condition here? We might need to connect before calling thread->exit + QThread *thread = m_mainWindow->m_dspEngine->getDeviceEngineThread(m_deviceSetIndex); + + if (thread) { - connect(thread, &QThread::finished, m_mainWindow, &MainWindow::engineStopped); + bool finished = thread->isFinished(); + + if (!finished) { + connect(thread, &QThread::finished, m_mainWindow, &MainWindow::engineStopped); + } + m_mainWindow->m_dspEngine->removeDeviceEngineAt(m_deviceSetIndex); + if (finished) { + emit m_mainWindow->engineStopped(); + } } else { @@ -801,12 +810,20 @@ void RemoveDeviceSetFSM::removeDeviceSet() DeviceAPI *deviceAPI = m_deviceUISet->m_deviceAPI; delete m_deviceUISet; - if (m_deviceSourceEngine) { + if (m_deviceSourceEngine) + { delete deviceAPI->getSampleSource(); - } else if (m_deviceSinkEngine) { + delete m_deviceSourceEngine; + } + else if (m_deviceSinkEngine) + { delete deviceAPI->getSampleSink(); - } else { + delete m_deviceSinkEngine; + } + else + { delete deviceAPI->getSampleMIMO(); + delete m_deviceMIMOEngine; } delete deviceAPI;