Channel deleted before Channel GUI can result in a crash

[srcejon]

I've been trying to track down some of the reported crashes - here's one possible cause found running with a debug build on windows.

It seems the Channel object is deleted before the Channel GUI. E.g. if we add some debug to the destructors in say the NFMDemod:

2022-07-08 23:32:49.197 (D) NFMDemod::~NFMDemod
2022-07-08 23:32:49.197 (D) DSPDeviceSourceEngine::removeSink: NFMDemod
2022-07-08 23:32:49.198 (D) DSPDeviceSourceEngine::handleSynchronousMessages: DSPRemoveBasebandSampleSink
2022-07-08 23:32:49.198 (D) AudioDeviceManager::removeAudioSink: 0x23fca2cdaf0
2022-07-08 23:32:49.199 (D) MessageQueue::~MessageQueue: message: NFMDemodBaseband::MsgConfigureNFMDemodBaseband was still in queue
2022-07-08 23:32:49.199 (D) MessageQueue::~MessageQueue: message: DSPSignalNotification was still in queue
2022-07-08 23:32:49.200 (D) MessageQueue::~MessageQueue: message: NFMDemodBaseband::MsgConfigureNFMDemodBaseband was still in queue
2022-07-08 23:32:49.200 (D) MessageQueue::~MessageQueue: message: NFMDemodBaseband::MsgConfigureNFMDemodBaseband was still in queue
2022-07-08 23:32:49.201 (D) MessageQueue::~MessageQueue: message: NFMDemodBaseband::MsgConfigureNFMDemodBaseband was still in queue
2022-07-08 23:32:49.201 (D) NFMDemod::~NFMDemod complete
2022-07-08 23:32:49.202 (D) NFMDemodGUI::~NFMDemodGUI

We can see the NFMDemod destructor competes before the NFMDemodGUI destructor is called.

This can occasionally crash if the NFMDemodGUI::tick() method is called inbetween the two, because tick() uses the pointer to the deleted object:

m_nfmDemod->getMagSqLevels(magsqAvg, magsqPeak, nbMagsqSamples);

The same problem occurs in many of the channel GUIs that call getMagSqLevels in a similar way.

[f4exb]

This is very problematic because the destruction action is initiated from the GUI thus it emits a closing signal that the DeviceUISet listens to and upon which it destroys the channel object. There is no other way to know that the channel object should be deleted. When the closing signal is processed the GUI is not yet deleted so this results in the channel object deleted before the GUI.

[f4exb]

Well this seems to work:

--- a/sdrgui/device/deviceuiset.cpp
+++ b/sdrgui/device/deviceuiset.cpp
@@ -723,8 +723,17 @@ void DeviceUISet::handleChannelGUIClosing(ChannelGUI* channelGUI)
     {
         if (it->m_gui == channelGUI)
         {
-            m_deviceSet->removeChannelInstance(it->m_channelAPI);
-            it->m_channelAPI->destroy();
+            ChannelAPI *channelAPI = it->m_channelAPI;
+            m_deviceSet->removeChannelInstance(channelAPI);
+            qDebug("DeviceUISet::handleChannelGUIClosing: channelAPI: %p", channelAPI);
+
+            QObject::connect(
+                channelGUI,
+                &ChannelGUI::destroyed,
+                this,
+                [this, channelAPI](){ this->handleDeleteChannel(channelAPI); }
+            );
+
             m_channelInstanceRegistrations.erase(it);
             break;
         }
@@ -736,6 +745,12 @@ void DeviceUISet::handleChannelGUIClosing(ChannelGUI* channelGUI)
     }
 }
 
