Fix a possible crash in process launcher

It may apparently happen that when calling QProcess::start()
we may receive a synchronous signal QProcess::errorOccurred()
from the process we are trying to start. In this case
the handler of the error signal might have removed the
process from m_processes hash, which invalidated the
"Process *& process" reference inside
LauncherSocketHandler::handleStartPacket(). So, using
process reference after calling start() may be dangerous.

Refactor ProcessStartHandler so that it stores the pointer
to the process it handles. The pointer to the handler
should still be valid after calling start(), since
the process itself is being deleted with a delay.

Make ProcessStartHandler a member of ProcessHelper. In this
way it's being reused in QProcessImpl and ProcessLauncher.

Fixes: QTCREATORBUG-26726
Change-Id: I8e3f39953035d76c83bbbb13bd78e3042ba2a14e
Reviewed-by: Orgad Shaneh <orgads@gmail.com>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>

src/libs/utils/processutils.cpp

@@ -44,44 +44,40 @@ QIODevice::OpenMode ProcessStartHandler::openMode() const
     return QIODevice::ReadWrite; // initial write and then reading (close the write channel)
 }
 
-void ProcessStartHandler::handleProcessStart(QProcess *process)
+void ProcessStartHandler::handleProcessStart()
 {
     if (m_processMode == ProcessMode::Writer)
         return;
     if (m_writeData.isEmpty())
-        process->closeWriteChannel();
+        m_process->closeWriteChannel();
 }
 
-void ProcessStartHandler::handleProcessStarted(QProcess *process)
+void ProcessStartHandler::handleProcessStarted()
 {
     if (!m_writeData.isEmpty()) {
-        process->write(m_writeData);
+        m_process->write(m_writeData);
         m_writeData = {};
         if (m_processMode == ProcessMode::Reader)
-            process->closeWriteChannel();
+            m_process->closeWriteChannel();
     }
 }
 
