From ace765c199bf126769f564228a9f1509f0e38fc6 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Mon, 6 Sep 2021 14:48:08 +0200 Subject: [PATCH] Move ProcessReaper into lib/utils Reuse ProcessReaper inside process launcher. Automatically reap all internal QProcesses of QtcProcess (either direct child of QtcProcess in QProcessImpl or indirectly inside process launcher). Make ProcessReaper work again on QProcess instead of on QtcProcess, so it may still be reused for non-QtcProcesses. Change-Id: I950cac5cec28f17ae97fe474d6a4e48c01d6aaa2 Reviewed-by: Qt CI Bot Reviewed-by: hjk --- src/app/main.cpp | 2 + src/libs/utils/CMakeLists.txt | 1 + src/libs/utils/launcherinterface.cpp | 3 +- .../utils/processreaper.cpp} | 77 ++++++++++--------- .../reaper_p.h => libs/utils/processreaper.h} | 33 ++++---- src/libs/utils/qtcprocess.cpp | 75 +++++++++--------- src/libs/utils/utils-lib.pri | 2 + src/libs/utils/utils.qbs | 2 + .../cmakeprojectmanager/cmakebuildsystem.cpp | 1 - .../cmakeprojectmanager/cmakeprocess.cpp | 7 +- src/plugins/coreplugin/CMakeLists.txt | 1 - src/plugins/coreplugin/coreplugin.cpp | 3 - src/plugins/coreplugin/coreplugin.h | 2 - src/plugins/coreplugin/coreplugin.pro | 3 - src/plugins/coreplugin/coreplugin.qbs | 3 - .../coreplugin/locator/executefilter.cpp | 8 +- .../locator/spotlightlocatorfilter.cpp | 6 +- src/plugins/coreplugin/reaper.h | 38 --------- .../projectexplorer/abstractprocessstep.cpp | 4 +- src/tools/processlauncher/CMakeLists.txt | 4 + .../processlauncher/launchersockethandler.cpp | 52 +++---------- .../processlauncher/processlauncher-main.cpp | 2 + src/tools/processlauncher/processlauncher.pro | 8 +- src/tools/processlauncher/processlauncher.qbs | 4 + .../auto/utils/qtcprocess/tst_qtcprocess.cpp | 2 + tests/unit/unittest/unittests-main.cpp | 2 + 26 files changed, 142 insertions(+), 203 deletions(-) rename src/{plugins/coreplugin/reaper.cpp => libs/utils/processreaper.cpp} (73%) rename src/{plugins/coreplugin/reaper_p.h => libs/utils/processreaper.h} (73%) delete mode 100644 src/plugins/coreplugin/reaper.h diff --git a/src/app/main.cpp b/src/app/main.cpp index 81bb30a3786..2446f9b7024 100644 --- a/src/app/main.cpp +++ b/src/app/main.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -534,6 +535,7 @@ int main(int argc, char **argv) QCoreApplication::setOrganizationName(QLatin1String(Core::Constants::IDE_SETTINGSVARIANT_STR)); QGuiApplication::setApplicationDisplayName(Core::Constants::IDE_DISPLAY_NAME); + Utils::ProcessReaper processReaper; Utils::LauncherInterface::startLauncher(); auto cleanup = qScopeGuard([] { Utils::LauncherInterface::stopLauncher(); }); diff --git a/src/libs/utils/CMakeLists.txt b/src/libs/utils/CMakeLists.txt index e5292624a07..7cb7560940b 100644 --- a/src/libs/utils/CMakeLists.txt +++ b/src/libs/utils/CMakeLists.txt @@ -121,6 +121,7 @@ add_qtc_library(Utils portlist.cpp portlist.h predicates.h processhandle.cpp processhandle.h + processreaper.cpp processreaper.h processutils.cpp processutils.h progressindicator.cpp progressindicator.h projectintropage.cpp projectintropage.h projectintropage.ui diff --git a/src/libs/utils/launcherinterface.cpp b/src/libs/utils/launcherinterface.cpp index a12ef6e4c5c..269e4393d73 100644 --- a/src/libs/utils/launcherinterface.cpp +++ b/src/libs/utils/launcherinterface.cpp @@ -28,6 +28,7 @@ #include "filepath.h" #include "launcherpackets.h" #include "launchersocket.h" +#include "processreaper.h" #include "qtcassert.h" #include @@ -143,7 +144,7 @@ void LauncherInterfacePrivate::doStop() m_process->disconnect(); m_socket->shutdown(); m_process->waitForFinished(3000); - m_process->deleteLater(); + ProcessReaper::reap(m_process); m_process = nullptr; } diff --git a/src/plugins/coreplugin/reaper.cpp b/src/libs/utils/processreaper.cpp similarity index 73% rename from src/plugins/coreplugin/reaper.cpp rename to src/libs/utils/processreaper.cpp index 1932d60dba8..04a4f710f80 100644 --- a/src/plugins/coreplugin/reaper.cpp +++ b/src/libs/utils/processreaper.cpp @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2016 The Qt Company Ltd. +** Copyright (C) 2021 The Qt Company Ltd. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of Qt Creator. @@ -23,29 +23,26 @@ ** ****************************************************************************/ -#include "reaper.h" -#include "reaper_p.h" - -#include -#include -#include +#include "processreaper.h" +#include "qtcassert.h" +#include #include #include #include using namespace Utils; -namespace Core { +namespace Utils { namespace Internal { -static ProcessReapers *d = nullptr; +static ProcessReaper *d = nullptr; -class ProcessReaper final : public QObject +class Reaper final : public QObject { public: - ProcessReaper(QtcProcess *p, int timeoutMs); - ~ProcessReaper() final; + Reaper(QProcess *p, int timeoutMs); + ~Reaper() final; int timeoutMs() const; bool isFinished() const; @@ -53,28 +50,28 @@ public: private: mutable QTimer m_iterationTimer; - QtcProcess *m_process; + QProcess *m_process = nullptr; int m_emergencyCounter = 0; QProcess::ProcessState m_lastState = QProcess::NotRunning; }; -ProcessReaper::ProcessReaper(QtcProcess *p, int timeoutMs) : m_process(p) +Reaper::Reaper(QProcess *p, int timeoutMs) : m_process(p) { d->m_reapers.append(this); m_iterationTimer.setInterval(timeoutMs); m_iterationTimer.setSingleShot(true); - connect(&m_iterationTimer, &QTimer::timeout, this, &ProcessReaper::nextIteration); + connect(&m_iterationTimer, &QTimer::timeout, this, &Reaper::nextIteration); - QMetaObject::invokeMethod(this, &ProcessReaper::nextIteration, Qt::QueuedConnection); + QMetaObject::invokeMethod(this, &Reaper::nextIteration, Qt::QueuedConnection); } -ProcessReaper::~ProcessReaper() +Reaper::~Reaper() { d->m_reapers.removeOne(this); } -int ProcessReaper::timeoutMs() const +int Reaper::timeoutMs() const { const int remaining = m_iterationTimer.remainingTime(); if (remaining < 0) @@ -83,12 +80,12 @@ int ProcessReaper::timeoutMs() const return remaining; } -bool ProcessReaper::isFinished() const +bool Reaper::isFinished() const { return !m_process; } -void ProcessReaper::nextIteration() +void Reaper::nextIteration() { QProcess::ProcessState state = m_process ? m_process->state() : QProcess::NotRunning; if (state == QProcess::NotRunning || m_emergencyCounter > 5) { @@ -113,19 +110,23 @@ void ProcessReaper::nextIteration() ++m_emergencyCounter; } -ProcessReapers::ProcessReapers() +} // namespace Internal + +ProcessReaper::ProcessReaper() { - d = this; + QTC_ASSERT(Internal::d == nullptr, return); + Internal::d = this; } -ProcessReapers::~ProcessReapers() +ProcessReaper::~ProcessReaper() { + QTC_ASSERT(Internal::d == this, return); while (!m_reapers.isEmpty()) { int alreadyWaited = 0; - QList toDelete; + QList toDelete; // push reapers along: - foreach (ProcessReaper *pr, m_reapers) { + for (Internal::Reaper *pr : qAsConst(m_reapers)) { const int timeoutMs = pr->timeoutMs(); if (alreadyWaited < timeoutMs) { const unsigned long toSleep = static_cast(timeoutMs - alreadyWaited); @@ -145,25 +146,30 @@ ProcessReapers::~ProcessReapers() toDelete.clear(); } - d = nullptr; + Internal::d = nullptr; } -} // namespace Internal - -namespace Reaper { - -void reap(QtcProcess *process, int timeoutMs) +void ProcessReaper::reap(QProcess *process, int timeoutMs) { if (!process) return; QTC_ASSERT(QThread::currentThread() == process->thread(), return); + process->disconnect(); + process->setParent(nullptr); + if (process->state() == QProcess::NotRunning) { + process->deleteLater(); + return; + } else { + process->kill(); + } + // Neither can move object with a parent into a different thread // nor reaping the process with a parent makes any sense. process->setParent(nullptr); - if (process->thread() != qApp->thread()) { - process->moveToThread(qApp->thread()); + if (process->thread() != QCoreApplication::instance()->thread()) { + process->moveToThread(QCoreApplication::instance()->thread()); QMetaObject::invokeMethod(process, [process, timeoutMs] { reap(process, timeoutMs); }); // will be queued @@ -172,9 +178,8 @@ void reap(QtcProcess *process, int timeoutMs) QTC_ASSERT(Internal::d, return); - new Internal::ProcessReaper(process, timeoutMs); + new Internal::Reaper(process, timeoutMs); } -} // namespace Reaper -} // namespace Core +} // namespace Utils diff --git a/src/plugins/coreplugin/reaper_p.h b/src/libs/utils/processreaper.h similarity index 73% rename from src/plugins/coreplugin/reaper_p.h rename to src/libs/utils/processreaper.h index 78140c0eba6..d26325b6f02 100644 --- a/src/plugins/coreplugin/reaper_p.h +++ b/src/libs/utils/processreaper.h @@ -1,6 +1,6 @@ /**************************************************************************** ** -** Copyright (C) 2016 The Qt Company Ltd. +** Copyright (C) 2021 The Qt Company Ltd. ** Contact: https://www.qt.io/licensing/ ** ** This file is part of Qt Creator. @@ -25,25 +25,28 @@ #pragma once +#include "utils_global.h" + #include -namespace Core { -namespace Internal { +QT_BEGIN_NAMESPACE +class QProcess; +QT_END_NAMESPACE -class CorePlugin; -class ProcessReaper; +namespace Utils { -class ProcessReapers final +namespace Internal { class Reaper; } + +class QTCREATOR_UTILS_EXPORT ProcessReaper final { +public: + ProcessReaper(); + ~ProcessReaper(); + + static void reap(QProcess *process, int timeoutMs = 500); private: - ~ProcessReapers(); - ProcessReapers(); - - QList m_reapers; - - friend class CorePlugin; - friend class ProcessReaper; + QList m_reapers; + friend class Internal::Reaper; }; -} // namespace Internal -} // namespace Core +} // namespace Utils diff --git a/src/libs/utils/qtcprocess.cpp b/src/libs/utils/qtcprocess.cpp index 8d9ea7c508f..9d2e4945890 100644 --- a/src/libs/utils/qtcprocess.cpp +++ b/src/libs/utils/qtcprocess.cpp @@ -31,6 +31,7 @@ #include "launcherinterface.h" #include "launcherpackets.h" #include "launchersocket.h" +#include "processreaper.h" #include "qtcassert.h" #include "stringutils.h" @@ -278,86 +279,90 @@ class QProcessImpl : public ProcessInterface public: QProcessImpl(QObject *parent, ProcessMode processMode) : ProcessInterface(parent, processMode) - , m_process(parent) + , m_process(new ProcessHelper(parent)) { - connect(&m_process, &QProcess::started, + connect(m_process, &QProcess::started, this, &QProcessImpl::handleStarted); - connect(&m_process, QOverload::of(&QProcess::finished), + connect(m_process, QOverload::of(&QProcess::finished), this, &ProcessInterface::finished); - connect(&m_process, &QProcess::errorOccurred, + connect(m_process, &QProcess::errorOccurred, this, &ProcessInterface::errorOccurred); - connect(&m_process, &QProcess::readyReadStandardOutput, + connect(m_process, &QProcess::readyReadStandardOutput, this, &ProcessInterface::readyReadStandardOutput); - connect(&m_process, &QProcess::readyReadStandardError, + connect(m_process, &QProcess::readyReadStandardError, this, &ProcessInterface::readyReadStandardError); } + ~QProcessImpl() override + { + ProcessReaper::reap(m_process); + } - QByteArray readAllStandardOutput() override { return m_process.readAllStandardOutput(); } - QByteArray readAllStandardError() override { return m_process.readAllStandardError(); } + QByteArray readAllStandardOutput() override { return m_process->readAllStandardOutput(); } + QByteArray readAllStandardError() override { return m_process->readAllStandardError(); } void setProcessEnvironment(const QProcessEnvironment &environment) override - { m_process.setProcessEnvironment(environment); } + { m_process->setProcessEnvironment(environment); } void setWorkingDirectory(const QString &dir) override - { m_process.setWorkingDirectory(dir); } + { m_process->setWorkingDirectory(dir); } void start(const QString &program, const QStringList &arguments, const QByteArray &writeData) override { m_processStartHandler.setProcessMode(processMode()); m_processStartHandler.setWriteData(writeData); if (isBelowNormalPriority()) - m_processStartHandler.setBelowNormalPriority(&m_process); - m_processStartHandler.setNativeArguments(&m_process, nativeArguments()); + m_processStartHandler.setBelowNormalPriority(m_process); + m_processStartHandler.setNativeArguments(m_process, nativeArguments()); if (isLowPriority()) - m_process.setLowPriority(); + m_process->setLowPriority(); if (isUnixTerminalDisabled()) - m_process.setUnixTerminalDisabled(); - m_process.start(program, arguments, m_processStartHandler.openMode()); - m_processStartHandler.handleProcessStart(&m_process); + m_process->setUnixTerminalDisabled(); + m_process->start(program, arguments, m_processStartHandler.openMode()); + m_processStartHandler.handleProcessStart(m_process); } void terminate() override - { m_process.terminate(); } + { m_process->terminate(); } void kill() override - { m_process.kill(); } + { m_process->kill(); } void close() override - { m_process.close(); } + { m_process->close(); } qint64 write(const QByteArray &data) override - { return m_process.write(data); } + { return m_process->write(data); } void setStandardInputFile(const QString &fileName) override - { m_process.setStandardInputFile(fileName); } + { m_process->setStandardInputFile(fileName); } void setProcessChannelMode(QProcess::ProcessChannelMode mode) override - { m_process.setProcessChannelMode(mode); } + { m_process->setProcessChannelMode(mode); } QString program() const override - { return m_process.program(); } + { return m_process->program(); } QProcess::ProcessError error() const override - { return m_process.error(); } + { return m_process->error(); } QProcess::ProcessState state() const override - { return m_process.state(); } + { return m_process->state(); } qint64 processId() const override - { return m_process.processId(); } + { return m_process->processId(); } int exitCode() const override - { return m_process.exitCode(); } + { return m_process->exitCode(); } QProcess::ExitStatus exitStatus() const override - { return m_process.exitStatus(); } + { return m_process->exitStatus(); } QString errorString() const override - { return m_process.errorString(); } + { return m_process->errorString(); } void setErrorString(const QString &str) override - { m_process.setErrorString(str); } + { m_process->setErrorString(str); } bool waitForStarted(int msecs) override - { return m_process.waitForStarted(msecs); } + { return m_process->waitForStarted(msecs); } bool waitForReadyRead(int msecs) override - { return m_process.waitForReadyRead(msecs); } + { return m_process->waitForReadyRead(msecs); } bool waitForFinished(int msecs) override - { return m_process.waitForFinished(msecs); } + { return m_process->waitForFinished(msecs); } private: void handleStarted() { - m_processStartHandler.handleProcessStarted(&m_process); + m_processStartHandler.handleProcessStarted(m_process); emit started(); } - ProcessHelper m_process; + ProcessHelper *m_process; ProcessStartHandler m_processStartHandler; }; diff --git a/src/libs/utils/utils-lib.pri b/src/libs/utils/utils-lib.pri index 176fda8f03b..63053a82161 100644 --- a/src/libs/utils/utils-lib.pri +++ b/src/libs/utils/utils-lib.pri @@ -84,6 +84,7 @@ SOURCES += \ $$PWD/json.cpp \ $$PWD/portlist.cpp \ $$PWD/processhandle.cpp \ + $$PWD/processreaper.cpp \ $$PWD/processutils.cpp \ $$PWD/appmainwindow.cpp \ $$PWD/basetreeview.cpp \ @@ -222,6 +223,7 @@ HEADERS += \ $$PWD/runextensions.h \ $$PWD/portlist.h \ $$PWD/processhandle.h \ + $$PWD/processreaper.h \ $$PWD/processutils.h \ $$PWD/appmainwindow.h \ $$PWD/basetreeview.h \ diff --git a/src/libs/utils/utils.qbs b/src/libs/utils/utils.qbs index a11d2d75ab9..8f6d3b40860 100644 --- a/src/libs/utils/utils.qbs +++ b/src/libs/utils/utils.qbs @@ -218,6 +218,8 @@ Project { "portlist.h", "processhandle.cpp", "processhandle.h", + "processreaper.cpp", + "processreaper.h", "processutils.cpp", "processutils.h", "progressindicator.cpp", diff --git a/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp b/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp index 82c909619ca..e7397c033ee 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp +++ b/src/plugins/cmakeprojectmanager/cmakebuildsystem.cpp @@ -39,7 +39,6 @@ #include #include #include -#include #include #include #include diff --git a/src/plugins/cmakeprojectmanager/cmakeprocess.cpp b/src/plugins/cmakeprojectmanager/cmakeprocess.cpp index e9baa731f67..64e59109ddf 100644 --- a/src/plugins/cmakeprojectmanager/cmakeprocess.cpp +++ b/src/plugins/cmakeprojectmanager/cmakeprocess.cpp @@ -28,7 +28,6 @@ #include "cmakeparser.h" #include -#include #include #include #include @@ -57,11 +56,7 @@ CMakeProcess::CMakeProcess() CMakeProcess::~CMakeProcess() { - if (m_process) { - m_process->disconnect(); - Core::Reaper::reap(m_process.release()); - } - + m_process.reset(); m_parser.flush(); if (m_future) { diff --git a/src/plugins/coreplugin/CMakeLists.txt b/src/plugins/coreplugin/CMakeLists.txt index 5c04a179e84..eabae35f2a3 100644 --- a/src/plugins/coreplugin/CMakeLists.txt +++ b/src/plugins/coreplugin/CMakeLists.txt @@ -135,7 +135,6 @@ add_qtc_plugin(Core progressmanager/progressbar.cpp progressmanager/progressbar.h progressmanager/progressmanager.cpp progressmanager/progressmanager.h progressmanager/progressmanager_p.h progressmanager/progressview.cpp progressmanager/progressview.h - reaper.cpp reaper.h reaper_p.h rightpane.cpp rightpane.h settingsdatabase.cpp settingsdatabase.h shellcommand.cpp shellcommand.h diff --git a/src/plugins/coreplugin/coreplugin.cpp b/src/plugins/coreplugin/coreplugin.cpp index 916e3803dae..54de03300e8 100644 --- a/src/plugins/coreplugin/coreplugin.cpp +++ b/src/plugins/coreplugin/coreplugin.cpp @@ -32,7 +32,6 @@ #include "iwizardfactory.h" #include "mainwindow.h" #include "modemanager.h" -#include "reaper_p.h" #include "themechooser.h" #include @@ -96,13 +95,11 @@ CorePlugin::CorePlugin() qRegisterMetaType(); qRegisterMetaType(); m_instance = this; - m_reaper = new ProcessReapers; setupSystemEnvironment(); } CorePlugin::~CorePlugin() { - delete m_reaper; IWizardFactory::destroyFeatureProvider(); Find::destroy(); diff --git a/src/plugins/coreplugin/coreplugin.h b/src/plugins/coreplugin/coreplugin.h index 669244e767b..36a368c6c0f 100644 --- a/src/plugins/coreplugin/coreplugin.h +++ b/src/plugins/coreplugin/coreplugin.h @@ -44,7 +44,6 @@ namespace Internal { class EditMode; class MainWindow; class Locator; -class ProcessReapers; class CorePlugin : public ExtensionSystem::IPlugin { @@ -93,7 +92,6 @@ private: MainWindow *m_mainWindow = nullptr; EditMode *m_editMode = nullptr; Locator *m_locator = nullptr; - ProcessReapers *m_reaper = nullptr; Utils::Environment m_startupSystemEnvironment; Utils::EnvironmentItems m_environmentChanges; }; diff --git a/src/plugins/coreplugin/coreplugin.pro b/src/plugins/coreplugin/coreplugin.pro index 5aeec3f716f..a16dbfa0388 100644 --- a/src/plugins/coreplugin/coreplugin.pro +++ b/src/plugins/coreplugin/coreplugin.pro @@ -54,7 +54,6 @@ SOURCES += corejsextensions.cpp \ progressmanager/progressview.cpp \ progressmanager/progressbar.cpp \ progressmanager/futureprogress.cpp \ - reaper.cpp \ coreplugin.cpp \ modemanager.cpp \ basefilewizard.cpp \ @@ -161,8 +160,6 @@ HEADERS += corejsextensions.h \ progressmanager/progressbar.h \ progressmanager/futureprogress.h \ progressmanager/progressmanager.h \ - reaper.h \ - reaper_p.h \ icontext.h \ icore.h \ imode.h \ diff --git a/src/plugins/coreplugin/coreplugin.qbs b/src/plugins/coreplugin/coreplugin.qbs index 3acda9e1a17..c88226fa320 100644 --- a/src/plugins/coreplugin/coreplugin.qbs +++ b/src/plugins/coreplugin/coreplugin.qbs @@ -144,9 +144,6 @@ Project { "plugindialog.h", "plugininstallwizard.cpp", "plugininstallwizard.h", - "reaper.cpp", - "reaper.h", - "reaper_p.h", "rightpane.cpp", "rightpane.h", "settingsdatabase.cpp", diff --git a/src/plugins/coreplugin/locator/executefilter.cpp b/src/plugins/coreplugin/locator/executefilter.cpp index 7d9a78b0637..7924b8a0053 100644 --- a/src/plugins/coreplugin/locator/executefilter.cpp +++ b/src/plugins/coreplugin/locator/executefilter.cpp @@ -27,7 +27,6 @@ #include #include -#include #include #include @@ -199,12 +198,7 @@ void ExecuteFilter::removeProcess() return; m_taskQueue.dequeue(); - m_process->disconnect(); - if (m_process->state() == QProcess::NotRunning) - m_process->deleteLater(); - else - Reaper::reap(m_process); - + delete m_process; m_process = nullptr; } diff --git a/src/plugins/coreplugin/locator/spotlightlocatorfilter.cpp b/src/plugins/coreplugin/locator/spotlightlocatorfilter.cpp index 5c0f1195fe0..f3c25754d68 100644 --- a/src/plugins/coreplugin/locator/spotlightlocatorfilter.cpp +++ b/src/plugins/coreplugin/locator/spotlightlocatorfilter.cpp @@ -28,7 +28,6 @@ #include "../messagemanager.h" #include -#include #include #include #include @@ -156,10 +155,7 @@ void SpotlightIterator::killProcess() QMutexLocker lock(&m_mutex); m_finished = true; m_waitForItems.wakeAll(); - if (m_process->state() == QProcess::NotRunning) - m_process.reset(); - else - Reaper::reap(m_process.release()); + m_process.reset(); } void SpotlightIterator::ensureNext() diff --git a/src/plugins/coreplugin/reaper.h b/src/plugins/coreplugin/reaper.h deleted file mode 100644 index 9ea61b219cf..00000000000 --- a/src/plugins/coreplugin/reaper.h +++ /dev/null @@ -1,38 +0,0 @@ -/**************************************************************************** -** -** Copyright (C) 2016 The Qt Company Ltd. -** Contact: https://www.qt.io/licensing/ -** -** This file is part of Qt Creator. -** -** Commercial License Usage -** Licensees holding valid commercial Qt licenses may use this file in -** accordance with the commercial license agreement provided with the -** Software or, alternatively, in accordance with the terms contained in -** a written agreement between you and The Qt Company. For licensing terms -** and conditions see https://www.qt.io/terms-conditions. For further -** information use the contact form at https://www.qt.io/contact-us. -** -** GNU General Public License Usage -** Alternatively, this file may be used under the terms of the GNU -** General Public License version 3 as published by the Free Software -** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT -** included in the packaging of this file. Please review the following -** information to ensure the GNU General Public License requirements will -** be met: https://www.gnu.org/licenses/gpl-3.0.html. -** -****************************************************************************/ - -#pragma once - -#include "core_global.h" - -namespace Utils { class QtcProcess; } - -namespace Core { -namespace Reaper { - -CORE_EXPORT void reap(Utils::QtcProcess *p, int timeoutMs = 500); - -} // namespace Reaper -} // namespace Core diff --git a/src/plugins/projectexplorer/abstractprocessstep.cpp b/src/plugins/projectexplorer/abstractprocessstep.cpp index a460b118055..d29eecfe11e 100644 --- a/src/plugins/projectexplorer/abstractprocessstep.cpp +++ b/src/plugins/projectexplorer/abstractprocessstep.cpp @@ -34,8 +34,6 @@ #include "target.h" #include "task.h" -#include - #include #include #include @@ -255,7 +253,7 @@ void AbstractProcessStep::setLowPriority() void AbstractProcessStep::doCancel() { - Core::Reaper::reap(d->m_process.release()); + d->m_process.reset(); } ProcessParameters *AbstractProcessStep::processParameters() diff --git a/src/tools/processlauncher/CMakeLists.txt b/src/tools/processlauncher/CMakeLists.txt index 8ad6f8636bf..f65bf11d989 100644 --- a/src/tools/processlauncher/CMakeLists.txt +++ b/src/tools/processlauncher/CMakeLists.txt @@ -11,6 +11,10 @@ add_qtc_executable(qtcreator_processlauncher processlauncher-main.cpp ${UTILSDIR}/launcherpackets.cpp ${UTILSDIR}/launcherpackets.h + ${UTILSDIR}/processreaper.cpp + ${UTILSDIR}/processreaper.h ${UTILSDIR}/processutils.cpp ${UTILSDIR}/processutils.h + ${UTILSDIR}/qtcassert.cpp + ${UTILSDIR}/qtcassert.h ) diff --git a/src/tools/processlauncher/launchersockethandler.cpp b/src/tools/processlauncher/launchersockethandler.cpp index c342eb020bc..b29998856b2 100644 --- a/src/tools/processlauncher/launchersockethandler.cpp +++ b/src/tools/processlauncher/launchersockethandler.cpp @@ -26,12 +26,12 @@ #include "launchersockethandler.h" #include "launcherlogging.h" +#include "processreaper.h" #include "processutils.h" #include #include #include -#include namespace Utils { namespace Internal { @@ -41,43 +41,13 @@ class Process : public ProcessHelper Q_OBJECT public: Process(quintptr token, QObject *parent = nullptr) : - ProcessHelper(parent), m_token(token), m_stopTimer(new QTimer(this)) - { - m_stopTimer->setSingleShot(true); - connect(m_stopTimer, &QTimer::timeout, this, &Process::cancel); - } - - void cancel() - { - if (state() == QProcess::NotRunning) { - deleteLater(); - return; - } - switch (m_stopState) { - case StopState::Inactive: - m_stopState = StopState::Terminating; - m_stopTimer->start(3000); - terminate(); - break; - case StopState::Terminating: - m_stopState = StopState::Killing; - m_stopTimer->start(3000); - kill(); - break; - case StopState::Killing: - m_stopState = StopState::Inactive; - deleteLater(); // TODO: employ something like Core::Reaper here - break; - } - } + ProcessHelper(parent), m_token(token) { } quintptr token() const { return m_token; } ProcessStartHandler *processStartHandler() { return &m_processStartHandler; } private: const quintptr m_token; - QTimer * const m_stopTimer; - enum class StopState { Inactive, Terminating, Killing } m_stopState = StopState::Inactive; ProcessStartHandler m_processStartHandler; }; @@ -97,7 +67,7 @@ LauncherSocketHandler::~LauncherSocketHandler() m_socket->close(); } for (auto it = m_processes.cbegin(); it != m_processes.cend(); ++it) - it.value()->disconnect(); + ProcessReaper::reap(it.value()); } void LauncherSocketHandler::start() @@ -269,13 +239,15 @@ void LauncherSocketHandler::handleStopPacket() { Process * const process = m_processes.value(m_packetParser.token()); if (!process) { - logWarn("got stop request for unknown process"); + // This can happen when the process finishes on its own at about the same time the client + // sends the request. In this case the process was already deleted. + logDebug("got stop request for unknown process"); return; } if (process->state() == QProcess::NotRunning) { - // This can happen if the process finishes on its own at about the same time the client - // sends the request. - logDebug("got stop request when process was not running"); + // This shouldn't happen, since as soon as process finishes or error occurrs + // the process is being removed. + logWarn("got stop request when process was not running"); } else { // We got the client request to stop the starting / running process. // We report process exit to the client. @@ -331,11 +303,7 @@ void LauncherSocketHandler::removeProcess(quintptr token) Process *process = it.value(); m_processes.erase(it); - process->disconnect(); - if (process->state() != QProcess::NotRunning) - process->cancel(); - else - process->deleteLater(); + ProcessReaper::reap(process); } Process *LauncherSocketHandler::senderProcess() const diff --git a/src/tools/processlauncher/processlauncher-main.cpp b/src/tools/processlauncher/processlauncher-main.cpp index 7dc4cefd986..c651a276cc5 100644 --- a/src/tools/processlauncher/processlauncher-main.cpp +++ b/src/tools/processlauncher/processlauncher-main.cpp @@ -25,6 +25,7 @@ #include "launcherlogging.h" #include "launchersockethandler.h" +#include "processreaper.h" #include #include @@ -51,6 +52,7 @@ int main(int argc, char *argv[]) return 1; } + Utils::ProcessReaper processReaper; Utils::Internal::LauncherSocketHandler launcher(app.arguments().constLast()); QTimer::singleShot(0, &launcher, &Utils::Internal::LauncherSocketHandler::start); return app.exec(); diff --git a/src/tools/processlauncher/processlauncher.pro b/src/tools/processlauncher/processlauncher.pro index 69bfc5fc5e9..260e701e3c2 100644 --- a/src/tools/processlauncher/processlauncher.pro +++ b/src/tools/processlauncher/processlauncher.pro @@ -13,11 +13,15 @@ HEADERS += \ launcherlogging.h \ launchersockethandler.h \ $$UTILS_DIR/launcherpackets.h \ - $$UTILS_DIR/processutils.h + $$UTILS_DIR/processreaper.h \ + $$UTILS_DIR/processutils.h \ + $$UTILS_DIR/qtcassert.h SOURCES += \ launcherlogging.cpp \ launchersockethandler.cpp \ processlauncher-main.cpp \ $$UTILS_DIR/launcherpackets.cpp \ - $$UTILS_DIR/processutils.cpp + $$UTILS_DIR/processreaper.cpp \ + $$UTILS_DIR/processutils.cpp \ + $$UTILS_DIR/qtcassert.cpp diff --git a/src/tools/processlauncher/processlauncher.qbs b/src/tools/processlauncher/processlauncher.qbs index 56c91f1520a..83cdbcad8a1 100644 --- a/src/tools/processlauncher/processlauncher.qbs +++ b/src/tools/processlauncher/processlauncher.qbs @@ -23,8 +23,12 @@ QtcTool { files: [ "launcherpackets.cpp", "launcherpackets.h", + "processreaper.cpp", + "processreaper.h", "processutils.cpp", "processutils.h", + "qtcassert.cpp", + "qtcassert.h", ] } } diff --git a/tests/auto/utils/qtcprocess/tst_qtcprocess.cpp b/tests/auto/utils/qtcprocess/tst_qtcprocess.cpp index 3bfd404758a..a525db4154e 100644 --- a/tests/auto/utils/qtcprocess/tst_qtcprocess.cpp +++ b/tests/auto/utils/qtcprocess/tst_qtcprocess.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -203,6 +204,7 @@ private slots: private: void iteratorEditsHelper(OsType osType); + Utils::ProcessReaper processReaper; Environment envWindows; Environment envLinux; diff --git a/tests/unit/unittest/unittests-main.cpp b/tests/unit/unittest/unittests-main.cpp index 532d4961f5b..4d48f2c1dcc 100644 --- a/tests/unit/unittest/unittests-main.cpp +++ b/tests/unit/unittest/unittests-main.cpp @@ -30,6 +30,7 @@ #include #include +#include #include #include @@ -60,6 +61,7 @@ int main(int argc, char *argv[]) Sqlite::Database::activateLogging(); QGuiApplication application(argc, argv); + Utils::ProcessReaper processReaper; Utils::LauncherInterface::startLauncher(qApp->applicationDirPath() + '/' + QLatin1String(TEST_RELATIVE_LIBEXEC_PATH)); auto cleanup = qScopeGuard([] { Utils::LauncherInterface::stopLauncher(); });