+void DeviceUISet::handleDeleteChannel(ChannelAPI *channelAPI)
+{
+    qDebug("DeviceUISet::handleDeleteChannel: channelAPI: %p", channelAPI);
+    channelAPI->destroy();
+}
+
 int DeviceUISet::webapiSpectrumSettingsGet(SWGSDRangel::SWGGLSpectrum& response, QString& errorMessage) const
 {
     return m_spectrumVis->webapiSpectrumSettingsGet(response, errorMessage);

This yields when channel is closed from GUI:

2022-07-21 00:07:11.958 (D) ChannelGUI::closeEvent
2022-07-21 00:07:11.958 (D) NFMDemodGUI::applySettings
2022-07-21 00:07:11.958 (D) NFMDemod::handleMessage: MsgConfigureNFMDemod
2022-07-21 00:07:11.958 (D) NFMDemod::applySettings:  m_inputFrequencyOffset:  0  m_rfBandwidth:  12500  m_afBandwidth:  3000  m_fmDeviation:  5000  m_volume:  1  m_squelchGate:  5  m_deltaSquelch:  false  m_squelch:  -30  m_ctcssIndex:  0  m_ctcssOn:  false  m_dcsOn:  false  m_dcsCode:  23  m_dcsPositive:  false  m_highPass:  true  m_audioMute:  false  m_audioDeviceName:  "System default device"  m_streamIndex:  0  m_useReverseAPI:  false  m_reverseAPIAddress:  "127.0.0.1"  m_reverseAPIPort:  8888  m_reverseAPIDeviceIndex:  0  m_reverseAPIChannelIndex:  0  force:  false
2022-07-21 00:07:11.958 (D) DeviceUISet::handleChannelGUIClosing: NFM Demodulator: 1
2022-07-21 00:07:11.958 (D) DeviceUISet::handleChannelGUIClosing: channelAPI: 0x561a6aba49b8
2022-07-21 00:07:11.959 (D) NFMDemodGUI::~NFMDemodGUI
2022-07-21 00:07:11.959 (D) NFMDemodGUI::~NFMDemodGUI: done
2022-07-21 00:07:11.959 (D) ChannelGUI::~ChannelGUI
2022-07-21 00:07:11.959 (D) ChannelGUI::~ChannelGUI: end
2022-07-21 00:07:11.959 (D) DeviceUISet::handleDeleteChannel: channelAPI: 0x561a6aba49b8
2022-07-21 00:07:11.959 (D) NFMDemod::~NFMDemod
2022-07-21 00:07:11.959 (D) DSPDeviceSourceEngine::removeSink:  NFMDemod
2022-07-21 00:07:11.959 (D) DSPDeviceSourceEngine::handleSynchronousMessages:  DSPRemoveBasebandSampleSink
2022-07-21 00:07:11.959 (D) AudioDeviceManager::removeAudioSink: 0x561a6639fd70
2022-07-21 00:07:11.959 (D) MessageQueue::~MessageQueue: message:  NFMDemodBaseband::MsgConfigureNFMDemodBaseband  was still in queue
2022-07-21 00:07:11.959 (D) MessageQueue::~MessageQueue: message:  DSPSignalNotification  was still in queue
2022-07-21 00:07:11.959 (D) MessageQueue::~MessageQueue: message:  NFMDemodBaseband::MsgConfigureNFMDemodBaseband  was still in queue
2022-07-21 00:07:11.960 (D) MessageQueue::~MessageQueue: message:  NFMDemodBaseband::MsgConfigureNFMDemodBaseband  was still in queue
2022-07-21 00:07:11.960 (D) MessageQueue::~MessageQueue: message:  NFMDemodBaseband::MsgConfigureNFMDemodBaseband  was still in queue
2022-07-21 00:07:11.960 (D) NFMDemod::~NFMDemod: done

[f4exb]

The same issue appears when initiating deletion from the main window exit. In this case it takes a different path via DeviceUISet::freeChannels:

2022-07-21 05:09:09.963 (D) DeviceUISet::freeChannels: destroying channel [sdrangel.channel.nfmdemod]
2022-07-21 05:09:09.963 (D) NFMDemod::~NFMDemod
2022-07-21 05:09:09.963 (D) DSPDeviceSourceEngine::removeSink:  NFMDemod
2022-07-21 05:09:09.963 (D) DSPDeviceSourceEngine::handleSynchronousMessages:  DSPRemoveBasebandSampleSink
2022-07-21 05:09:09.963 (D) AudioDeviceManager::removeAudioSink: 0x562f5cd870c0
2022-07-21 05:09:09.963 (D) MessageQueue::~MessageQueue: message:  NFMDemodBaseband::MsgConfigureNFMDemodBaseband  was still in queue
2022-07-21 05:09:09.963 (D) MessageQueue::~MessageQueue: message:  DSPSignalNotification  was still in queue
2022-07-21 05:09:09.963 (D) MessageQueue::~MessageQueue: message:  NFMDemodBaseband::MsgConfigureNFMDemodBaseband  was still in queue
2022-07-21 05:09:09.963 (D) MessageQueue::~MessageQueue: message:  NFMDemodBaseband::MsgConfigureNFMDemodBaseband  was still in queue
2022-07-21 05:09:09.963 (D) NFMDemod::~NFMDemod: done
2022-07-21 05:09:09.963 (D) NFMDemodGUI::~NFMDemodGUI
2022-07-21 05:09:09.964 (D) NFMDemodGUI::~NFMDemodGUI: done
2022-07-21 05:09:09.964 (D) ChannelGUI::~ChannelGUI
2022-07-21 05:09:09.964 (D) ChannelGUI::~ChannelGUI: end

[f4exb]

The latter is fixed easily along with DeviceUISet::deleteChannel that is initiated from API:

--- a/sdrgui/device/deviceuiset.cpp
+++ b/sdrgui/device/deviceuiset.cpp
@@ -161,8 +161,8 @@ void DeviceUISet::freeChannels()
     for(int i = 0; i < m_channelInstanceRegistrations.count(); i++)
     {
         qDebug("DeviceUISet::freeChannels: destroying channel [%s]", qPrintable(m_channelInstanceRegistrations[i].m_channelAPI->getURI()));
-        m_channelInstanceRegistrations[i].m_channelAPI->destroy();
         m_channelInstanceRegistrations[i].m_gui->destroy();
+        m_channelInstanceRegistrations[i].m_channelAPI->destroy();
     }
 
     m_channelInstanceRegistrations.clear();
@@ -176,8 +176,8 @@ void DeviceUISet::deleteChannel(int channelIndex)
         qDebug("DeviceUISet::deleteChannel: delete channel [%s] at %d",
                 qPrintable(m_channelInstanceRegistrations[channelIndex].m_channelAPI->getURI()),
                 channelIndex);
-        m_channelInstanceRegistrations[channelIndex].m_channelAPI->destroy();
         m_channelInstanceRegistrations[channelIndex].m_gui->destroy();
+        m_channelInstanceRegistrations[channelIndex].m_channelAPI->destroy();
         m_channelInstanceRegistrations.removeAt(channelIndex);
     }
 

[f4exb]

Needs to be done for Features as well (FeatureUISet)