My program crashes if QProcess is restarted directly when it has finished

I have created a program that is supposed to run a process over and over again, so that the program is restarted as soon as it it finished. Here is the code that handles that:

Code:

---

// From process_handler.h
class process_handler : public QObject
{
    Q_OBJECT
 
public:
    process_handler();
 
private:
    QProcess p;
 
public slots:
    void start_process();
 
private slots:
    void process_state_changed();
};

---

Code:

---

// From process_handler.cpp
process_handler::process_handler()
{
    connect(&p, SIGNAL(stateChanged(QProcess::ProcessState)), this, SLOT(process_state_changed()));
}
 
void process_handler::process_state_changed()
{
    if (p.state() == QProcess::Starting) {
        cout << endl;
        cout << "Process is starting up..." << endl;
    }
    if (p.state() == QProcess::Running) {
        cout << "Process is now running." << endl;
    }
    if (p.state() == QProcess::NotRunning) {
        cout << "Process is finished running." << endl;
 
        /* Start a new process */
        //start_process();
        QTimer::singleShot(0, this, SLOT(start_process()));
    }
}
 
void process_handler::start_process()
{
    p.start("program");
}

---

"program" is a program that takes a few seconds to run and thex terminates. As you see I have used a QTimer::singleShot to call start_process() from process_state_changed(). If I don't use it, but call start_process() directly, the text

Code:

---

Process is starting up...
Process is now running.
Process is finished running.
 
Process is starting up...
Process is now running.

---

is written to cout and then I get a segmentation fault. Does anyone know why this segmentation fault occurs? I would like it if I didn't have to use a singleShot, since that will make coding much more complicated later on.

Not a whole lot of error or status checking in that code. Do you even know that "program" runs when you start it the first time? How do you start it the first time?

Also don't know why you aren't using the QProcess:ProcessState argument of the stateChanged signal in your slot instead of repeatedly calling p.state().

You're not assigning a parent QObject to your QProcess member variable, nor are you assigning one to your process_handler instance, even though both inherit from QObject.

I doubt that any of this is the cause of your problem. My guess is that there is some side effect of running "program" the first time that results in the crash the second time around. Try substituting something trivial for your own "program" and see if that changes things.

Try running your program in your debugger and, when it crashes, using the stack backtrace to locate where it crashes from. There's nothing in the code you posted that is an obvious source, and it runs fine here with "sleep 5" as the command.

Thank you for your answers.

Quote:

---

Originally Posted by d_stranz

Not a whole lot of error or status checking in that code. Do you even know that "program" runs when you start it the first time? How do you start it the first time?

---

What do you mean with error or status checking? I have done some debugging to try to see where the bug is and I'm checking the status of the process in process_state_changed(), but I'm not sure that is what you mean. I know the program is running because it lets me wait at least 10 seconds before it terminates. What do you mean by how I start it? I start it by calling p[idx].start("program");, where "program.exe" is a program I have put in the project folder.

Quote:

---

Originally Posted by d_stranz

Also don't know why you aren't using the QProcess:ProcessState argument of the stateChanged signal in your slot instead of repeatedly calling p.state().

---

Originally I had a lot of QProcesses stored in a vector, and all of them had to be connected to the process_handler::process_state_changed slot. But since the normal connection function will not let the slot know which object it is that has emitted the signal, I had to use a QSignalMapper to map the different processes to their indexes and connect their stateChanged signals with the map() slot of the QSignalMapper and connect the mapped(int) signal of the QSignalMapper to my process_handler::process_state_changed slot. So now I didn't have the QProcess::ProcessState anymore and I had to find out which state the process had changed to by calling p.state(), where idx was the index of the process.

This example is just a simplification of it to generate the same behaviour with less code.

Quote:

---

Originally Posted by d_stranz

You're not assigning a parent QObject to your QProcess member variable, nor are you assigning one to your process_handler instance, even though both inherit from QObject.

---

Why should I? My QProcess object will be destroyed anyway when the process_handler is destroyed, and the the process_handler object is a direct member of the main window so it will automatically be destroyed when the main window is destroyed. But maybe I should update the process_handler constructor with a parameter that takes a pointer to the parent.

Quote:

---

Originally Posted by d_stranz

I doubt that any of this is the cause of your problem. My guess is that there is some side effect of running "program" the first time that results in the crash the second time around. Try substituting something trivial for your own "program" and see if that changes things.

---

I tried replacing it with "ls", but I still have the same problem; it crashes after the second time the process state has just changed to QProcess::Running. If I use the QSingleShot approach, the program calls the process over and over again without crashing.

Quote:

---

Originally Posted by ChrisW67

