MainWindow: Add FSMs to avoid blocking on GUI Thread #2274

srcejon · 2024-10-10T15:06:38Z

This PR adds some FSMs to MainWindow in order wait for events, while not blocking on the GUI Thread as discussed in #2159

DSPDeviceEngine: Add signals to indicate when commands have been processed.
DSPDeviceSourceEngine: Fix small memory leak.
DSPEngine::removeDeviceEngineAt: Remove wait to avoid blocking thread. Return QThread to get QThread :finished signal.
DSPEngine::addDeviceEngine: Don't call deleteLater for device*Engine, as these objects are deleted manually in MainWindow, which will crash if deleteLater called first.

I don't quite understand some of the code in MainWindow::sampleSinkCreate and MainWindow::sampleMIMOCreate. In the original code, it does:

const PluginInterface::SamplingDevice *samplingDevice = DeviceEnumerator::instance()->getMIMOSamplingDevice(deviceIndex);

but then:

const PluginInterface::SamplingDevice *selectedDevice = DeviceEnumerator::instance()->getRxSamplingDevice(selectedDeviceIndex);

Why getRxSamplingDevice rather than getMIMOSamplingDevice for the second call? I've left that as is for now.

DSPDevice*Engine: Add signals to indicate when commands have been processed. DSPDeviceSourceEngine: Fix small memory leak. DSPEngine::removeDeviceEngineAt: Remove wait to avoid blocking thread. Return QThread to get finished signal. DSPEngine::addDevice*Engine: Don't call deleteLater for device*Engine, as these objects are deleted manually in MainWindow, which will crash if deleteLater called first.

…ndow::RemoveDeviceSetFSM::removeUI will do it

sonarcloud · 2024-10-10T15:13:57Z

Quality Gate failed

Failed conditions
7.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

f4exb · 2024-10-10T15:22:36Z

Don't call deleteLater for device*Engine, as these objects are deleted manually in MainWindow ...

I am just wondering why the objects are deleted manually in MainWindow.
Note: this is probably old code when I had not much experience with Qt and threads so it can be challenged possibly.

srcejon · 2024-10-10T15:47:50Z

Don't call deleteLater for device*Engine, as these objects are deleted manually in MainWindow ...

I am just wondering why the objects are deleted manually in MainWindow. Note: this is probably old code when I had not much experience with Qt and threads so it can be challenged possibly.

I think previously it wasn't a problem, as the device removal all happen in one go. Now the FSMs return control part way through the removal it gives the deleteLater a chance to be called and then occasionally it would crash on the delete in MainWindow. This seemed to depend on whether I just closed the device or was exiting the program. Obviously feel free to change it however you want.

f4exb · 2024-10-12T06:44:40Z

It seems I cannot close the TestSink device using the cross (X) icon. This is what I see in the log:

2024-10-12 08:44:58.766 (D) DeviceGUI::closeEvent
2024-10-12 08:44:58.766 (D) MainWindow::removeDeviceSet: index: 1
2024-10-12 08:44:58.766 (D) RemoveDeviceSetFSM::stopAcquisition
2024-10-12 08:44:58.766 (D) DSPDeviceSinkEngine::stopGeneration

The generation stops but the GUI is still there.

EDit: this is not systematic. In this case it happens in the second workspace (W1).

srcejon · 2024-10-12T07:05:32Z

Ok, I'll take a look

Note that in DeviceGUi I changed the code to ignore the close event, as otherwise we would get a double delete (as MainWindow tries to delete it as well)

f4exb · 2024-10-12T07:23:46Z

It is a bit strange when I tried to reproduce the issue on a new instance of SDRangel the TestSink closed as it should.

srcejon · 2024-10-12T07:24:47Z

Note that in DeviceGUi I changed the code to ignore the close event, as otherwise we would get a double delete (as MainWindow tries to delete it as well)

This is because all the subclasses of DeviceGUI call setAttribute(Qt::WA_DeleteOnClose, true);

It is a bit strange when I tried to reproduce the issue on a new instance of SDRangel the TestSink closed as it should.

I just tried on Windows a few times and it worked OK.

srcejon · 2024-10-12T07:33:24Z

It seems I cannot close the TestSink device using the cross (X) icon. This is what I see in the log:

2024-10-12 08:44:58.766 (D) DeviceGUI::closeEvent
2024-10-12 08:44:58.766 (D) MainWindow::removeDeviceSet: index: 1
2024-10-12 08:44:58.766 (D) RemoveDeviceSetFSM::stopAcquisition
2024-10-12 08:44:58.766 (D) DSPDeviceSinkEngine::stopGeneration

