Important: Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

QProcess, ready ?



  • I've written a class that is suppose to monitor a supplied application.

    The intention is that if the application isn't running it will launch a new instance of it. The class installs a thread for each instance of the class, as long as the process returns a PID it will query the processes running on the system to check that the process is still running.

    The function that checks the process list:

    bool clsModule::blnGetPIDs(const QString& crstrApp, lstPIDs& lstOfPIDs) {
        if ( crstrApp.isEmpty() == true ) {
            return false;
        }
        FILE* pFP = popen("ps -A", "r");
        lstOfPIDs.clear();
    
        if ( pFP != nullptr ) {
            QString strApp(crstrApp);
            int intLastDelimiter = strApp.lastIndexOf(QDir::separator());
    
            if ( intLastDelimiter > 0 ) {
                strApp = strApp.mid(intLastDelimiter);
            }
            char szBuffer[1024];
            while( fgets(szBuffer, sizeof(szBuffer), pFP) ) {
                QStringList slstCols = QString(szBuffer).split(clsDebugService::msccSpace), slstPInf;
    
                for( QString strCol : slstCols ) {
                    if ( strCol.trimmed().isEmpty() == true ) {
                        continue;
                    }
                    slstPInf << strCol;
                }
                if ( slstPInf.length() >= 4 ) {
                    QString strFound(slstPInf[3].trimmed());
    
                    if ( strFound.endsWith(strApp) == true ) {
                        lstOfPIDs.push_back(static_cast<qint64>(slstPInf[0].toLong()));
                    }
                }
            }
            pclose(pFP);
        }
        return (lstOfPIDs.size() > 0) ? true : false;
    }
    

    The module thread:

        mpthrdPing = QThread::create([this] {
            qint64 int64ThisPID = 0;
            while( mblnExit == false ) {
        //Pause between checks
                QThread::msleep(clsModule::mscuint16DelayBetweenChecks);
        //Is the process ID known yet?
                if ( int64ThisPID == 0 ) {
                    if ( state() != QProcess::NotRunning ) {
                        int64ThisPID = processId();
                    }
                    if ( int64ThisPID == 0 ) {
    //PID not known yet, can't check it yet!
                        continue;
                    }
                }
                const QString cstrModule = program();
                lstPIDs lstOfPIDs;
                if ( clsModule::blnGetPIDs(cstrModule, lstOfPIDs) != true ) {
                    continue;
                }
                bool blnFound = false;
                foreach( qint64 int64PID, lstOfPIDs ) {
                    if ( int64ThisPID == int64PID ) {
                        blnFound = true;
                        break;
                    }
                }
                if ( blnFound != true ) {
        //This module is no longer running, remove it from the internal list
                    removeModule(cstrModule);
                    break;
                }
            }
            if ( state() != QProcess::NotRunning ) {
                terminate();
            }
            delete this;
        });
        mpthrdPing->start();
    

    I have a problem here, the debugger is stopping in QProcess functions, I'm not really sure why. Is there a simpler / easier way to achieve this and is there something already available?



  • @jsulm , state is a function in QProcess, I compare it for QProcess::NotRunning, because he alternatives are Starting and Running.

    https://doc.qt.io/qt-5/qprocess.html#ProcessState-enum

    It appears to be fixed now by re-organising the class.


  • Lifetime Qt Champion

    @SPlatten said in QProcess, ready ?:

    debugger is stopping in QProcess functions, I'm not really sure why

    Do you mean there is a crash? If so please tell what kind of crash and post stack trace.



  • @jsulm , it stops on the source in QProcess.


  • Lifetime Qt Champion

    @SPlatten said in QProcess, ready ?:

    state() != QProcess::NotRunning

    What is state() and why do you compare its return value with QProcess::NotRunning?


  • Lifetime Qt Champion

    @SPlatten said in QProcess, ready ?:

    it stops on the source in QProcess.

    Again: please post stack trace



  • @jsulm , state is a function in QProcess, I compare it for QProcess::NotRunning, because he alternatives are Starting and Running.

    https://doc.qt.io/qt-5/qprocess.html#ProcessState-enum

    It appears to be fixed now by re-organising the class.



  • @SPlatten said in QProcess, ready ?:

    I have a problem here, the debugger is stopping in QProcess functions, I'm not really sure why. Is there a simpler / easier way to achieve this and is there something already available?

    Once again, your code is locking the event loop of the thread.
    There are always active waits, so no chance to get signals processed by the event loop.
    Qt is a asynchronous framework, you have to think in this way to get all the power of it.

    I am quite sure that this is the main problem of your code.



  • @KroMignon How am I locking the event loop? If I am doing it wrong, can you suggest the correct way to do this?



  • @SPlatten
    Where does your thread allow the event loop to run?



  • @SPlatten said in QProcess, ready ?:

    How am I locking the event loop? If I am doing it wrong, can you suggest the correct way to do this?

    I have told you this many times, but you don't want to lose time to read documentation. So you lose time with always doing same errors.

    You have a while loop that never ends, with QThread::msleep(), which is an active wait. So you never gives the event loop time to do his job.
    In this way, no signals/slots can be processed. Signals/slots are the base mechanism of Qt.
    There are use to update object properties, etc.

    As side notes:

    • as Qt is asynchronous, most of the time there is no need to create additional threads
    • Subclassing QThread is in most case the worth solution for multi-threading, it is better to create a QObject base class and move this class or create the instance in the new thread


  • @KroMignon , my understanding was that the event loop is running in the main application thread and that worker threads are just that...



  • @SPlatten said in QProcess, ready ?:

    my understanding was that the event loop is running in the main application thread and that worker threads are just that

    Just an extract of very, very basis documentation (https://doc.qt.io/qt-5/threads-qobject.html):

    Each thread can have its own event loop. The initial thread starts its event loop using QCoreApplication::exec(), or for single-dialog GUI applications, sometimes QDialog::exec(). Other threads can start an event loop using QThread::exec(). Like QCoreApplication, QThread provides an exit(int) function and a quit() slot.

    An event loop in a thread makes it possible for the thread to use certain non-GUI Qt classes that require the presence of an event loop (such as QTimer, QTcpSocket, and QProcess). It also makes it possible to connect signals from any threads to slots of a specific thread.



  • @KroMignon From what I can see it looks like all I'm missing is a call to exec() in the thread body.


  • Lifetime Qt Champion

    @SPlatten said in QProcess, ready ?:

    I'm missing is a call to exec() in the thread body

    That would not solve the problem with endless loop with a sleep() inside...



  • @SPlatten said in QProcess, ready ?:

    From what I can see it looks like all I'm missing is a call to exec() in the thread body.

    You are looking in the wrong direction!
    It is complicated to explain it in comprehensive way without writing a too long text.

    To made it short:

    1. There are situations where a event loop is not required, for example you are processing big calculation.
    2. When you want to use signals/slots or using Qt classes which requires signals/slots to work like QTimer, QProcess or QTcpSocket, then you should ensure event loop can be handle by the thread which hold those instances.

    As I can not understand what is the purpose of the thread you have create, I can't tell you how to do it in a more Qt friendly way.
    Your code always looks "over-complicated" to me.

    If you can describe in short sentences what you want to do, I could show you how I would do it.



  • In plain English, the purpose of the class is to:

    1. Start the specified process
    2. Check at regular intervals if the process is still running and if not launch it again.

    The processes that are launched check to see if the master / server / launching application is still running and if not they self terminate.



  • @KroMignon , question, when a thread is created like so:

    QThread* pThread = QThread::create([this] {
    /*Do something*/
    ...
    });
    

    no QThread::exec() is available to call, so how do you maintain the event loop?



  • @SPlatten said in QProcess, ready ?:

    In plain English, the purpose of the class is to:

    Start the specified process
    Check at regular intervals if the process is still running and if not launch it again.

    The processes that are launched check to see if the master / server / launching application is still running and if not they self terminate.

    My way to do it would be:

    class Monitor : public QObject
    {
        Q_OBJECT
        
    public:
        explicit Monitor(QObject *parent=nullptr):QObject(parent){}
        
        void addProcess(QProcess* newProcess)
        {
            m_procList.add(newProcess);
            
            connect(newProcess, &QProcess::stateChanged, this, 
                   [this, newProcess](QProcess::ProcessState newState){
                       if(newState == QProcess::NotRunning)
                       {
                           newProcess->start();
                           emit restartProcess(newProcess);
                       }
                   });
            if(newProcess->state() == QProcess::NotRunning)
                newProcess->start();
        }
        
    signals:
        void restartProcess(QProcess* proc);
    
    private:
        QList<QProcess*> m_procList;
    };
    
    


  • @KroMignon , thank you, I will take a look.


  • Lifetime Qt Champion

    @SPlatten said in QProcess, ready ?:

    Start the specified process
    Check at regular intervals if the process is still running and if not launch it again.

    Why do you need a thread for that? QProcess is asynchronous.



  • @SPlatten
    As @jsulm has said, why the thread at all? Yesterday we discussed this in https://forum.qt.io/topic/120854/executing-qprocess-in-qthread-memory-leak. May I suggest you have a read through that, and see if it doesn't apply equally to your situation.



  • @jsulm , so I've learnt today...I've implemented @KroMignon solution.



  • @SPlatten said in QProcess, ready ?:

    so I've learnt today...I've implemented @KroMignon solution.

    Your welcome, always keep in mind that Qt is an asynchronous framework. And reading documentation is never a bad idea ;-)

    Thread a not always a good solution to solve problems... Most of the time they creates bigger one!



  • @KroMignon , thank you, I'm contracting and have been working flat out, I agree its good to read the documentation, its a balance, between getting the job done and learning the technology.


Log in to reply