Executing QProcess in QThread: memory leak

[sitesv]

Trying to execute 'ping' from QThread. There is a memory leak after this.

void TMyThread::run(){
      while(work){
            QProcess myProcess;
            myProcess.start(exe_path,arguments);
            myProcess.waitForFinished(500);
            myProcess.close();
            msleep(1000);
      }
}

I tried to use a pointer and delete statement... The result is the same...

[mrjj]

Hi
How do you know there is a leak ?

[sitesv]

@mrjj said in Executing QProcess in QThread: memory leak:

Hi
How do you know there is a leak ?

By system monitor...
A leak is about 4k/sec. I have been checking for a few minutes...

[mrjj]

@sitesv
Ok. it might internally allocate something
that is first freed on app termination.

If you reuse the same QProcess
(let it be a member of the TMyThread, allocate once)

Does it still then still leak ?

[sitesv]

@mrjj said in Executing QProcess in QThread: memory leak:

Does it still then still leak ?

Yes, there is still a leak.
QProcess myProcess; moved into the class.

[sitesv]

@mrjj
If I would use this code in while(1) of the 'main' function of the console app - there is NO memory leak.

[mrjj]

@sitesv
but you only create the thread once , so it should be the same ?

[sitesv]

@mrjj said in Executing QProcess in QThread: memory leak:

but you only create the thread once ,

Yes.

[mrjj]

Hi
Hmm really strange then.
Is this on windows ?
I did something similar on linux and didnt notice it would leak 4K

[Christian Ehrlicher]

@sitesv said in Executing QProcess in QThread: memory leak:

By system monitor...

This is no valid memory leak checker. Use a correct to to find leaks like e.g. valgrind.

[sitesv]

@mrjj said in Executing QProcess in QThread: memory leak:

Is this on windows ?

Linux

@Christian-Ehrlicher
Is it correct, If a memory value is increasing for a few minutes?

[Christian Ehrlicher]

@sitesv A system monitor is not a valid tool for such an investigation. It may be a hint that there is something wrong but nothing more. The system monitor only shows what the OS gave the process, but the OS may not free resources immediately but only when it needs it later on so use a correct tool for this.

[sitesv]

@Christian-Ehrlicher
Checked app by Valgrind. There is no memory leak...
Thank you. I hope there really are no errors.

[mrjj]

@sitesv
I would leave it running for like 48 hours and see it would stabilize.
If its related to actually running the process on Os level, it might only allocate up to some max.

[Christian Ehrlicher]

Since Qt cleans up a lot of stuff during exit maybe also stop the application started with valgrind with CTRL+C and take a look at the reachable memory in the valgrind output to see if there is something interesting (start valgrind with '--show-reachable=yes')

[sitesv]

@mrjj, @Christian-Ehrlicher
Thank you!

[sitesv]

Hi!
Made a simple test:

void mythread::run(){
    while(work){
        bool boolRESULT;
        QString output_str;
        QByteArray output;
        QStringList output_strlst;
        QTextCodec *codec;
        QString exe_path = "/bin/ping";;
        QStringList arguments;
        arguments << "127.0.0.1" << "-c" << "1";

        if(myProcess == nullptr) myProcess = new QProcess;
        myProcess->start(exe_path, arguments);
        myProcess->waitForFinished(500);

        output = myProcess->readAll();
        output_str = codec->toUnicode(output);
        output_strlst = output_str.split("\r\n");

        myProcess->close();
         
        boolRESULT = false;
        for (int i = 0; i < output_strlst.count(); i++){
            boolRESULT = output_strlst[i].contains("ttl",Qt::CaseInsensitive);
            if (boolRESULT) break;
        }
        emit setStatus(boolRESULT);
   
        msleep(1000);
    }
}