My log shows:

2024-10-12 08:28:08.304 (D) DeviceGUI::closeEvent
2024-10-12 08:28:08.304 (D) MainWindow::removeDeviceSet: index: 1
2024-10-12 08:28:08.304 (D) RemoveDeviceSetFSM::stopAcquisition
2024-10-12 08:28:08.304 (D) DSPDeviceSinkEngine::stopGeneration
2024-10-12 08:28:08.304 (D) DSPDeviceSinkEngine::handleInputMessages: message: DSPGenerationStop
2024-10-12 08:28:08.304 (D) DSPDeviceSinkEngine::gotoIdle
2024-10-12 08:28:08.306 (D) RemoveDeviceSetFSM::removeSink
2024-10-12 08:28:08.306 (D) DSPDeviceSinkEngine::removeSpectrumSink:  SpectrumVis
2024-10-12 08:28:08.306 (D) DSPDeviceSinkEngine::handleInputMessages: message: DSPRemoveSpectrumSink
2024-10-12 08:28:08.306 (D) RemoveDeviceSetFSM::removeUI
2024-10-12 08:28:08.306 (D) DeviceGUI::~DeviceGUI
2024-10-12 08:28:08.306 (D) DeviceGUI::~DeviceGUI: end
2024-10-12 08:28:08.425 (D) RemoveDeviceSetFSM::stopEngine

So after stopGeneration, I'd expect to see DSPDeviceSinkEngine::handleInputMessages to handle DSPGenerationStop.

If that message isn't handled, it will not generate the generationStopped signal, and thus the RemoveDeviceSetFSM will not progress - as it waits for it, before going on to the next state.

So I guess the question is why might DSPDeviceSinkEngine not be handling messages?

f4exb · 2024-10-12T07:53:54Z

The problem only happens when there is a modulator e.g the SSB modulator. Firstly when I stop the TestSink the SSB modultor stops but when I start again I cannot see the SSB modulator start debug message:

2024-10-12 09:50:16.761 (D) TestSinkOutput::handleMessage: MsgStartStop:  stop
2024-10-12 09:50:16.761 (D) DSPDeviceSinkEngine::stopGeneration
2024-10-12 09:50:16.761 (D) DSPDeviceSinkEngine::handleInputMessages: message: DSPGenerationStop
2024-10-12 09:50:16.761 (D) DSPDeviceSinkEngine::gotoIdle
2024-10-12 09:50:16.761 (D) TestSinkOutput::stop
2024-10-12 09:50:16.761 (D) TestSinkWorker::stopWork
2024-10-12 09:50:16.761 (D) DSPDeviceSinkEngine::gotoIdle: stopping  SSBMod
2024-10-12 09:50:16.761 (D) SSBMod::stop

2024-10-12 09:50:35.647 (D) TestSinkOutput::handleMessage: MsgStartStop:  start
2024-10-12 09:50:35.647 (D) DSPDeviceSinkEngine::initGeneration
2024-10-12 09:50:35.647 (D) DSPDeviceSinkEngine::startGeneration

Then if I stop again and try to close with (X) I get to the situation above:

2024-10-12 09:52:18.240 (D) TestSinkOutput::handleMessage: MsgStartStop:  stop
2024-10-12 09:52:18.240 (D) DSPDeviceSinkEngine::stopGeneration

2024-10-12 09:52:28.185 (D) DeviceGUI::closeEvent
2024-10-12 09:52:28.185 (D) MainWindow::removeDeviceSet: index: 0
2024-10-12 09:52:28.185 (D) RemoveDeviceSetFSM::stopAcquisition
2024-10-12 09:52:28.185 (D) DSPDeviceSinkEngine::stopGeneration

srcejon · 2024-10-12T08:33:58Z

Ok, I can see the problem with the SSB and AM modulators, - but not the AIS or 802.15.4 modulators...

srcejon · 2024-10-12T08:47:17Z

This problem seems unrelated to this patch, as I think it also occurs in the release build of 7.22.1.

Both SSB and AM Mod had threading changes in 7.22.1, so possibly something there.

srcejon · 2024-10-12T08:55:36Z

It looks like m_thread->wait(); in SSBMod::stop never returns.

f4exb · 2024-10-12T09:16:38Z

also I forgot to mention that I was still on your branch at commit e3ec8759e [srcejon/freq_scanner] Remove unused variable so if you added something more I missed it. I am going to try on the latest master...

