Terminate an Ongoing Qprocess That Is Running Inside a Qthread

Terminate an ongoing QProcess that is running inside a QThread?

It is really bad style to use a QThread to manage a running process. I'm seeing it again and again and it's some fundamental misunderstanding about how to write asynchronous applications properly. Processes are separate from your own application. QProcess provides a beautiful set of signals to notify you when it has successfully started, failed to start, and finished. Simply hook those signals to slots in an instance of a QObject-derived class of yours, and you'll be all set.

It's bad design if the number of threads in your application can exceed significantly the number of cores/hyperhtreads available on the platform, or if the number of threads is linked to some unrelated runtime factor like number of running subprocesses.

See my other other answer.

You can create QProcess on the heap, as a child of your monitoring QObject. You could connect QProcess's finished() signal to its own deleteLater() slot, so that it will automatically delete itself when it's done. The monitoring QObject should forcibly terminate any remaining running processes when it gets itself destroyed, say as a result of your application shutting down.

Further to the question was how to execute uncontrollably long running functions, say database queries for which there's no asynchronous API, with minimal impact, when interspersed with things for which there is good asynchronous API, such as QProcess.

A canonical way would be: do things synchronously where you must, asynchronously otherwise. You can stop the controlling object, and any running process, by invoking its deleteLater() slot -- either via a signal/slot connection, or using QMetaObject::invokeMethod() if you want to do it directly while safely crossing the thread boundary. This is the major benefit of using as few blocking calls as possible: you have some control over the processing and can stop it some of the time. With purely blocking implementation, there's no way to stop it short of using some flag variables and sprinkling your code with tests for it.

The deleteLater() will get processed any time the event loop can spin in the thread where a QObject lives. This means that it will get a chance between the database query calls -- any time when the process is running, in fact.

Untested code:

class Query : public QObject
{
  Q_OBJECT
public:
  Query(QObject * parent = 0) : QObject(parent) {
    connect(process, SIGNAL(error(QProcess::ProcessError)), SLOT(error()));
    connect(process, SIGNAL(finished(int,QProcess::ExitStatus)), SLOT(finished(int,QProcess::ExitStatus)));
  }
  ~Query() { process.kill(); }
  void start() {
    QTimer::singleShot(0, this, SLOT(slot1()));
  }
protected slots:
  void slot1() {
    // do a database query
    process.start(....);
    next = &Query::slot2;
  }
protected:
  // slot2 and slot3 don't have to be slots
  void slot2() {
    if (result == Error) {...}
    else {...}
    // another database query
    process.start(...); // yet another process gets fired
    next = &Query::slot3;
  }
  void slot3() {
    if (result == Error) {...}
    deleteLater();
  }

protected slots:
  void error() {
    result = Error;
    (this->*next)();
  }
  void finished(int code, QProcess::ExitStatus status) {
    result = Finished;
    exitCode = code;
    exitStatus = status;
    (this->*next)();
  }
private:
  QProcess process; 
  enum { Error, Finished } result;
  int exitCode;
  QProcess::ExitStatus exitStatus;
  void (Query::* next)();
};

Personally, I'd check if the database that you're using has an asynchronous API. If it doesn't, but if the client library has available sources, then I'd do a minimal port to use Qt's networking stack to make it asynchronous. It would lower the overheads because you'd no more have one thread per database connection, and as you'd get closer to saturating the CPU, the overheads wouldn't rise: ordinarily, to saturate the CPU you'd need many, many threads, since they mostly idle. With asynchronous interface, the number of context switches would go down, since a thread would process one packet of data from the database, and could immediately process another packet from a different connection, without having to do a context switch: the execution stays within the event loop of that thread.

How to stop running non-loop QThread correctly?

You can't forcibly terminate a thread; all you can do is ask it to quit, and then wait for it to exit of its own accord. (there does exist a QThread::terminate() method, but you shouldn't use it in production code, as it will cause problems: for example, if the thread had a mutex locked at the moment it got terminated, that mutex will remain locked forever, and your program will deadlock and freeze up the next time it attempts to lock that mutex).

So you have two options: either figure out a way to ask the Python thread to quit, or use a QProcess object (or something equivalent to it) to run the Python code in a child process instead of inside a thread. The benefit of running the Python code in a separate process is that you can safely kill() a child process -- since the child process doesn't share any state with your GUI process, and the OS will automatically clean up any resources allocated by the child process, there is no problem with the child process leaving mutexes locked or other resources un-freed.

If you'd rather ask the Python thread (or process) politely to exit instead of simply bringing down the hammer on it, you could do so via a networking interface; for example, you could create a TCP connection between your GUI code and the Python event loop, and the Python event loop could periodically do a non-blocking read on its end of the TCP connection. Then when your GUI wants the Python loop to exit, the GUI could close its TCP socket, and that would cause the Python loop's call to read() to return 0 (aka EOF), which the Python loop would know means "time to exit", so it could then exit voluntarily.

QProcess doesn't kill/terminate the process if be defined on heap

tl;dr

In both cases, the signals aren't delivered; in the first case the destructor kills the process, in the second one it doesn't even have a chance to run.

