From 6e85ff9f4bcecb330b40afbe7e5d5d5f735864f8 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Tue, 1 Jun 2021 13:45:20 +0200 Subject: [PATCH] Get rid of OutputProxy in ShellCommand Before we have used the OutputProxyFactory, which was called in the non-gui thread. The factory, when run, created the connection between the thread and outside world (e.g. VersionControl output window). Instead of setting the factory we provide a set of virtual functions called directly from non-gui threads. We also provide overrides for them in VcsCommand class. Their implementation safely redirects the calls directly to the VcsOutputWindow through the QMetaObject::invokeMethod() with auto connection as a default. Task-number: QTCREATORBUG-25744 Change-Id: I09f2da278003b71095e953a51499a5513cb8f03f Reviewed-by: Eike Ziller --- src/libs/utils/shellcommand.cpp | 34 +++++++------------ src/libs/utils/shellcommand.h | 31 ++++++------------ src/plugins/vcsbase/vcscommand.cpp | 52 +++++++++++++++++++++--------- src/plugins/vcsbase/vcscommand.h | 10 ++++++ 4 files changed, 68 insertions(+), 59 deletions(-) diff --git a/src/libs/utils/shellcommand.cpp b/src/libs/utils/shellcommand.cpp index 9bdd878765f..a06f4409e94 100644 --- a/src/libs/utils/shellcommand.cpp +++ b/src/libs/utils/shellcommand.cpp @@ -85,7 +85,6 @@ public: ~ShellCommandPrivate() { delete m_progressParser; } - std::function m_proxyFactory = []() { return new OutputProxy; }; QString m_displayName; const QString m_defaultWorkingDirectory; const Environment m_environment; @@ -318,33 +317,30 @@ void ShellCommand::runCommand(SynchronousProcess &proc, return; } - QSharedPointer proxy(d->m_proxyFactory()); - if (!(d->m_flags & SuppressCommandLogging)) - emit proxy->appendCommand(dir, command); + appendCommand(dir, command); proc.setCommand(command); if ((d->m_flags & FullySynchronously) || (!(d->m_flags & NoFullySync) && QThread::currentThread() == QCoreApplication::instance()->thread())) { - runFullySynchronous(proc, proxy, dir); + runFullySynchronous(proc, dir); } else { - runSynchronous(proc, proxy, dir); + runSynchronous(proc, dir); } if (!d->m_aborted) { // Success/Fail message in appropriate window? if (proc.result() == QtcProcess::FinishedWithSuccess) { if (d->m_flags & ShowSuccessMessage) - emit proxy->appendMessage(proc.exitMessage()); + appendMessage(proc.exitMessage()); } else if (!(d->m_flags & SuppressFailMessage)) { - emit proxy->appendError(proc.exitMessage()); + appendError(proc.exitMessage()); } } } void ShellCommand::runFullySynchronous(SynchronousProcess &process, - QSharedPointer proxy, const QString &workingDirectory) { // Set up process @@ -364,20 +360,19 @@ void ShellCommand::runFullySynchronous(SynchronousProcess &process, if (!d->m_aborted) { const QString stdErr = process.stdErr(); if (!stdErr.isEmpty() && !(d->m_flags & SuppressStdErr)) - emit proxy->append(stdErr); + append(stdErr); const QString stdOut = process.stdOut(); if (!stdOut.isEmpty() && d->m_flags & ShowStdOut) { if (d->m_flags & SilentOutput) - emit proxy->appendSilently(stdOut); + appendSilently(stdOut); else - emit proxy->append(stdOut); + append(stdOut); } } } void ShellCommand::runSynchronous(SynchronousProcess &process, - QSharedPointer proxy, const QString &workingDirectory) { connect(this, &ShellCommand::terminate, &process, &SynchronousProcess::stopProcess); @@ -393,11 +388,11 @@ void ShellCommand::runSynchronous(SynchronousProcess &process, if (d->m_flags & MergeOutputChannels) { process.setProcessChannelMode(QProcess::MergedChannels); } else if (d->m_progressiveOutput || !(d->m_flags & SuppressStdErr)) { - process.setStdErrCallback([this, proxy](const QString &text) { + process.setStdErrCallback([this](const QString &text) { if (d->m_progressParser) d->m_progressParser->parseProgress(text); if (!(d->m_flags & SuppressStdErr)) - emit proxy->appendError(text); + appendError(text); if (d->m_progressiveOutput) emit stdErrText(text); }); @@ -405,11 +400,11 @@ void ShellCommand::runSynchronous(SynchronousProcess &process, // connect stdout to the output window if desired if (d->m_progressParser || d->m_progressiveOutput || (d->m_flags & ShowStdOut)) { - process.setStdOutCallback([this, proxy](const QString &text) { + process.setStdOutCallback([this](const QString &text) { if (d->m_progressParser) d->m_progressParser->parseProgress(text); if (d->m_flags & ShowStdOut) - emit proxy->append(text); + append(text); if (d->m_progressiveOutput) { emit stdOutText(text); d->m_hadOutput = true; @@ -463,11 +458,6 @@ void ShellCommand::setProgressiveOutput(bool progressive) d->m_progressiveOutput = progressive; } -void ShellCommand::setOutputProxyFactory(const std::function &factory) -{ - d->m_proxyFactory = factory; -} - void ShellCommand::setDisableUnixTerminal() { d->m_disableUnixTerminal = true; diff --git a/src/libs/utils/shellcommand.h b/src/libs/utils/shellcommand.h index 5c606cb6563..309fb216ad8 100644 --- a/src/libs/utils/shellcommand.h +++ b/src/libs/utils/shellcommand.h @@ -29,8 +29,6 @@ #include "qtcprocess.h" -#include - QT_BEGIN_NAMESPACE class QMutex; class QVariant; @@ -62,22 +60,6 @@ private: friend class ShellCommand; }; -// Users of this class can either be in the GUI thread or in other threads. -// Use Qt::AutoConnection to always append in the GUI thread (directly or queued) -class QTCREATOR_UTILS_EXPORT OutputProxy : public QObject -{ - Q_OBJECT - - friend class ShellCommand; - -signals: - void append(const QString &text); - void appendSilently(const QString &text); - void appendError(const QString &text); - void appendCommand(const QString &workingDirectory, const Utils::CommandLine &command); - void appendMessage(const QString &text); -}; - class QTCREATOR_UTILS_EXPORT ShellCommand : public QObject { Q_OBJECT @@ -136,7 +118,6 @@ public: bool hasProgressParser() const; void setProgressiveOutput(bool progressive); - void setOutputProxyFactory(const std::function &factory); void setDisableUnixTerminal(); // This is called once per job in a thread. @@ -162,16 +143,24 @@ protected: int timeoutS() const; QString workDirectory(const QString &wd) const; + // Below methods are called directly from other threads + virtual void append(const QString &text) { Q_UNUSED(text) } + virtual void appendSilently(const QString &text) { Q_UNUSED(text) } + virtual void appendError(const QString &text) { Q_UNUSED(text) } + virtual void appendCommand(const QString &workingDirectory, const Utils::CommandLine &command) { + Q_UNUSED(workingDirectory) + Q_UNUSED(command) + } + virtual void appendMessage(const QString &text) { Q_UNUSED(text) } + private: void run(QFutureInterface &future); // Run without a event loop in fully blocking mode. No signals will be delivered. void runFullySynchronous(SynchronousProcess &proc, - QSharedPointer proxy, const QString &workingDirectory); // Run with an event loop. Signals will be delivered. void runSynchronous(SynchronousProcess &proc, - QSharedPointer proxy, const QString &workingDirectory); class Internal::ShellCommandPrivate *const d; diff --git a/src/plugins/vcsbase/vcscommand.cpp b/src/plugins/vcsbase/vcscommand.cpp index deb12aecc13..e518ffac73a 100644 --- a/src/plugins/vcsbase/vcscommand.cpp +++ b/src/plugins/vcsbase/vcscommand.cpp @@ -42,23 +42,8 @@ VcsCommand::VcsCommand(const QString &workingDirectory, const Environment &envir { VcsOutputWindow::setRepository(workingDirectory); setDisableUnixTerminal(); - setOutputProxyFactory([this] { - auto proxy = new OutputProxy; - VcsOutputWindow *outputWindow = VcsOutputWindow::instance(); + m_outputWindow = VcsOutputWindow::instance(); - connect(proxy, &OutputProxy::append, - outputWindow, [](const QString &txt) { VcsOutputWindow::append(txt); }); - connect(proxy, &OutputProxy::appendSilently, - outputWindow, &VcsOutputWindow::appendSilently); - connect(proxy, &OutputProxy::appendError, - outputWindow, &VcsOutputWindow::appendError); - connect(proxy, &OutputProxy::appendCommand, - outputWindow, &VcsOutputWindow::appendCommand); - connect(proxy, &OutputProxy::appendMessage, - outputWindow, &VcsOutputWindow::appendMessage); - - return proxy; - }); connect(this, &VcsCommand::started, this, [this] { if (flags() & ExpectRepoChanges) Utils::GlobalFileChangeBlocker::instance()->forceBlocked(true); @@ -84,6 +69,41 @@ void VcsCommand::runCommand(SynchronousProcess &proc, emitRepositoryChanged(workingDirectory); } +void VcsCommand::append(const QString &text) +{ + QMetaObject::invokeMethod(m_outputWindow, [this, text] { + m_outputWindow->append(text); + }); +} + +void VcsCommand::appendSilently(const QString &text) +{ + QMetaObject::invokeMethod(m_outputWindow, [this, text] { + m_outputWindow->appendSilently(text); + }); +} + +void VcsCommand::appendError(const QString &text) +{ + QMetaObject::invokeMethod(m_outputWindow, [this, text] { + m_outputWindow->appendError(text); + }); +} + +void VcsCommand::appendCommand(const QString &workingDirectory, const Utils::CommandLine &command) +{ + QMetaObject::invokeMethod(m_outputWindow, [this, workingDirectory, command] { + m_outputWindow->appendCommand(workingDirectory, command); + }); +} + +void VcsCommand::appendMessage(const QString &text) +{ + QMetaObject::invokeMethod(m_outputWindow, [this, text] { + m_outputWindow->appendMessage(text); + }); +} + void VcsCommand::emitRepositoryChanged(const QString &workingDirectory) { if (m_preventRepositoryChanged || !(flags() & VcsCommand::ExpectRepoChanges)) diff --git a/src/plugins/vcsbase/vcscommand.h b/src/plugins/vcsbase/vcscommand.h index 37fc6f23b3a..2a43af3f993 100644 --- a/src/plugins/vcsbase/vcscommand.h +++ b/src/plugins/vcsbase/vcscommand.h @@ -31,6 +31,8 @@ namespace VcsBase { +class VcsOutputWindow; + class VCSBASE_EXPORT VcsCommand : public Core::ShellCommand { Q_OBJECT @@ -49,12 +51,20 @@ public: const Utils::CommandLine &command, const QString &workDirectory = {}) override; +protected: + void append(const QString &text) override; + void appendSilently(const QString &text) override; + void appendError(const QString &text) override; + void appendCommand(const QString &workingDirectory, const Utils::CommandLine &command) override; + void appendMessage(const QString &text) override; + private: void emitRepositoryChanged(const QString &workingDirectory); void coreAboutToClose() override; bool m_preventRepositoryChanged; + VcsOutputWindow *m_outputWindow = nullptr; }; } // namespace VcsBase