qt serial port memory leak

Question

I use the following code to talk to a USB-serial port device:

#include "masterthread.h"
#include <QtSerialPort/QSerialPort>
#include <QTime>
#include "Windows.h"
#include "Psapi.h"
#include <QDebug>
QT_USE_NAMESPACE

MasterThread::MasterThread(QObject *parent)
: QThread(parent), waitTimeout(0), quit(false)
{
}

MasterThread::~MasterThread()
{
    mutex.lock();
    quit = true;
    cond.wakeOne();
    mutex.unlock();
    wait();
}

void MasterThread::run()
{
    bool currentPortNameChanged = false;

    QSerialPort serial;
    serial.setPortName("COM3");
    serial.setBaudRate(57600);
    serial.setStopBits(static_cast<QSerialPort::StopBits>(1));
    serial.setDataBits(static_cast<QSerialPort::DataBits>(8));
    serial.setParity(static_cast<QSerialPort::Parity>(0));
    serial.open(QIODevice::ReadWrite);

    //Tell the serial port connected device to start talking
    //--------------------------------------
    const char init[] = { 0x0d, 0x0d, 0x0d };
    serial.write(init, sizeof(init));
    const char* cmd = "mavlink stop\n";
    serial.write(cmd, strlen(cmd));
    serial.write(init, 2);
    cmd = "uorb start";
    serial.write(cmd, strlen(cmd));
    serial.write(init, 2);
    cmd = "sh /etc/init.d/rc.usb\n";
    serial.write(cmd, strlen(cmd));
    serial.write(init, 4);
    serial.waitForBytesWritten(100);

    int i = 0;
    int j = 0;
    forever
    {

        //Write test data out
        //-----------------------------
        QByteArray test(2000, 't');
        serial.write(test);
        bool check = serial.waitForBytesWritten(100);
        if (!check)
        {
            qDebug() << "FAIL: " << j++;
        }

        if (serial.waitForReadyRead(20))
        {
            QByteArray responseData = serial.readAll();
            while (serial.waitForReadyRead(10))
                responseData += serial.readAll();

            QString response(responseData);
            qDebug() << response;
        }
        QThread::msleep(20);

        //Print memory usage
        //---------------------------------------------------
        if (i++ % 10 == 0)
        {
            PROCESS_MEMORY_COUNTERS memcount;
            if (!GetProcessMemoryInfo(GetCurrentProcess(), &memcount, sizeof(memcount))) return;
            qDebug()<<"----------------------------" << memcount.WorkingSetSize / 1024 << "KB memory used";
        }
    } // end foever

    qDebug() << "Exiting forever loop";
}

with a simple main.cpp as:

#include <QApplication>
#include "masterthread.h"

int main(int argc, char *argv[])
{
    QApplication app(argc, argv);

    MasterThread thread;
    thread.start();
    return app.exec();
}

But the memory usage keeps increasing, like 5~10MB per hour as if there are some leakage. The device is suppose to be connected for days and weeks...

What am I doing wrong here? I am on Qt5.6 windows7 debug

Answer 1

Many Qt Components have an implicit dependency on its event loop.

While you are starting the main threads event loop with the call to app.exec(); you are not handling events generated by the QObjects created in the QThread MasterThread thread;. The details and nuances of Event handling in Qt are very well described on this page: https://wiki.qt.io/Threads_Events_QObjects#Threads_and_QObjects

But the solution boils down to: if you want to be able to process queued up Qt events in a thread where you are processing some long-running task you should call QCoreApplication::processEvents(); from time to time. This will prevent Qt events from endlessly queueing up.

Answer 2

EDITED after looking on the code Qt 5.7,5.6,5.5 and reading docs.

As an answer is already accepted, I would just add some thoughts here as it's too long for comments.

Keep things short - an answer you accepted is wrong..

There are two sides of the story. And as SO answers often taken 'as it is as long as they work' I'd like to explain myself...

If you look on a code provided - there is nothing wrong with it. All objects are properly stack allocated and should be destroyed automatically.

Point is that QtSerial uses deleteLater() and then a question - how to delete those allocations properly.

If any module/object/code uses deleteLater() it requires an event loop, if deleteLater() called on a thread without event loop, object will be deleted after thread is terminated. As long as there is no event loop running for code above, processEvents will no work.. actually processEvents() is not something which is used for this, because a whole idea to return from the context which is called deleteLater() and have a next run, and that's checked in the Qt Source Code, so calling processEvent() straight after without incrementing loop count will do nothing at all, that's why answer you accepted is totally wrong.

Conclusion:

If any object requires event loop running it should be EXPLICITELY stated in the documentation as there is nothing wrong in using QIODevice in sync mode outside event loop.

So at my opinion,point is - its a bug in the QT Serial itself which I suggest you report.

In general it's really wrong practice for Qt to run never-ending loops.. It's much much better and cleaner to use QObject Worker tactic which is pushed to the thread, have proper even loop running etc.

For small 'threaded' tasks it's much better to use QtConcurrent.

Proper Workaround:

you will have a thread with properly running event loop and a timer firing at 20ms to do your things

// main thread:
class Worker: public QObject {
    public:
       Worker();

    public slots:

       onInit() {
          // initialize everything
          startTimer(20);
       }

    protected:

       void timerEvent(..) {
          // do your things every 20ms
       }   
  }

  ...

  QThread * pWorkerThread = new QThread();
  pWorkerThread->setObjectName(QString("Serial"));

  Worker * pWorker = new Worker();
  Worker->setObjectName(QString("Common Storage Impl"));
  Worker->moveToThread(WorkerThread);

  connect(pWorkerThread, SIGNAL(started()), pWorker, SLOT(onInit()));
  connect(pWorkerThread, SIGNAL(finished()), pWorker, SLOT(deleteLater()));
  connect(pWorkerThread, SIGNAL(finished()), pWorkerThread, SLOT(deleteLater()));

  pWorkerThread->start();

  ...