Valgrind output:
...
==23633== 2,400 bytes in 15 blocks are still reachable in loss record 2,049 of 2,114
==23633== at 0x4C2BBEF: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==23633== by 0x6267251: resizeSignalVector (qobject_p.h:297)
==23633== by 0x6267251: QObjectPrivate::addConnection(int, QObjectPrivate::Connection*) (qobject.cpp:330)
==23633== by 0x6268A18: QMetaObjectPrivate::connect(QObject const*, int, QMetaObject const*, QObject const*, int, QMetaObject const*, int, int*) (qobject.cpp:3453)
==23633== by 0x626FD58: QObject::connect(QObject const*, char const*, QObject const*, char const*, Qt::ConnectionType) (qobject.cpp:2911)
==23633== by 0x61BFB1F: QProcessPrivate::startProcess() (qprocess_unix.cpp:384)
==23633== by 0x61BA34D: QProcessPrivate::start(QFlagsQIODevice::OpenModeFlag) (qprocess.cpp:2246)
==23633== by 0x61BA582: QProcess::start(QString const&, QStringList const&, QFlagsQIODevice::OpenModeFlag) (qprocess.cpp:2094)
==23633== by 0x10BBBF: mythread::run() (mythread.cpp:19)
==23633== by 0x605A3B4: QThreadPrivate::start(void*) (qthread_unix.cpp:342)
==23633== by 0x6A164A3: start_thread (pthread_create.c:456)
==23633== by 0x75B1D0E: clone (clone.S:97)
...

mythread.cpp 19 line is:

myProcess->start(exe_path, arguments);

My log by system monitor:

• 16:50 4300 KB
• 17:06 4790 KB
• 17:20 5200 KB
• 17:30 5552 KB

Any ideas?

[KroMignon]

@sitesv said in Executing QProcess in QThread: memory leak:

Made a simple test:

You are aware that doing so (subclassing QThread and redefining slot run()), there will be not QEventLoop to handle signals/slots for all QObjects running in this thread?
For example QTimer::singleShot() will not work.

[JonB]

@sitesv
In addition, from @KroMignon no QEventLoop also means no deleting/disposing/freeing of Qt objects.

On a side note: nothing to do with your findings, but for the record

 arguments << "127.0.0.1" << "-c 1";

is wrong: -c & 1 are separate arguments, it ought to be << "-c" << "1". You are lucky it works as one argument.

[sitesv]

@KroMignon @JonB
Are there any methods to do this correctly?
If I do this in the main thread: the app will freeze while QProcess execution - this is unacceptable for me.

[KroMignon]

@sitesv said in Executing QProcess in QThread: memory leak:

Are there any methods to do this correctly?

I don't say this is incorrect.
I depends what you want to achieve.
If you don't need an event queue, it works. If you are using signals/slots, this will not work.

I do not know what is the purpose of this code extract, so maybe it is the best way to do.

[sitesv]

@KroMignon said in Executing QProcess in QThread: memory leak:

If you don't need an event queue, it works. If you are using signals/slots, this will not work.

I don't know, but from this qthread in my main app I emit signal... and it is emitted... slot is from the main thread. QThread starts by start() method.
The problem for me is memory leak only.

[sitesv]

@JonB said in Executing QProcess in QThread: memory leak:

In addition, from @KroMignon no QEventLoop also means no deleting/disposing/freeing of Qt objects.

QCoreApplication::processEvents would help?

@JonB said in Executing QProcess in QThread: memory leak:

is wrong: -c & 1 are separate arguments, it ought to be << "-c" << "1". You are lucky it works as one argument.

Thank you! Fixed.

[KroMignon]

@sitesv said in Executing QProcess in QThread: memory leak:

I don't know, but from this qthread in my main app I emit signal... and it is emitted... slot is from the main thread. QThread starts by start() method.
The problem for me is memory leak only.

For my comprehension of QObject and QThread (cf Threads and QObjects), your implementation is not really Qt friendly.

If I right understand what you want to achieve: you want to run "ping" a regular interval and be informed when ping has failed.

To do this, I would create a QObject class which handles the ping requests and eventually moves this class instance to a dedicated thread to avoid lock on main thread.

Something like:

class PingTester : public QObject
{
    Q_OBJECT

public:
    explicit PingTester(QObject* parent = nullptr): QObject(parent), m_timer(new QTimer(this))
    {
        m_timer->setInterval(30*1000); // per default 30 seconds
        m_timer->setSingleShot(false);
        connect(m_timer, &QTimer::timeout, this, &PingTester::doPing);
    }

