Calling QEventLoop in QRunnable (to connect to multiple devices)

Question

I have a QthreadPool to connect to upto 5 serial devices at the same time. each serial device is defined as:

struct serialDevice
{
    std::shared_ptr<QSerialPort>  port;
    QByteArray                    portData;
};

Inside a GUI and upon clicking a button, I call a serial device manager serialManager->startAcquisition(portNames) as follows:

#include "serialmanager.h"
serialManager::serialManager(QObject *parent) : QObject(parent)
{
    pool = new QThreadPool();
    pool->setMaxThreadCount(5);
}

void serialManager::startAcquisition(QStringList pNames)
{
    foreach (QString port, pNames)
    {
        this->stablishConnection(port);
    }
}

void serialManager::stablishConnection(QString pName)
{
    ComPort *workerPort = new ComPort();
    workerPort->setAutoDelete(true);
    workerPort->setName(pName);

    connect(this, SIGNAL(finished()), workerPort, SLOT(onTaskeFinished()));

    pool->start(workerPort);
}

Once a device is connected I poll the device. The thread terminates when a stop function is called. I expect that readyRead() signal is being emitted, in order to fill inside the sensor->portData:

#include "serialmanager.h"
#include <QEventLoop>

ComPort::ComPort(QObject *parent) : QObject(parent)
{
}

void ComPort::run()
{
    QEventLoop loop;
    connect(this, SIGNAL(finished()), &loop, SLOT(quit()));

    sensor       = std::make_shared<serialDevice> ();
    sensor->port = std::make_shared<QSerialPort> ();
    sensor->port->setPortName(this->portName);
    sensor->port->setBaudRate(QSerialPort::Baud19200);
    sensor->port->setDataBits(QSerialPort::Data8);
    sensor->port->setFlowControl(QSerialPort::NoFlowControl);
    sensor->port->setParity(QSerialPort::NoParity);
    sensor->port->setStopBits(QSerialPort::OneStop);

    QString strPort;
    if (sensor->port->open(QIODevice::ReadWrite) == true)
    {
        connect(sensor->port.get(), SIGNAL(error(QSerialPort::SerialPortError)), this, SLOT(onErrorReceived()));
        connect(sensor->port.get(), SIGNAL(readyRead()), this, SLOT(onReadyRead()));
    }
    else
    {
        strPort = "> " + sensor->port->portName() + " device is not streaming: " + sensor->port->errorString();
        emit this->newMsgAvailable(strPort);
        emit this->finished();
    }
    loop.exec();
}

void ComPort::onReadyRead()
{
    QByteArray bytes = this->sensor->port->readAll();
    this->sensor->portData.append(bytes);
}

void ComPort::onTaskeFinished()
{
    QString strPort = "> Finished streaming at " + sensor->port->portName();
    emit newMsgAvailable(strPort);
    emit this->finished();
}

void ComPort::onErrorReceived()
{
    QString strPort = "> Data stream on "  + sensor->port->portName() + " failed: " + sensor->port->errorString();
    emit newMsgAvailable(strPort);
}

Problem definition: The readyRead() signal was not emitted. I have read here, that to process events in a QRunnable, a thread needs to have its own event loop. So I included QEventLoop loop to the Run() function of each ComPort. Now readyRead() is being triggered and I receive data from each port for a while, and then all of a sudden the application crashes with a segmentation fault as:

0   VerifierDisableFaultInjectionExclusionRange C:\WINDOWS\syswow64\verifier.dll                0x53457378  
1   VerifierDisableFaultInjectionExclusionRange C:\WINDOWS\syswow64\verifier.dll                0x53457495  
2   VerifierDisableFaultInjectionExclusionRange C:\WINDOWS\syswow64\verifier.dll                0x5345b651  
3   VerifierCheckPageHeapAllocation             C:\WINDOWS\syswow64\verifier.dll                0x53460b12  
4   ??                                          C:\WINDOWS\SysWOW64\vfbasics.dll                0x533d179f  
5   msvcrt!free                                 C:\WINDOWS\SysWOW64\msvcrt.dll                  0x7509b0f9  
6   ??                                                                                          0x19120000  
7   ??                                          C:\WINDOWS\SysWOW64\vfbasics.dll                0x533d26a5  
8   QArrayData::deallocate                                                              130 
9   QTypedArrayData<char>::deallocate                                                   234     0x6b9cbcfd  
10  QByteArray::resize                                                                  1448    0x6b79bc23  
11  QRingBuffer::append                                                                 383     0x645893ae  
12  QSerialPortPrivate::_q_completeAsyncRead                                            529     0x6458523f  
13  QSerialPort::qt_static_metacall                                                     353     0x64582dae  
14  QMetaObject::activate                                                               3681    0x6b955027  
15  QMetaObject::activate                                                               3547    0x6b95498c  
16  QWinEventNotifier::activated                                                        134     0x6b9ad529  
17  QWinEventNotifier::event                                                            241     0x6b976af1  
18  QApplicationPrivate::notify_helper                                                  3499    0xf33df61   
19  QApplication::notify                                                                2952    0xf33ba13   
20  QCoreApplication::notifyInternal                                                    935     0x6b929eee  
... <More>