Try running your program in your debugger and, when it crashes, using the stack backtrace to locate where it crashes from. There's nothing in the code you posted that is an obvious source, and it runs fine here with "sleep 5" as the command.

---

The segmentation fault occurs in qtimer.cpp, which just contains a bunch of assembly code. Then I have to backtrace through eleven more Qt files, before ending up in a Windows dll (user32.dll). Then it backtraces through two more Qt files and two more Window dlls. At place 18 it just says "??" in the function field, which I have no idea why it does. May the program stack has been corrupted?

Quote:

---

Originally Posted by Yes

I would like it if I didn't have to use a singleShot, since that will make coding much more complicated later on.

---

Why not connect the signal QProcess::finished(int) to your slot?

You are saying the actual code posted above, with a minimal main() fails. You don't say what line 18 lists as the source file and line. Is it your code? Has your program been built in debug mode?

The vast majority of segmentation faults are caused by programmers using dangling or uninitialised pointers (or undefined references). There are no pointers in your example code, except for a pointer to p that can only become invalid during program tear-down. It seems likely that the actual code you are running is more complex and is suffering a dangling pointer issue.

I agree with ChrisW67 - since your actual code is undoubtedly more complex than what you have posted, the error probably lies elsewhere in the code and the crash in QTimer is simply a red herring brought about by the fact that the stack or heap have been corrupted earlier on.

Quote:

---

But since the normal connection function will not let the slot know which object it is that has emitted the signal

---

Not true. See QObject::sender().

Quote:

---

Originally Posted by norobro

Why not connect the signal QProcess::finished(int) to your slot?

---

That is one way to do it too, but now I wanted to use the stateChanged signal. It's possible that I'll change to using the finished signal later on if I notice that I don't need to know when it changes to states other than QProcess::NotRunning.

Quote:

---

Originally Posted by ChrisW67

You are saying the actual code posted above, with a minimal main() fails. You don't say what line 18 lists as the source file and line. Is it your code? Has your program been built in debug mode?

---

What do you mean with line 18? There isn't any line 18 in the code I posted with code on it... Yes, this is my code and I have tried running it in debug mode (see my last post).

Quote:

---

Originally Posted by ChrisW67

The vast majority of segmentation faults are caused by programmers using dangling or uninitialised pointers (or undefined references). There are no pointers in your example code, except for a pointer to p that can only become invalid during program tear-down. It seems likely that the actual code you are running is more complex and is suffering a dangling pointer issue.

---

I don't think I have any dangling pointer anywhere since I never use the keyword new (or malloc or anything like that), but maybe some pointer is created somewhere that I am not aware of. I had better post the entire code here:

main.cpp:

Code:

---

#include <QtGui/QApplication>
#include "mainwindow.h"
 
int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    MainWindow w;
    w.show();
 
    return a.exec();
}

---

mainwindow.h:

Code:

---

#ifndef MAINWINDOW_H
#define MAINWINDOW_H
 
#include <QMainWindow>
#include "process_handler.h"
 
namespace Ui {
class MainWindow;
}
 
class MainWindow : public QMainWindow
{
    Q_OBJECT
 
public:
    explicit MainWindow(QWidget *parent = 0);
    ~MainWindow();
 
private:
    Ui::MainWindow *ui;
    process_handler s;
};
 
#endif // MAINWINDOW_H

---

mainwindow.cpp:

Code:

---

#include <QTimer>
#include "mainwindow.h"
#include "ui_mainwindow.h"
 
MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow)
{
    ui->setupUi(this);
 
    /* Begin start processes when the windows has finished loading */
    QTimer::singleShot(0, &s, SLOT(start_process()));
}
 
MainWindow::~MainWindow()
{
    delete ui;
}

---

process_handler.h:

Code:

---

#ifndef PROCESS_HANDLER_H
#define PROCESS_HANDLER_H
 
#include <QProcess>
 
class process_handler : public QObject
{
    Q_OBJECT
 
public:
    process_handler(QObject *parent = 0);
 
private:
    QProcess p;
 
public slots:
    void start_process();
 
private slots:
    void process_state_changed();
};
 
#endif // PROCESS_HANDLER_H

---

process_handler.cpp:

Code:

---

#include <iostream>
using std::cout;
using std::endl;
#include <QTimer>
#include "process_handler.h"
 
process_handler::process_handler(QObject *parent) :
    QObject(parent)
{
    connect(&p, SIGNAL(stateChanged(QProcess::ProcessState)), this, SLOT(process_state_changed()));
}
 
void process_handler::process_state_changed()
{
    if (p.state() == QProcess::Starting) {
        cout << endl;
        cout << "Process is starting up..." << endl;
    }
    if (p.state() == QProcess::Running) {
        cout << "Process is now running." << endl;
    }
    if (p.state() == QProcess::NotRunning) {
        cout << "Process is finished running." << endl;
 
        /* Start a new process */
        start_process();
        //QTimer::singleShot(0, this, SLOT(start_process()));
    }
}
 