srcejon · 2024-10-12T09:25:06Z

For AISMod, after we call:

m_thread->exit();

m_thread->isFinished() returns true and m_thread->wait() returns immediately.

However, for SSBMod, this isn't the case. m_thread->isFinished() returns false and m_thread->wait() hangs.

It may be something to do with SSBMod being an audio sink, which the AISMod isn't.

If I remove:

DSPEngine::instance()->getAudioDeviceManager()->addAudioSink(m_source.getFeedbackAudioFifo(), getInputMessageQueue());

From SSBModBaseband::SSBModBaseband then the thread exits.

srcejon · 2024-10-12T10:26:46Z

The thread isn't exiting, because it is blocked in SSBModBaseband::processFifo m_channelizer->pull() - it doesn't seem to return from that call.

If I check the Audio Input button in the SSBMod, I don't see anything in the spectrum, with the 7.22.1 release, as I do with 7.22.0. (Maybe related to #2273?)

f4exb · 2024-10-12T15:06:05Z

Maybe related to #2273

I don't think so... More probably due to this commit: 75d40c8 as you suggested before. It does not seem possible to handle the thread life cycle in the start / stop methods. It has to be tightly linked to SSBModBaseband.

Edit: what I find strange is that on the demod side (SSBDemod or WDSPRx) it works with the thread handled in start / stop.

srcejon added 7 commits October 10, 2024 13:57

Fix warnings,

3df1ce2

DeviceGUI::closeEvent - Don't automatically delete the GUI, as MainWi…

5ac2081

…ndow::RemoveDeviceSetFSM::removeUI will do it

Adds comment

a04f84f

Cleanup

41f2b04

Don't load defaults if loading a preset

d5dc848

Remove unused variable

e3ec875

f4exb merged commit 4dcf050 into f4exb:master Oct 12, 2024
3 of 4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MainWindow: Add FSMs to avoid blocking on GUI Thread #2274

MainWindow: Add FSMs to avoid blocking on GUI Thread #2274

srcejon commented Oct 10, 2024

sonarcloud bot commented Oct 10, 2024

f4exb commented Oct 10, 2024 •

edited

Loading

srcejon commented Oct 10, 2024

f4exb commented Oct 12, 2024 •

edited

Loading

srcejon commented Oct 12, 2024 •

edited

Loading

f4exb commented Oct 12, 2024

srcejon commented Oct 12, 2024 •

edited

Loading

srcejon commented Oct 12, 2024 •

edited

Loading

f4exb commented Oct 12, 2024

srcejon commented Oct 12, 2024

srcejon commented Oct 12, 2024 •

edited

Loading

srcejon commented Oct 12, 2024

f4exb commented Oct 12, 2024

srcejon commented Oct 12, 2024 •

edited

Loading

srcejon commented Oct 12, 2024

f4exb commented Oct 12, 2024 •

edited

Loading

MainWindow: Add FSMs to avoid blocking on GUI Thread #2274

MainWindow: Add FSMs to avoid blocking on GUI Thread #2274

Conversation

srcejon commented Oct 10, 2024

sonarcloud bot commented Oct 10, 2024

Quality Gate failed

f4exb commented Oct 10, 2024 • edited Loading

srcejon commented Oct 10, 2024

f4exb commented Oct 12, 2024 • edited Loading

srcejon commented Oct 12, 2024 • edited Loading

f4exb commented Oct 12, 2024

srcejon commented Oct 12, 2024 • edited Loading

srcejon commented Oct 12, 2024 • edited Loading

f4exb commented Oct 12, 2024

srcejon commented Oct 12, 2024

srcejon commented Oct 12, 2024 • edited Loading

srcejon commented Oct 12, 2024

f4exb commented Oct 12, 2024

srcejon commented Oct 12, 2024 • edited Loading

srcejon commented Oct 12, 2024

f4exb commented Oct 12, 2024 • edited Loading

f4exb commented Oct 10, 2024 •

edited

Loading

f4exb commented Oct 12, 2024 •

edited

Loading

srcejon commented Oct 12, 2024 •

edited

Loading

srcejon commented Oct 12, 2024 •

edited

Loading

srcejon commented Oct 12, 2024 •

edited

Loading

srcejon commented Oct 12, 2024 •

edited

Loading

srcejon commented Oct 12, 2024 •

edited

Loading

f4exb commented Oct 12, 2024 •

edited

Loading