It's impossible to answer your question without seeing more of the code. I question the use of multiple threads for multiple serial connections; your use of a blocking `QRunnable` is unnecessary. Just shove all those serial ports into one dedicated `QThread`. Make sure you don't block in your code and you'll be OK. If you need to do a lot of computation based on the serial data, do it using `QtConcurrent::run`, it'll be spread across the global pool's threads automagically. See e.g. [this answer](http://stackoverflow.com/a/32595398/1329652) for async data processing inspiration. — Kuba hasn't forgotten Monica, Dec 02 '15 at 15:42
@Kuba I used a `QThreadPool`, since I did not know a way to handle the `readyRead()` of multiple serial objects in the same `QThread`. Could you please give me an example of how to implement it in one thread? — QuestionMark, Dec 02 '15 at 16:03
You connect your slot to a particular `QSerialPort` instance - the threads don't figure anywhere in your `connect` call. This is the simplest example: `{ QSerialPort a, b; a.connect(&a, &QSerialPort::readyRead, []{ qDebug() << "a got data"; }); b.connect(&b, &QSerialPort::readyRead, []{ qDebug() << "b got data"; }); }`. I don't understand the problem at all. Make sure that you're not deriving from `QThread` [other than perhaps to make it safe](http://stackoverflow.com/a/25230470/1329652). — Kuba hasn't forgotten Monica, Dec 02 '15 at 16:16
One simple approach is to have a single `QObject` that encapsulates the port and the device protocol. It's not aware of any other instances of itself, so the "problem" of there being multiples of them can't be a problem. You can move all instances of this object to a dedicated thread: `{ QApplication app(...); CommHandler a,b,c; Thread thread; a.open("portname"); b.open("portname"); c.open("portname"); a.moveToThread(&thread); b.moveToThread(&thread); c.moveToThread(&thread); return app.exec(); }` — Kuba hasn't forgotten Monica, Dec 02 '15 at 16:24
@Kuba Thank you for the example. If I cannot call the `ComHandler` objects in the main thread (e.g. they live in a MainWindow instance), how should I define the `QEventLoop` and its `exec()`? — QuestionMark, Dec 02 '15 at 18:59
If you think you need an explicit `QEventLoop`, you're probably doing things wrong. `QThread::run` already spins an event loop for you. All of your code should run fine in the main(gui) thread anyway. "If I cannot call the ComHandler objects in the main thread" I don't understand what you mean by *calling something in the main thread*, and how does `MainWindow` figure into any of that. You do understand that any `QWidget`-deriving class runs in the main thread by definition, right? You seem to be fundamentally confused about something here, but I can't quite figure out what. — Kuba hasn't forgotten Monica, Dec 02 '15 at 19:15
@Kuba Great! This was reassuring. I think, I did not thoroughly understand [this post] [1] then! [1]:http://stackoverflow.com/a/20321568/4058363 — QuestionMark, Dec 02 '15 at 19:23
A `QObject` can be moved to any thread directly. `QRunnable` is meant to be used when you have legacy code that doesn't use `QObject`. Today there's very little need for `QRunnable`: to run async jobs on worker threads, use the Qt Concurrent framework. Otherwise, for a fixed thread-to-object mapping, you simply create an explicit thread, and move your objects there. — Kuba hasn't forgotten Monica, Dec 02 '15 at 20:38
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/96890/discussion-between-questionmark-and-kuba-ober). — QuestionMark, Dec 03 '15 at 14:11

Calling QEventLoop in QRunnable (to connect to multiple devices)

0 Answers0