From 1b5c4504af5c1f119e68ee10781f0851e9618a44 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Tue, 22 Mar 2022 16:51:38 +0100 Subject: [PATCH] Get rid of no-op calls to QtcProcess::kill() Leave the work for implicit ProcessReaper. Change-Id: Ie01c4e996fda18b7cee77851394174556c6f3857 Reviewed-by: Eike Ziller --- src/libs/clangsupport/processcreator.cpp | 2 +- src/plugins/android/androidavdmanager.cpp | 2 +- .../uncrustify/uncrustifysettings.cpp | 30 ++++++++----------- .../uncrustify/uncrustifysettings.h | 7 +++-- .../coreplugin/locator/executefilter.cpp | 2 +- src/plugins/git/changeselectiondialog.cpp | 16 ++-------- src/plugins/git/changeselectiondialog.h | 3 +- .../perfprofiler/perftracepointdialog.cpp | 10 +------ .../projectexplorer/abstractprocessstep.cpp | 2 +- src/plugins/projectexplorer/extracompiler.cpp | 5 +--- src/plugins/texteditor/formattexteditor.cpp | 1 - 11 files changed, 25 insertions(+), 55 deletions(-) diff --git a/src/libs/clangsupport/processcreator.cpp b/src/libs/clangsupport/processcreator.cpp index 9425336e0fc..c193ab82e6c 100644 --- a/src/libs/clangsupport/processcreator.cpp +++ b/src/libs/clangsupport/processcreator.cpp @@ -70,7 +70,7 @@ std::future ProcessCreator::createProcess() const { return std::async(std::launch::async, [&] { checkIfProcessPathExists(); - auto process = QProcessUniquePointer(new QtcProcess()); + auto process = QProcessUniquePointer(new QtcProcess); process->setProcessChannelMode(QProcess::ForwardedChannels); process->setEnvironment(processEnvironment()); process->setCommand(CommandLine(FilePath::fromString(m_processPath), m_arguments)); diff --git a/src/plugins/android/androidavdmanager.cpp b/src/plugins/android/androidavdmanager.cpp index 0393b1d0a3a..c82779b6dc8 100644 --- a/src/plugins/android/androidavdmanager.cpp +++ b/src/plugins/android/androidavdmanager.cpp @@ -294,7 +294,7 @@ bool AndroidAvdManager::startAvdAsync(const QString &avdName) const .arg(m_config.emulatorToolPath().toString())); return false; } - auto avdProcess = new QtcProcess(); + auto avdProcess = new QtcProcess; avdProcess->setProcessChannelMode(QProcess::MergedChannels); QObject::connect(avdProcess, &QtcProcess::finished, avdProcess, [avdProcess] { avdProcessFinished(avdProcess->exitCode(), avdProcess); }); diff --git a/src/plugins/beautifier/uncrustify/uncrustifysettings.cpp b/src/plugins/beautifier/uncrustify/uncrustifysettings.cpp index 78b0529fa4e..77b775bc9c3 100644 --- a/src/plugins/beautifier/uncrustify/uncrustifysettings.cpp +++ b/src/plugins/beautifier/uncrustify/uncrustifysettings.cpp @@ -56,9 +56,6 @@ const char SETTINGS_NAME[] = "uncrustify"; UncrustifySettings::UncrustifySettings() : AbstractSettings(SETTINGS_NAME, ".cfg") { - connect(&m_versionProcess, &QtcProcess::finished, - this, &UncrustifySettings::parseVersionProcessResult); - setCommand("uncrustify"); m_settings.insert(USE_OTHER_FILES, QVariant(true)); m_settings.insert(USE_HOME_FILE, QVariant(false)); @@ -70,6 +67,8 @@ UncrustifySettings::UncrustifySettings() : read(); } +UncrustifySettings::~UncrustifySettings() = default; + bool UncrustifySettings::useOtherFiles() const { return m_settings.value(USE_OTHER_FILES).toBool(); @@ -227,21 +226,16 @@ static bool parseVersion(const QString &text, int &version) void UncrustifySettings::updateVersion() { - if (m_versionProcess.state() != QProcess::NotRunning) { - m_versionProcess.kill(); - m_versionProcess.waitForFinished(); - } - m_versionProcess.setCommand({ command(), { "--version" } }); - m_versionProcess.start(); -} - -void UncrustifySettings::parseVersionProcessResult() -{ - if (m_versionProcess.exitStatus() != QProcess::NormalExit) - return; - - if (!parseVersion(QString::fromUtf8(m_versionProcess.readAllStandardOutput()), m_version)) - parseVersion(QString::fromUtf8(m_versionProcess.readAllStandardError()), m_version); + m_versionProcess.reset(new QtcProcess); + connect(m_versionProcess.get(), &QtcProcess::finished, this, [this] { + if (m_versionProcess->exitStatus() == QProcess::NormalExit) { + if (!parseVersion(QString::fromUtf8(m_versionProcess->readAllStandardOutput()), m_version)) + parseVersion(QString::fromUtf8(m_versionProcess->readAllStandardError()), m_version); + } + m_versionProcess.release()->deleteLater(); + }); + m_versionProcess->setCommand({ command(), { "--version" } }); + m_versionProcess->start(); } } // namespace Internal diff --git a/src/plugins/beautifier/uncrustify/uncrustifysettings.h b/src/plugins/beautifier/uncrustify/uncrustifysettings.h index 7c6e0a7e327..8c2db0ef2c4 100644 --- a/src/plugins/beautifier/uncrustify/uncrustifysettings.h +++ b/src/plugins/beautifier/uncrustify/uncrustifysettings.h @@ -27,7 +27,8 @@ #include "../abstractsettings.h" #include -#include + +namespace Utils { class QtcProcess; } namespace Beautifier { namespace Internal { @@ -38,6 +39,7 @@ class UncrustifySettings : public AbstractSettings public: UncrustifySettings(); + ~UncrustifySettings() override; bool useOtherFiles() const; void setUseOtherFiles(bool useOtherFiles); @@ -66,8 +68,7 @@ public: void setUseSpecificConfigFile(bool useConfigFile); private: - Utils::QtcProcess m_versionProcess; - void parseVersionProcessResult(); + std::unique_ptr m_versionProcess; }; } // namespace Internal diff --git a/src/plugins/coreplugin/locator/executefilter.cpp b/src/plugins/coreplugin/locator/executefilter.cpp index 61df5e88553..91f0ae9b1f6 100644 --- a/src/plugins/coreplugin/locator/executefilter.cpp +++ b/src/plugins/coreplugin/locator/executefilter.cpp @@ -186,7 +186,7 @@ void ExecuteFilter::createProcess() if (m_process) return; - m_process = new Utils::QtcProcess(); + m_process = new Utils::QtcProcess; m_process->setEnvironment(Utils::Environment::systemEnvironment()); connect(m_process, &QtcProcess::finished, this, &ExecuteFilter::finished); connect(m_process, &QtcProcess::readyReadStandardOutput, this, &ExecuteFilter::readStandardOutput); diff --git a/src/plugins/git/changeselectiondialog.cpp b/src/plugins/git/changeselectiondialog.cpp index 0313304f7d0..a32047d7f87 100644 --- a/src/plugins/git/changeselectiondialog.cpp +++ b/src/plugins/git/changeselectiondialog.cpp @@ -106,7 +106,6 @@ ChangeSelectionDialog::ChangeSelectionDialog(const FilePath &workingDirectory, I ChangeSelectionDialog::~ChangeSelectionDialog() { - terminateProcess(); delete m_ui; } @@ -182,16 +181,6 @@ void ChangeSelectionDialog::enableButtons(bool b) m_ui->checkoutButton->setEnabled(b); } -void ChangeSelectionDialog::terminateProcess() -{ - if (!m_process) - return; - m_process->kill(); - m_process->waitForFinished(); - delete m_process; - m_process = nullptr; -} - void ChangeSelectionDialog::recalculateCompletion() { const FilePath workingDir = workingDirectory(); @@ -214,7 +203,6 @@ void ChangeSelectionDialog::recalculateCompletion() void ChangeSelectionDialog::recalculateDetails() { - terminateProcess(); enableButtons(true); const FilePath workingDir = workingDirectory(); @@ -229,12 +217,12 @@ void ChangeSelectionDialog::recalculateDetails() return; } - m_process = new QtcProcess(this); + m_process.reset(new QtcProcess); m_process->setWorkingDirectory(workingDir); m_process->setEnvironment(m_gitEnvironment); m_process->setCommand({m_gitExecutable, {"show", "--decorate", "--stat=80", ref}}); - connect(m_process, &QtcProcess::finished, this, &ChangeSelectionDialog::setDetails); + connect(m_process.get(), &QtcProcess::finished, this, &ChangeSelectionDialog::setDetails); m_process->start(); if (!m_process->waitForStarted()) diff --git a/src/plugins/git/changeselectiondialog.h b/src/plugins/git/changeselectiondialog.h index 45e9cb43fa0..d4bb7dc607f 100644 --- a/src/plugins/git/changeselectiondialog.h +++ b/src/plugins/git/changeselectiondialog.h @@ -72,11 +72,10 @@ private: void acceptCommand(ChangeCommand command); void enableButtons(bool b); - void terminateProcess(); Ui::ChangeSelectionDialog *m_ui; - Utils::QtcProcess *m_process = nullptr; + std::unique_ptr m_process; Utils::FilePath m_gitExecutable; Utils::Environment m_gitEnvironment; ChangeCommand m_command = NoCommand; diff --git a/src/plugins/perfprofiler/perftracepointdialog.cpp b/src/plugins/perfprofiler/perftracepointdialog.cpp index 3a360c5c974..22d69e0aaa4 100644 --- a/src/plugins/perfprofiler/perftracepointdialog.cpp +++ b/src/plugins/perfprofiler/perftracepointdialog.cpp @@ -80,15 +80,7 @@ PerfTracePointDialog::PerfTracePointDialog() : ? QLatin1String("pkexec") : QLatin1String("n.a.")); } -PerfTracePointDialog::~PerfTracePointDialog() -{ - if (m_process && m_process->state() != QProcess::NotRunning) { - QtcProcess *process = m_process.release(); - connect(process, &QtcProcess::finished, process, &QObject::deleteLater); - process->kill(); - QTimer::singleShot(10000, process, &QObject::deleteLater); - } -} +PerfTracePointDialog::~PerfTracePointDialog() = default; void PerfTracePointDialog::runScript() { diff --git a/src/plugins/projectexplorer/abstractprocessstep.cpp b/src/plugins/projectexplorer/abstractprocessstep.cpp index 7c20ca27a7d..0088fcd9fbe 100644 --- a/src/plugins/projectexplorer/abstractprocessstep.cpp +++ b/src/plugins/projectexplorer/abstractprocessstep.cpp @@ -216,7 +216,7 @@ void AbstractProcessStep::doRun() ? QTextCodec::codecForName("UTF-8") : QTextCodec::codecForLocale()); d->stderrStream = std::make_unique(QTextCodec::codecForLocale()); - d->m_process.reset(new QtcProcess()); + d->m_process.reset(new QtcProcess); d->m_process->setUseCtrlCStub(HostOsInfo::isWindowsHost()); d->m_process->setWorkingDirectory(wd); // Enforce PWD in the environment because some build tools use that. diff --git a/src/plugins/projectexplorer/extracompiler.cpp b/src/plugins/projectexplorer/extracompiler.cpp index 36a8d68b889..ded77e903d7 100644 --- a/src/plugins/projectexplorer/extracompiler.cpp +++ b/src/plugins/projectexplorer/extracompiler.cpp @@ -426,11 +426,8 @@ void ProcessExtraCompiler::runInThread( if (process.waitForFinished(200)) break; - if (futureInterface.isCanceled()) { - process.kill(); - process.waitForFinished(); + if (futureInterface.isCanceled()) return; - } futureInterface.reportResult(handleProcessFinished(&process)); } diff --git a/src/plugins/texteditor/formattexteditor.cpp b/src/plugins/texteditor/formattexteditor.cpp index e5109bbf682..58f48a0bf86 100644 --- a/src/plugins/texteditor/formattexteditor.cpp +++ b/src/plugins/texteditor/formattexteditor.cpp @@ -127,7 +127,6 @@ static FormatTask format(FormatTask task) return task; } if (!process.waitForFinished(5000)) { - process.kill(); task.error = QString(QT_TRANSLATE_NOOP("TextEditor", "Cannot call %1 or some other error occurred. Timeout " "reached while formatting file %2."))