From a30aa4421a0257b048197b51330e6bf5c2732af5 Mon Sep 17 00:00:00 2001
From: Christian Kandeler <christian.kandeler@qt.io>
Date: Tue, 5 Jan 2021 12:33:09 +0100
Subject: [PATCH] Qmake: Prevent unresponsive processes from hanging Qt Creator

That is, processes run via qmake's system() function.
Fun fact disovered while implementing this: "Canceling" the loading of a
qmake project did nothing at all except making the progress bar red at
the end of the parsing procedure. Now at least we terminate the
currently running processes invoked by system(), so the parsing threads
can continue and eventually finish.

Task-number: QTCREATORBUG-24825
Task-number: QTCREATORBUG-25000
Task-number: QTCREATORBUG-25194
Change-Id: Ic92ef200d3a49bbdeff0429ae7038fe3f1935b38
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>
---
 .../qmakeprojectmanager/qmakeproject.cpp      | 12 ++++++++++++
 src/shared/proparser/qmakebuiltins.cpp        | 19 +++++++++++++++++--
 src/shared/proparser/qmakeglobals.cpp         | 11 +++++++++++
 src/shared/proparser/qmakeglobals.h           |  4 ++++
 4 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/src/plugins/qmakeprojectmanager/qmakeproject.cpp b/src/plugins/qmakeprojectmanager/qmakeproject.cpp
index 5d1d848e4d2..cbaa22c7881 100644
--- a/src/plugins/qmakeprojectmanager/qmakeproject.cpp
+++ b/src/plugins/qmakeprojectmanager/qmakeproject.cpp
@@ -666,6 +666,18 @@ void QmakeBuildSystem::asyncUpdate()
                                    Constants::PROFILE_EVALUATE);
 
     m_asyncUpdateFutureInterface.reportStarted();
+    const auto watcher = new QFutureWatcher<void>(this);
+    connect(watcher, &QFutureWatcher<void>::canceled, this, [this, watcher] {
+        if (!m_qmakeGlobals)
+            return;
+        watcher->disconnect();
+        m_qmakeGlobals->killProcesses();
+    });
+    connect(watcher, &QFutureWatcher<void>::finished, this, [watcher] {
+        watcher->disconnect();
+        watcher->deleteLater();
+    });
+    watcher->setFuture(m_asyncUpdateFutureInterface.future());
 
     const Kit *const k = kit();
     QtSupport::BaseQtVersion *const qtVersion = QtSupport::QtKitAspect::qtVersion(k);
diff --git a/src/shared/proparser/qmakebuiltins.cpp b/src/shared/proparser/qmakebuiltins.cpp
index fe12f2d32e5..a697133896f 100644
--- a/src/shared/proparser/qmakebuiltins.cpp
+++ b/src/shared/proparser/qmakebuiltins.cpp
@@ -473,13 +473,28 @@ void QMakeEvaluator::runProcess(QProcess *proc, const QString &command) const
         proc->setProcessEnvironment(env);
     }
 # endif
+# ifdef PROEVALUATOR_THREAD_SAFE
+    m_option->mutex.lock();
+    if (m_option->canceled) {
+        m_option->mutex.unlock();
+        return;
+    }
+    m_option->runningProcs << proc;
+# endif
 # ifdef Q_OS_WIN
     proc->setNativeArguments(QLatin1String("/v:off /s /c \"") + command + QLatin1Char('"'));
     proc->start(m_option->getEnv(QLatin1String("COMSPEC")), QStringList());
 # else
     proc->start(QLatin1String("/bin/sh"), QStringList() << QLatin1String("-c") << command);
+# endif
+# ifdef PROEVALUATOR_THREAD_SAFE
+    m_option->mutex.unlock();
 # endif
     proc->waitForFinished(-1);
+# ifdef PROEVALUATOR_THREAD_SAFE
+    QMutexLocker(&m_option->mutex);
+    m_option->runningProcs.removeOne(proc);
+# endif
 }
 #endif
 
@@ -490,7 +505,7 @@ QByteArray QMakeEvaluator::getCommandOutput(const QString &args, int *exitCode)
     QProcess proc;
     runProcess(&proc, args);
     *exitCode = (proc.exitStatus() == QProcess::NormalExit) ? proc.exitCode() : -1;
-    QByteArray errout = proc.readAllStandardError();
+    QByteArray errout = proc.isReadable() ? proc.readAllStandardError() : QByteArray();
 # ifdef PROEVALUATOR_FULL
     // FIXME: Qt really should have the option to set forwarding per channel
     fputs(errout.constData(), stderr);
@@ -503,7 +518,7 @@ QByteArray QMakeEvaluator::getCommandOutput(const QString &args, int *exitCode)
             QString::fromLocal8Bit(errout));
     }
 # endif
-    out = proc.readAllStandardOutput();
+    out = proc.isReadable() ? proc.readAllStandardOutput() : QByteArray();
 # ifdef Q_OS_WIN
     // FIXME: Qt's line end conversion on sequential files should really be fixed
     out.replace("\r\n", "\n");
diff --git a/src/shared/proparser/qmakeglobals.cpp b/src/shared/proparser/qmakeglobals.cpp
index 07cb8be258f..ea429145e3c 100644
--- a/src/shared/proparser/qmakeglobals.cpp
+++ b/src/shared/proparser/qmakeglobals.cpp
@@ -89,6 +89,17 @@ QMakeGlobals::~QMakeGlobals()
     qDeleteAll(baseEnvs);
 }
 
+void QMakeGlobals::killProcesses()
+{
+#ifdef PROEVALUATOR_THREAD_SAFE
+    QMutexLocker lock(&mutex);
+    canceled = true;
+    for (QProcess * const proc : runningProcs)
+        proc->kill();
+    runningProcs.clear();
+#endif
+}
+
 QString QMakeGlobals::cleanSpec(QMakeCmdLineParserState &state, const QString &spec)
 {
     QString ret = QDir::cleanPath(spec);
diff --git a/src/shared/proparser/qmakeglobals.h b/src/shared/proparser/qmakeglobals.h
index c8efbb22529..2fa9a332135 100644
--- a/src/shared/proparser/qmakeglobals.h
+++ b/src/shared/proparser/qmakeglobals.h
@@ -112,6 +112,8 @@ public:
     QString extra_cmds[4];
     bool runSystemFunction = false;
 
+    void killProcesses();
+
 #ifdef PROEVALUATOR_DEBUG
     int debugLevel;
 #endif
@@ -157,6 +159,8 @@ private:
 
 #ifdef PROEVALUATOR_THREAD_SAFE
     QMutex mutex;
+    bool canceled = false;
+    QList<QProcess *> runningProcs;
 #endif
     QHash<QMakeBaseKey, QMakeBaseEnv *> baseEnvs;