From 6dcc342c10d1b2095a04a516953e6cc684c6b778 Mon Sep 17 00:00:00 2001
From: srcejon <jon@beniston.com>
Date: Thu, 14 Nov 2024 11:42:53 +0000
Subject: [PATCH] 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;