f4exb / sdrangel

SDR Rx/Tx software for Airspy, Airspy HF+, BladeRF, HackRF, LimeSDR, PlutoSDR, RTL-SDR, SDRplay and FunCube
GNU General Public License v3.0
2.72k stars 420 forks source link

No blocking on the main thread #2159

Open srcejon opened 2 weeks ago

srcejon commented 2 weeks ago

For the WASM port, we need to ensure that there is no blocking on the main thread, otherwise SDRangel will hang.

This is described more in the Qt docs here https://doc.qt.io/qt-6/wasm.html under "Multithreading" and here in the Emscripten docs https://emscripten.org/docs/porting/pthreads.html#blocking-on-the-main-browser-thread

So far, I see two main areas that need to change in SDRangel:

When a device start button is pressed, DSPDeviceSourceEngine::startAcquisition is eventually called and is executed on the main/GUI thread. Its code blocks the thread until the device is started:

DSPAcquisitionStart cmd;
return m_syncMessenger.sendWait(cmd) == StRunning;

However, on WASM, USB devices will unable to start, because libusb calls get deferred to the main thread, so we get a deadlock.

As a workaround, it seems the blocking can be removed with:

DSPAcquisitionStart *cmd = new DSPAcquisitionStart();
m_inputMessageQueue.push(cmd);
return true;

and the RTLSDR driver (and a few others I've quickly looked at) don't appear to require it to block.

Removing this blocking would potentially have a small benefit for all platforms - as it would remove the small GUI freeze that can sometimes be seen when one device is already running and the start button is pressed on another device that takes a little while to initialize.

The above might not be the best way to implement this change - it's just the first thing I tried - and similar would also need to be implemented for TX and MIMO devices - and also for the stopAcquistion method.

Currently DeviceInput::applySettings is also often called from the main/GUI thread, and the libusb calls result in a hang. I worked around this in RTLSDRInput::applySettings by moving all of the rtlsdr driver calls in to a separate thread. Similar would need to be done for other devices.

Again, this change is probably beneficial to all platforms, as discussed a while back in this issue: https://github.com/f4exb/sdrangel/issues/1522

srcejon commented 1 week ago

DSPAcquisitionInit can hang as well, the second time time an RTL SDR is started. Similar workaround applies.

And forgot to mention, there needs to be coded added to DSPDeviceSourceEngine::handleInputMessages() to handle those messages.

(RTL SDR driver crash on stop has been worked around by using rtlsdr_read_sync instead of rtlsdr_read_async)

srcejon commented 1 week ago

So, it appears there are cases where we do need to wait some of these DSPDeviceSourceEngine commands to complete.

One such case is in void MainWindow::removeDeviceSet. It starts by calling deviceEngine->stopAcquistion(); which results in RTLSDRInput::stop being called. It later on calls deleteSampleSourcePluginInstanceInput, which calls ~RTLSDRInput which also calls RTLSDRInput::stop(). In this instance, it's on the main thread, so it will hang if the previous stop hasn't completed.

So, we need a way for DSPDeviceSourceEngine and friends to indicate when they have completed, but without blocking. As an experiment, I've tried adding an acquistionStopped signal to DSPDeviceSourceEngine. MainWindow::removeDeviceSet then needs to be broken up in to two parts: First calling stopAcquistion - then when it gets the acquistionStopped signal, it can continue the rest of the device set deletion. Seems to work.

srcejon commented 1 week ago

It does get a little trickier though. In mainwindow.cpp, there are several places where all device sets are removed as part of some other sequential code, such as:

// Do X...
// Remove device sets
while (m_deviceUIs.size() > 0) {
    removeLastDeviceSet();
}
// Do Y...

This needs to be broken up in to separate steps, with a non-blocking wait for a signal after each device set is removed, before trying to remove the next.

Trying to think of the cleanest way to write this code, I came across Qt's State Machine API: https://doc.qt.io/qt-5/statemachine-api.html - which seems like a reasonable way to sequence different bits of code using signals. I will give it a try.

Any thoughts on any of this?

f4exb commented 1 week ago

I only had a quick look at the Qt state machine and I find it quite interesting. When the events and event handlers start to be rather complex it probably helps keep things tidy. State Machine is a well known design pattern.