    void setInterval(int interval) { m_timer->setInterval(interval); }

public slots:
     void start()
     {
         if(QThread::currentThrea() != thread())
         {
              QTimer::singleShot(this, 0, &PingTester::start);
              return;
         }
         if(!m_timer->isActive())
             m_timer->start();
     }
     void stop()
     {
         if(QThread::currentThrea() != thread())
         {
              QTimer::singleShot(this, 0, &PingTester::stop);
              return;
         }
         if(m_timer->isActive())
             m_timer->stop();
     }
     
private slots:
    void doPing();

signals:
    void pingFailure();
    void pingSuccess();

private:
     QTimer* m_timer;
};

[SGaist]

Hi,

Can you explain why exactly do you need that ping ?

[sitesv]

@SGaist
ping needs to check the availability of pc from the internal network. I thought that this is the simplest way to do this.

[Christian Ehrlicher]

But why a thread then?

[sitesv]

@KroMignon Thank you for your variant.
I have some Qs;
Why you are using QTimer for this purpose?
doPing() is represents my code with using QProcess?
This class should create in the main thread?

[sitesv]

@Christian-Ehrlicher said in Executing QProcess in QThread: memory leak:

But why a thread then?

I need to check availability constantly during the program. I thought the QThread is a good way to do this...

[Christian Ehrlicher]

QProcess is asynchronous.

[sitesv]

@Christian-Ehrlicher said in Executing QProcess in QThread: memory leak:

QProcess is asynchronous.

"waitForFinished" is not help?

[Christian Ehrlicher]

@sitesv said in Executing QProcess in QThread: memory leak:

"waitForFinished" is not help?

Why do you want to make a asynchronous object blocking? Please read the QProcess docs and use the proper signals.

[SGaist]

The thing is: you just ping in the void, you do not even do anything with that information so why even ping ?

[sitesv]

@SGaist said in Executing QProcess in QThread: memory leak:

The thing is: you just ping in the void, you do not even do anything with that information so why even ping ?

I just skipped this code, it is unimportant... It receives an answer, analyzes it, and emits a signal to the main thread. And this is works.

[JonB]

@sitesv
As @Christian-Ehrlicher has said. You are using this thread to run a QProcess, and you call waitForFinished() on it. You would be better just running QProcess asynchronously from your main thread, and acting on signals. Then perhaps any thread memory issue will go away.

[sitesv]

@JonB
I will try it.

[SGaist]

@sitesv said in Executing QProcess in QThread: memory leak:

@SGaist said in Executing QProcess in QThread: memory leak:

The thing is: you just ping in the void, you do not even do anything with that information so why even ping ?

I just skipped this code, it is unimportant... It receives an answer, analyzes it, and emits a signal to the main thread. And this is works.

Well, do not skip that kind of details when asking questions, it can hide the issue you are asking help for and it does not allow people to understand what is really happening.

[sitesv]

@SGaist code updated

[KroMignon]

@sitesv said in Executing QProcess in QThread: memory leak:

Why you are using QTimer for this purpose?

I use QTimer here to make a passive wait. When the interval is reached, the QTimer fires the timeout() event. This will not lock the event queue. QThread::msleep() is an active wait ==> CPU usage for nothing and no signals/slots handling possible for the used thread!

doPing() is represents my code with using QProcess?

Yes, but I would not use a pointer for the QProcess.

This class should create in the main thread?

As you like. The QTimer instance has this as parent, moving the PingTester instance to another thread, will also move the QTimer instance to the same thread.

[KroMignon]

@sitesv For example a possible implementation which will not lock the event loop could be:

void PingTester::doPing()
{		
    QProcess ping;
    QEventLoop l; // used for passive wait until process finished

    ping.setProcessChannelMode(QProcess::MergedChannels);
    connect(&ping, &QProcess::finished, 
            [this, &l, &ping]() {
                QString output(ping.readAll());
                if(output.contains("ttl",Qt::CaseInsensitive))
                    emit pingSuccess();
                else
                    emit pingFailure();
                // exit event loop on process end
                l.exit();
            });
    // starting ping request
    ping.start("/bin/ping", QStringList() << "127.0.0.1" << "-c" << "1");

    // wait until ping finished
    l.exec();

    // start next ping (timer should be configured as single shot)
    m_timer->start();
}