-
+void ProcessStartHandler::setBelowNormalPriority()
-void ProcessStartHandler::setBelowNormalPriority(QProcess *process)
 {
 #ifdef Q_OS_WIN
-    process->setCreateProcessArgumentsModifier(
+    m_process->setCreateProcessArgumentsModifier(
         [](QProcess::CreateProcessArguments *args) {
             args->flags |= BELOW_NORMAL_PRIORITY_CLASS;
     });
-#else
-    Q_UNUSED(process)
 #endif // Q_OS_WIN
 }
 
-void ProcessStartHandler::setNativeArguments(QProcess *process, const QString &arguments)
+void ProcessStartHandler::setNativeArguments(const QString &arguments)
 {
 #ifdef Q_OS_WIN
     if (!arguments.isEmpty())
-        process->setNativeArguments(arguments);
+        m_process->setNativeArguments(arguments);
 #else
-    Q_UNUSED(process)
     Q_UNUSED(arguments)
 #endif // Q_OS_WIN
 }

src/libs/utils/processutils.h

@@ -38,23 +38,26 @@ enum class ProcessMode {
 
 class ProcessStartHandler {
 public:
+    ProcessStartHandler(QProcess *process) : m_process(process) {}
+
     void setProcessMode(ProcessMode mode) { m_processMode = mode; }
     void setWriteData(const QByteArray &writeData) { m_writeData = writeData; }
     QIODevice::OpenMode openMode() const;
-    void handleProcessStart(QProcess *process);
+    void handleProcessStart();
-    void handleProcessStarted(QProcess *process);
+    void handleProcessStarted();
-    void setBelowNormalPriority(QProcess *process);
+    void setBelowNormalPriority();
-    void setNativeArguments(QProcess *process, const QString &arguments);
+    void setNativeArguments(const QString &arguments);
 
 private:
     ProcessMode m_processMode = ProcessMode::Reader;
     QByteArray m_writeData;
+    QProcess *m_process;
 };
 
 class ProcessHelper : public QProcess
 {
 public:
-    ProcessHelper(QObject *parent) : QProcess(parent)
+    ProcessHelper(QObject *parent) : QProcess(parent), m_processStartHandler(this)
     {
 #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) && defined(Q_OS_UNIX)
         setChildProcessModifier([this] { setupChildProcess_impl(); });
@@ -65,6 +68,8 @@ public:
     void setupChildProcess() override { setupChildProcess_impl(); }
 #endif
 
+    ProcessStartHandler *processStartHandler() { return &m_processStartHandler; }
+
     using QProcess::setErrorString;
 
     void setLowPriority() { m_lowPriority = true; }
@@ -74,6 +79,7 @@ private:
     void setupChildProcess_impl();
     bool m_lowPriority = false;
     bool m_unixTerminalDisabled = false;
+    ProcessStartHandler m_processStartHandler;
 };
 
 } // namespace Utils

src/libs/utils/qtcprocess.cpp

@@ -314,17 +314,18 @@ public:
     { m_process->setWorkingDirectory(dir); }
     void start(const QString &program, const QStringList &arguments, const QByteArray &writeData) override
     {
-        m_processStartHandler.setProcessMode(processMode());
+        ProcessStartHandler *handler = m_process->processStartHandler();
-        m_processStartHandler.setWriteData(writeData);
+        handler->setProcessMode(processMode());
+        handler->setWriteData(writeData);
         if (isBelowNormalPriority())
-            m_processStartHandler.setBelowNormalPriority(m_process);
+            handler->setBelowNormalPriority();
-        m_processStartHandler.setNativeArguments(m_process, nativeArguments());
+        handler->setNativeArguments(nativeArguments());
         if (isLowPriority())
             m_process->setLowPriority();
         if (isUnixTerminalDisabled())
             m_process->setUnixTerminalDisabled();
-        m_process->start(program, arguments, m_processStartHandler.openMode());
+        m_process->start(program, arguments, handler->openMode());
-        m_processStartHandler.handleProcessStart(m_process);
+        handler->handleProcessStart();
     }
     void terminate() override
     { m_process->terminate(); }
@@ -367,11 +368,10 @@ public:
 private:
     void handleStarted()
     {
-        m_processStartHandler.handleProcessStarted(m_process);
+        m_process->processStartHandler()->handleProcessStarted();
         emit started();
     }
     ProcessHelper *m_process;
-    ProcessStartHandler m_processStartHandler;
 };
 
 static uint uniqueToken()

src/tools/processlauncher/launchersockethandler.cpp

@@ -44,11 +44,9 @@ public:
         ProcessHelper(parent), m_token(token) { }
 
     quintptr token() const { return m_token; }
-    ProcessStartHandler *processStartHandler() { return &m_processStartHandler; }
 
 private:
     const quintptr m_token;
-    ProcessStartHandler m_processStartHandler;
 };
 
 LauncherSocketHandler::LauncherSocketHandler(QString serverPath, QObject *parent)
@@ -156,7 +154,7 @@ void LauncherSocketHandler::handleProcessStarted()
     Process *proc = senderProcess();
     ProcessStartedPacket packet(proc->token());
     packet.processId = proc->processId();
-    proc->processStartHandler()->handleProcessStarted(proc);
+    proc->processStartHandler()->handleProcessStarted();
     sendPacket(packet);
 }
 
@@ -212,14 +210,14 @@ void LauncherSocketHandler::handleStartPacket()
     handler->setProcessMode(packet.processMode);
     handler->setWriteData(packet.writeData);
     if (packet.belowNormalPriority)
-        handler->setBelowNormalPriority(process);
+        handler->setBelowNormalPriority();
-    handler->setNativeArguments(process, packet.nativeArguments);
+    handler->setNativeArguments(packet.nativeArguments);
     if (packet.lowPriority)
         process->setLowPriority();
     if (packet.unixTerminalDisabled)
         process->setUnixTerminalDisabled();
     process->start(packet.command, packet.arguments, handler->openMode());
-    handler->handleProcessStart(process);
+    handler->handleProcessStart();
 }
 
 void LauncherSocketHandler::handleWritePacket()