In general, your code is a nice compendium of almost all the no-dos with QObject, QThread, signals and the like; do read Threads and QObjects before doing anything with threads, QObjects and signals in Qt. This is essential information without which you'll only do a mess like this.* Also this wiki article provides a good rundown of the "right way" to use threads with Qt.

---

Detailed explanation

Let's call the main thread thread A and the thread started by QtConcurrent::run thread B.

Case 1

When run is run from the second thread, p is created, so it has thread affinity with thread B. For this reason, all the connect you perform on it are queued connections (the default for connect is AutoConnection, which uses a QueuedConnection if the connected objects have different thread affinity - and qApp is created in thread A).

The problem is, queued connections work only if the receiving thread has an event loop running (they are implemented as a sendEvent, so if there's no event loop processing events in the target thread they only pile up in the event queue), while here run returns right after starting the process.

So, kill, terminate, close and deleteLater are never called. Notice that:

• calling deleteLater in this case would have been an error anyway, since it would be trying to do a delete on a static object;
• neither kill nor terminate are synchronous, so to make sure that the process is dead before going on you would have needed also a waitForFinished;
• also, potentially the thread that has been spun by QtConcurrent::run is going to be dead after run terminates¹; this is definitely a bad thing, because you are going to have QObjects laying around with thread affinity to a thread that is dead. I don't know how gracefully sendEvent handles this situation.

Anyhow, when the program ends, the p's destructor is invoked automatically as a normal part of the shutdown of a C++ application²; as documented, the destructor of QProcess terminates the process it is linked to if it's still running (but also writes out the "scary message" you saw).

Case 2

As in case 1, you are creating the QProcess with thread B affinity; so all that we said above about events not being delivered & co. still applies.

There are three main differences here:

• you are setting the parent of p to qApp, which lives in the main thread; this is explicitly disallowed, all parent-child relationships between QObjects must exist between objects with the same thread affinity; probably you are getting some warning message in console about this fact (setParent explicitly checks if the objects live in the same thread, I expect QObject's constructor to do the same);
• in this case, the deleteLater could have been appropriate (if you had an event loop spinning), as you allocated with new;
• but most importantly, here p's destructor is never invoked, as it has been allocated with new and no one is calling delete on it; for this reason, the started process keeps running (also, you have a small memory leak).

---

So, what would have been the been the correct way to deal with this? Personally, I would have avoided threads and signals altogether. The starting a process is already asynchronous so you could have simply done:

int main(int argc, char *argv[]) {
    QApplication a(argc, argv);
    QProcess p;
    p.start("caffeinate -d");
    QPushButton w; w.show();
    int ret = a.exec();
    p.close();
    return ret;
}

as always with threads, event queues, signals and the like: don't make it more complicated than it needs to be.

---

Footnotes

1. In practice in this case you probably won't notice because QtConcurrent uses the global thread pool, which kills spun threads only after 30 seconds of idling.

2. General tip: you typically don't want "complicated" objects to be destroyed in this way, since the main has already terminated, so (1) this makes debugging more complicated and (2) if you have Qt objects that depend on the QApplication still being alive (typically, everything in QtGui and QtWidgets) you'll start to get weird crashes at program termination.

QProcess:execute blocks entire application

As far as I understand your question, your problem is here:

QThread* workerThread = new QThread();
Task *task = Task::createTask(Id);
task->moveToThread(workerThread);
...
workerThread->start();
task->execute();

Here, you're executing the task in the main thread not in that worker thread. Note that by moveToThread all slots are executed in the new thread, not explicit method calls.

In your code, task->execute(); is executed in the main thread.

QProcess: Destroyed while process (Web Browser) is still running

Well this is not the best solution but I here it is:

QProcess::execute("taskkill", QStringList() << "/IM" << "chrome.exe" << "/F");

It only works on windows system and closes all browser windows but that is the only solution I have reached and will use for now.

What is the reason for QProcess error status 5?

Tangential to your problem is the fact that you should not be starting a thread per each process. A QProcess emits a finished(int code, QProcess::ExitStatus status) signal when it's done. It will also emit started() and error() upon successful and unsuccessful startup, respectively. Connect all those three signals to a slot in a QObject, then start the process, and deal with the results in the slots. You won't need any extra threads.

If you get a started() signal, then you can be sure that the process's file name was correct, and the process was started. Whatever exit code you get from finished(int) is then indicative of what the process did, perhaps in response to potentially invalid arguments you might have passed to it. If you get a error() signal, the process has failed to start because you gave a wrong filename to QProcess::start(), or you don't have correct permissions.

You should not be writing synchronous code where things happen asynchronously. Synchronous code is code that blocks for a particular thing to happen, like calling waitForCmdFinished. I wish that there was a Qt configuration flag that disables all those leftover synchronous blocking APIs, just like there's a flag to disable/enable Qt 3 support APIs. The mere availability of those blocking APIs promotes horrible hacks like the code above. Those APIs should be disabled by default IMHO. Just as there should be a test for moving QThread and derived classes to another thread. It's also a sign of bad design in every example of publicly available code I could find, and I did a rather thorough search to convince myself I wasn't crazy or something.

The only reasonable use I recall for a waitxxx method in Qt is the wait for a QThread to finish. Even then, this should be only called from within the ~QThread, so as to prevent the QThread from being destroyed with the tread still running.

---

Related Topics