Terminating QProcess in a destructor

Question

A have a problem trying to stop my QProcess in it's parent destructor. Here is my code:

AbstractProcess::~AbstractProcess()
{
    if((m_process->state() == QProcess::Running)
       || (m_process->state() == QProcess::Starting))
    {
        m_process->terminate();
        m_process->waitForFinished();
    }
}

m_process is a pointer to QProcess. In AbstractProcess's constructor I have this code:

 m_process = new QProcess(this);

So, when AbstractProcess is deleted I get to it's destructor and I have a segmentation fault at :

m_process->waitForFinished();

Can anybody tell me what my mistake is?

UPD: As was said below in the comments the problem was not in the code that I provided. Very sorry for that. So I will try to explain what the problem was. Maybe it will help somebody. AbstractProcess as you might guess by the name is an abstract class. So it has some pure virtual functions. On of them is:

virtual void onProcessFinished(int exitCode, QProcess::ExitStatus
exitStatus) = 0;

The full body of my constructor is:

m_process = new QProcess(this);
connect(m_process,static_cast<void(QProcess::*)(int,QProcess::ExitStatus)>(&QProcess::finished),
        this, &AbstractProcess::onProcessFinished);

And now it's obvious that on calling waitForFinished the process emits signal finished and the pure virtual function is called. That leads to undefined behaviour. To fix this I call disconnect before stopping my process. The destructor now looks like this:

AbstractProcess::~AbstractProcess()
{
    disconnect(m_process,static_cast<void(QProcess::*)(int,QProcess::ExitStatus)>(&QProcess::finished),
               this, &AbstractProcess::onProcessFinished)        
    if((m_process->state() == QProcess::Running)
       || (m_process->state() == QProcess::Starting))
    {
        m_process->terminate();
        m_process->waitForFinished();
    }
}

Thanx everybody for your help.

Why do you need to call `m_process->waitForFinished();` after attempt to terminate that process? It's not a `wait for terminated`, though. — vahancho, May 04 '18 at 10:23
@vahancho as far as I know after calling terminate I have to call waitForFinished to let the process finish all the job it has been doing. — Polina Bodnar, May 04 '18 at 10:28
@vahancho Any program should clean up any subprocesses it starts, namely do operating system's "wait" for it. using `waitForFinished` is one way to do that in Qt application (as long as you do it in a place or at a time where event loop hanging until process exits is ok). — hyde, May 04 '18 at 10:34
@PolinaBodnar You should create a MCVE which duplicates the problem. As far as I can see (been a while since I coded similar things, though), the problem is not in the code you are showing. You probably have Undefined Behavior somewhere... — hyde, May 04 '18 at 10:36
I don't see anything obviously wrong with the code shown. Can you provide a [mcve] if at all possible. — G.M., May 04 '18 at 10:37
It is acceptable to answer your own question and mark it as the correct answer. — thuga, May 08 '18 at 09:11

score -1 · Answer 1 · answered May 04 '18 at 10:42

-1

When you call m_process->terminate(); first, there is no guarantee that the process will exit .. yet there is no guarantee that the process will continue to exist because of calling (WM_CLOSE on windows / SIGTERM on Linux) so calling for m_process->waitForFinished(); on a process that might have already been terminated might cause segmentation fault. the right and safe approach is to do the right things in order:

  m_process->waitForFinished();
  m_process->terminate();

answered May 04 '18 at 10:42

Mohammad Kanan

4,452
10
23
47

1

Sorry, but I'm fairly certain that's incorrect. Even if the child process has terminated the `QProcess` instance will maintain enough state that members such as `waitForFinished` can be called safely. – G.M. May 04 '18 at 10:46
@G.M. OK, but It would be great to know the reference for your certainty because I don't find it in documentation. – Mohammad Kanan May 04 '18 at 10:48
2

[`waitForFinished`](https://code.woboq.org/qt5/qtbase/src/corelib/io/qprocess.cpp.html#_ZN8QProcess15waitForFinishedEi) checks the state of the process and returns false if the state is `QProcess::NotRunning`. And the [docs](http://doc.qt.io/qt-5/qprocess.html#details) say *When the process exits, QProcess reenters the NotRunning state (the initial state), and emits finished().* They also say about `waitForFinished`: *Returns true if the process finished; otherwise returns false (if the operation timed out, if an error occurred, or if this QProcess is already finished).* – thuga May 04 '18 at 11:09
Umm. waitForFinished blocks until process is terminated, so terminate is never called, so it will wait indefinitely... IOW the main application will hang. – hyde May 04 '18 at 11:32
@hyde, well thats true if _terminated_ means _finished ... as the doc says (_Returns true if the process finished; otherwise returns false (if the operation timed out, if an error occurred, or if this QProcess is already finished)._ – Mohammad Kanan May 04 '18 at 11:36
@MohammadKanan Why would the process be already finished, if the premise of the question is "stop my QProcess in ... destructor"? Also, calling terminate after waitForFinished is still not useful (at least not in the code shown in this answer). – hyde May 04 '18 at 11:39
@hyde, if you look at the question ... the OP is checking for the process being either `QProcess::Running` or `QProcess::Starting` ..nothing else, so the essence of checking for `waitForFinished()` before terminating ... or even not calling `terminate()` at all! – Mohammad Kanan May 04 '18 at 11:46
Of course terminated means finished. There is no special `terminated` state that leaves the `QProcess` object in some invalid state, which can cause segmentation faults. That would be terrible design. – thuga May 04 '18 at 12:00
@thuga, agree, in that case the OP just needs `waitForFinished()` .. with or without calling `terminate()` ... I don't see any issue calling `terminate` after the process `finished()` .. on the other hand things might not be under control if `terminate()` is called on a _starting_ process ... what would happen to the process resources? – Mohammad Kanan May 04 '18 at 12:11
1

Don't worry, this has been taken into the account by the Qt developers. It even [lets the process emit `started`/`errorOccurred`](https://code.woboq.org/qt5/qtbase/src/corelib/io/qprocess.cpp.html#1134) if you kill it before it goes into `Running` state. – thuga May 04 '18 at 12:49

Terminating QProcess in a destructor

1 Answers1