WaitForReadyRead() in QThread results in memory corruption?

xedrac

I am experiencing memory corruption in a minimal test program I wrote, using Qt 5.3.1 and 64-bit Linux. The program simply communicates to a device over a QUdpSocket in a synchronous manner, using waitForReadyRead(). If I do this in the main event loop, it works perfectly. But if I move my Test object into its own event loop (which I actually need to do), memory corruption ensues.

Am I doing something incorrectly here?

@
Test::Test(const QString &ip, int port)
: QObject(),
_deviceAddress(ip),
_devicePort(port)
{
moveToThread(&_thread);
_socket.bind();
connect(&_socket, &QUdpSocket::readyRead, this, &Test::handleReadyRead);
_thread.start();
}

Test::~Test()
{
_thread.quit();
_thread.wait();
}

void Test::start()
{
while (1) {
sendCommand("ECHO");
}
}

void Test::sendCommand(const QString &command)
{
QByteArray data(command.toLatin1());
data.append("\n");

_socket.writeDatagram(data, _deviceAddress, _devicePort);
if (!_socket.waitForReadyRead(250)) {
    return;
} else {
    while (!_responses.isEmpty()) {
        QString response = _responses.takeFirst();
        Q_UNUSED(response);
    }
}

}

void Test::handleReadyRead()
{
while (_socket.hasPendingDatagrams()) {
qint64 pendingSize = _socket.pendingDatagramSize();
if (pendingSize < 0)
return;

    QByteArray datagram;
    datagram.resize(pendingSize);
    qint64 size = _socket.readDatagram(datagram.data(), pendingSize);
    if (size != pendingSize)
        continue;

    QStringList responses = QString(datagram.data()).split('\n', QString::SkipEmptyParts);
    foreach (const QString &response, responses) {
        _responses << response;
    }
}

}
@

JKSH

Hi, and welcome to the Qt Dev Net!

You created your socket in the main thread, so your socket lives in the main thread. However, you tried to read it from the other thread -- that's why memory corruption occurs. QUdpSocket (and all other QObjects) are not thread safe

The socket should be created in your worker thread, and you must only read/write the socket from your worker thread.

Some other notes:

Read the documentation about "QObject thread affinity":http://qt-project.org/doc/qt-5/QObject.html#thread-affinity to understand how QObjects work with threads
It looks like you create all of your member variables on the "stack", and you don't set their parents. This is dangerous. I recommend that you create QObject-derived member variables using new, and set this as their parent. This ensures that memory management and thread affinities are handled correctly. See "Object Trees & Ownership":http://qt-project.org/doc/qt-5/objecttrees.html for more details.
Test::start() contains an infinite loop. After you call the function, the calling thread will no longer be able to handle any signals or events. Do not mix infinite loops with signals-and-slots.

Question: Which thread calls Test::start()?

xedrac

Thank you for the detailed reply. You, of course, were correct. It looks like I have been doing that incorrectly for several years now. For some reason I thought "moveToThread" would also move all of it's children (despite the fact that I failed to pass it a parent) For whatever reason it never gave me trouble in Qt4, but as soon as I ported over to Qt5 it blew up.

As for start(), I actually forgot to revert that. That was just to test something without worrying about the event loop. I do realize the folly here.

So if a QObject is moved to another thread, ALL of its children that are also QObjects need to be constructed after the move. Is that correct? Can this happen in the constructor, or is it better to have some init function that is called afterwards?

Thank you again.