void process_handler::start_process()
{
    //p.start("program");
    p.start("ls");
}

---

As can be seen, main.cpp is unmodified, and only one line each has been added to mainwindow.cpp and two lines to mainwindow.h, dealing with the process_handler object. As I said before, it is possible to make a workaround by using a QTimer::SingleShot (substitude line 27 in process_handler.cpp for line 26), but this is a hack I would definitely prefer not having to use.

Quote:

---

Originally Posted by d_stranz

Not true. See QObject::sender().

---

Thanks! I didn't know about that function, it seems handy.

Quote:

---

Originally Posted by Yes

. . . but now I wanted to use the stateChanged signal. . . .

---

Then connecting both signals should work:

Code:

---

process_handler::process_handler(QObject *parent) :
    QObject(parent)
{
    connect(&p, SIGNAL(stateChanged(QProcess::ProcessState)), this, SLOT(process_state_changed(QProcess::ProcessState)));
    connect(&p, SIGNAL(finished()), this, SLOT(start_process()));
}
 
void process_handler::process_state_changed(QProcess::ProcessState state)
{
    if (state == QProcess::Starting) {
        cout << endl;
        cout << "Process is starting up..." << endl;
    }
    if (state == QProcess::Running) {
        cout << "Process is now running." << endl;
    }
    if (state == QProcess::NotRunning) {
        cout << "Process is finished running." << endl;
 
        /* Start a new process */
//        start_process();
        //QTimer::singleShot(0, this, SLOT(start_process()));
    }
}
 
. . .

---

Quote:

---

Originally Posted by norobro

Then connecting both signals should work

---

Yes, but I don't want to start a new process every time an old process has finished. Basically, I want to run processes untill some condition is reached, and then I want to stop running processes, so start_process() shouldn't be directly connected to the finished signal since then I wouldn't be able to stop running processes. The question is still why my program crashes if I call start_process() directly from process_state_changed() without using a QTimer::SingleShot...

Apparently notRunning != finished(). On Linux I get the following warning on the command line:

Quote:

---

QSocketNotifier: Multiple socket notifiers for same socket 15 and type Read

---

Try the following code:

Code:

---

#include <QCoreApplication>
#include <QProcess>
#include <QDebug>
#include <QTimer>
 
class process_handler : public QObject
{
    Q_OBJECT
public:
    process_handler() {
        connect(&p, SIGNAL(stateChanged(QProcess::ProcessState)), this, SLOT(process_state_changed(QProcess::ProcessState)));
        connect(&p, SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(process_finished(int, QProcess::ExitStatus)));
        start_process();
    }
public slots:
    void process_state_changed(QProcess::ProcessState state) {
        if (state == QProcess::Starting)
            qDebug() << "Process is starting up...";
        if (state == QProcess::Running)
            qDebug() << "Process is now running." ;
        if (state == QProcess::NotRunning){
            qDebug() << "Process is finished running.";
            start_process();
            //QTimer::singleShot(0, this, SLOT(start_process()));
        }
    }
    void process_finished(int  , QProcess::ExitStatus ){
        qDebug() << "received finished signal";
    }
    void start_process() {
        p.start("ping 1.1.1.1 -c 1 -W 3");  // from your previous thread
    }
private:
    QProcess p;
};
 
int main(int argc, char *argv[]) {
    QCoreApplication app(argc, argv);
    process_handler ph;
    app.exec();
}
#include "main.moc"

---

The output on my box is:

Quote:

---

Process is starting up...
Process is now running.
Process is finished running.
QSocketNotifier: Multiple socket notifiers for same socket 15 and type Read
Process is starting up...
received finished signal

---

I haven't looked at the source code but what appears to happen is the new process assigns a QSocketNotifier to the same
socket as the old process and the old process deletes the socket. My app just idles in the event loop after the output
above. Maybe Windows segfaults?

Okay, so maybe I need to wait untill I receive the finished signal before I can start up a new process.

Yes, I'm using Windows XP and I do receive a segmentation fault message. The output of your code in the command prompt (the terminal in Windows) before the segfault message comes is

Quote:

---

Process is starting up...
Process is now running.
Process is finished running.
Process is starting up...
Process is now running.
received finished signal

---

I don't get the same QSocketNotifier message as you do, though. I still only get to see Qt files containing a lot of assembly code when backtracing.

However, if I move the line containing start_process(); from the slot receiving the stateChanged signal to the slot receiving the finished signal, the program runs as expected. I think I will just stick to the QTimer::singleShot() workaround in my case, though. Anyway, thank you very much!