diff --git a/src/libs/utils/synchronousprocess.cpp b/src/libs/utils/synchronousprocess.cpp index 6a2b365202143d4994a966599ca096e5e1efd883..0367eb484255c1a962ed5fedc1113eb40f2ecd3d 100644 --- a/src/libs/utils/synchronousprocess.cpp +++ b/src/libs/utils/synchronousprocess.cpp @@ -288,7 +288,7 @@ void SynchronousProcess::slotTimeout() if (++m_d->m_hangTimerCount > m_d->m_maxHangTimerCount) { if (debug) qDebug() << Q_FUNC_INFO << "HANG detected, killing"; - m_d->m_process.kill(); + SynchronousProcess::stopProcess(m_d->m_process); m_d->m_result.result = SynchronousProcessResponse::Hang; } else { if (debug) @@ -399,6 +399,43 @@ void SynchronousProcess::processStdErr(bool emitSignals) } } +// Static utilities: Keep running as long as it gets data. +bool SynchronousProcess::readDataFromProcess(QProcess &p, int timeOutMS, + QByteArray *stdOut, QByteArray *stdErr) +{ + if (p.state() != QProcess::Running) { + qWarning("readDataFromProcess: Process in non-running state passed in."); + return false; + } + + // Keep the process running until it has no longer has data + bool finished = false; + bool hasData = false; + do { + finished = p.waitForFinished(timeOutMS); + if ( (hasData = p.bytesAvailable()) ) { + const QByteArray newStdOut = p.readAllStandardOutput(); + const QByteArray newStdErr = p.readAllStandardError(); + if (stdOut) + stdOut->append(newStdOut); + if (stdErr) + stdErr->append(newStdErr); + } + } while (hasData && !finished); + return finished; +} + +bool SynchronousProcess::stopProcess(QProcess &p) +{ + if (p.state() != QProcess::Running) + return true; + p.terminate(); + if (p.waitForFinished(300)) + return true; + p.kill(); + return p.waitForFinished(300); +} + // Path utilities enum OS_Type { OS_Mac, OS_Windows, OS_Unix }; diff --git a/src/libs/utils/synchronousprocess.h b/src/libs/utils/synchronousprocess.h index 8e7aa042f4a9d8fa94ab6b26b4b84381f1f01d8b..bf8c033e01cace91c52446a4c705b552af09cb91 100644 --- a/src/libs/utils/synchronousprocess.h +++ b/src/libs/utils/synchronousprocess.h @@ -82,7 +82,12 @@ QTCREATOR_UTILS_EXPORT QDebug operator<<(QDebug str, const SynchronousProcessRes * The stdOutBuffered(), stdErrBuffered() signals are emitted with complete * lines based on the '\n' marker if they are enabled using * stdOutBufferedSignalsEnabled()/setStdErrBufferedSignalsEnabled(). - * They would typically be used for log windows. */ + * They would typically be used for log windows. + * + * There is a timeout handling that takes effect after the last data have been + * read from stdout/stdin (as opposed to waitForFinished(), which measures time + * since it was invoked). It is thus also suitable for slow processes that continously + * output data (like version system operations). */ class QTCREATOR_UTILS_EXPORT SynchronousProcess : public QObject { @@ -91,7 +96,8 @@ public: SynchronousProcess(); virtual ~SynchronousProcess(); - /* Timeout for hanging processes (no reaction on stderr/stdout)*/ + /* Timeout for hanging processes (triggers after no more output + * occurs on stderr/stdout). */ void setTimeout(int timeoutMS); int timeout() const; @@ -118,6 +124,16 @@ public: SynchronousProcessResponse run(const QString &binary, const QStringList &args); + // Static helper for running a process synchronously in the foreground with timeout + // detection similar SynchronousProcess' handling (taking effect after no more output + // occurs on stderr/stdout as opposed to waitForFinished()). Returns false if a timeout + // occurs. Checking of the process' exit state/code still has to be done. + static bool readDataFromProcess(QProcess &p, int timeOutMS, + QByteArray *stdOut = 0, QByteArray *stdErr = 0); + // Stop a process by first calling terminate() (allowing for signal handling) and + // then kill(). + static bool stopProcess(QProcess &p); + // Helpers to find binaries. Do not use it for other path variables // and file types. static QString locateBinary(const QString &binary); diff --git a/src/plugins/git/gitclient.cpp b/src/plugins/git/gitclient.cpp index c7a150a2ef4af7cd9a15075e89de94adcd5bef9c..b9bbbdbc383f94a1e5442be9b55aa9b2798c56a7 100644 --- a/src/plugins/git/gitclient.cpp +++ b/src/plugins/git/gitclient.cpp @@ -49,6 +49,7 @@ #include <texteditor/itexteditor.h> #include <utils/qtcassert.h> +#include <utils/synchronousprocess.h> #include <vcsbase/vcsbaseeditor.h> #include <vcsbase/vcsbaseoutputwindow.h> @@ -1042,22 +1043,16 @@ bool GitClient::synchronousGit(const QString &workingDirectory, return false; } - if (!process.waitForFinished(m_settings.timeoutSeconds * 1000)) { - if (errorText) - *errorText = GitCommand::msgTimeout(m_settings.timeoutSeconds).toLocal8Bit(); - GitCommand::stopProcess(process); + if (!Utils::SynchronousProcess::readDataFromProcess(process, m_settings.timeoutSeconds * 1000, + outputText, errorText)) { + *errorText->append(GitCommand::msgTimeout(m_settings.timeoutSeconds).toLocal8Bit()); + Utils::SynchronousProcess::stopProcess(process); return false; } - if (outputText) - *outputText = process.readAllStandardOutput(); - - if (errorText) - *errorText = process.readAllStandardError(); - if (Git::Constants::debug) - qDebug() << "synchronousGit ex=" << process.exitCode(); - return process.exitCode() == 0; + qDebug() << "synchronousGit ex=" << process.exitStatus() << process.exitCode(); + return process.exitStatus() == QProcess::NormalExit && process.exitCode() == 0; } static inline int @@ -1546,7 +1541,8 @@ QString GitClient::readConfig(const QString &workingDirectory, const QStringList arguments << QLatin1String("config") << configVar; QByteArray outputText; - if (synchronousGit(workingDirectory, arguments, &outputText, 0, false)) + QByteArray errorText; + if (synchronousGit(workingDirectory, arguments, &outputText, &errorText, false)) return commandOutputFromLocal8Bit(outputText); return QString(); } diff --git a/src/plugins/git/gitclient.h b/src/plugins/git/gitclient.h index ea1fbd9f9fb7f4badbef0eb681318e03386fa1c2..acbe0906d3c47ec4dd44b45a86f56ab927521cc8 100644 --- a/src/plugins/git/gitclient.h +++ b/src/plugins/git/gitclient.h @@ -230,8 +230,8 @@ private: bool synchronousGit(const QString &workingDirectory, const QStringList &arguments, - QByteArray* outputText = 0, - QByteArray* errorText = 0, + QByteArray* outputText, + QByteArray* errorText, bool logCommandToWindow = true); // determine version as '(major << 16) + (minor << 8) + patch' or 0. unsigned synchronousGitVersion(bool silent, QString *errorMessage = 0); diff --git a/src/plugins/git/gitcommand.cpp b/src/plugins/git/gitcommand.cpp index e1f4ac3a9483849ea532a3becbe919566d6fd5ca..9f7018a2ae03ca8afb6a2a7aad8d76ce4fa245c3 100644 --- a/src/plugins/git/gitcommand.cpp +++ b/src/plugins/git/gitcommand.cpp @@ -33,6 +33,7 @@ #include <coreplugin/icore.h> #include <coreplugin/progressmanager/progressmanager.h> #include <extensionsystem/pluginmanager.h> +#include <utils/synchronousprocess.h> #include <QtCore/QDebug> #include <QtCore/QProcess> @@ -117,17 +118,6 @@ QString GitCommand::msgTimeout(int seconds) return tr("Error: Git timed out after %1s.").arg(seconds); } -bool GitCommand::stopProcess(QProcess &p) -{ - if (p.state() != QProcess::Running) - return true; - p.terminate(); - if (p.waitForFinished(300)) - return true; - p.kill(); - return p.waitForFinished(300); -} - void GitCommand::run() { if (Git::Constants::debug) @@ -138,7 +128,8 @@ void GitCommand::run() process.setEnvironment(m_environment); - QByteArray output; + QByteArray stdOut; + QByteArray stdErr; QString error; const int count = m_jobs.size(); @@ -156,20 +147,20 @@ void GitCommand::run() process.closeWriteChannel(); const int timeOutSeconds = m_jobs.at(j).timeout; - if (!process.waitForFinished(timeOutSeconds * 1000)) { - stopProcess(process); + if (!Utils::SynchronousProcess::readDataFromProcess(process, timeOutSeconds * 1000, + &stdOut, &stdErr)) { + Utils::SynchronousProcess::stopProcess(process); ok = false; error += msgTimeout(timeOutSeconds); break; } - output += process.readAllStandardOutput(); - error += QString::fromLocal8Bit(process.readAllStandardError()); + error += QString::fromLocal8Bit(stdErr); switch (m_reportTerminationMode) { case NoReport: break; case ReportStdout: - output += msgTermination(process.exitCode(), m_binaryPath, m_jobs.at(j).arguments).toUtf8(); + stdOut += msgTermination(process.exitCode(), m_binaryPath, m_jobs.at(j).arguments).toUtf8(); break; case ReportStderr: error += msgTermination(process.exitCode(), m_binaryPath, m_jobs.at(j).arguments); @@ -178,16 +169,16 @@ void GitCommand::run() } // Special hack: Always produce output for diff - if (ok && output.isEmpty() && m_jobs.front().arguments.at(0) == QLatin1String("diff")) { - output += "The file does not differ from HEAD"; + if (ok && stdOut.isEmpty() && m_jobs.front().arguments.at(0) == QLatin1String("diff")) { + stdOut += "The file does not differ from HEAD"; } else { // @TODO: Remove, see below if (ok && m_jobs.front().arguments.at(0) == QLatin1String("status")) - removeColorCodes(&output); + removeColorCodes(&stdOut); } - if (ok && !output.isEmpty()) - emit outputData(output); + if (ok && !stdOut.isEmpty()) + emit outputData(stdOut); if (!error.isEmpty()) emit errorText(error); diff --git a/src/plugins/git/gitcommand.h b/src/plugins/git/gitcommand.h index 9345ea4d3f14d3afdec902bc06be57b0bc0e3160..79690ba40295359ca2dee3135eabfe74d53d77b5 100644 --- a/src/plugins/git/gitcommand.h +++ b/src/plugins/git/gitcommand.h @@ -71,8 +71,6 @@ public: void setTerminationReportMode(TerminationReportMode m); static QString msgTimeout(int seconds); - // Helper to kill a process by SIGNAL first, allowing for cleanup - static bool stopProcess(QProcess &p); private: void run(); diff --git a/src/plugins/mercurial/mercurialclient.cpp b/src/plugins/mercurial/mercurialclient.cpp index 37aab25cdc1942589f2d3da18b4f8ca7f9dfbb32..eda48fe909e89c578b0fd278ab961df099e4d372 100644 --- a/src/plugins/mercurial/mercurialclient.cpp +++ b/src/plugins/mercurial/mercurialclient.cpp @@ -37,6 +37,7 @@ #include <coreplugin/editormanager/editormanager.h> #include <utils/qtcassert.h> +#include <utils/synchronousprocess.h> #include <vcsbase/vcsbaseeditor.h> #include <vcsbase/vcsbaseoutputwindow.h> @@ -82,14 +83,16 @@ bool MercurialClient::add(const QString &workingDir, const QString &filename) { QStringList args; args << QLatin1String("add") << filename; - return executeHgSynchronously(workingDir, args); + QByteArray stdOut; + return executeHgSynchronously(workingDir, args, &stdOut); } bool MercurialClient::remove(const QString &workingDir, const QString &filename) { QStringList args; args << QLatin1String("remove") << filename; - return executeHgSynchronously(workingDir, args); + QByteArray stdOut; + return executeHgSynchronously(workingDir, args, &stdOut); } bool MercurialClient::manifestSync(const QString &repository, const QString &relativeFilename) @@ -135,19 +138,17 @@ bool MercurialClient::executeHgSynchronously(const QString &workingDir, hgProcess.closeWriteChannel(); - if (!hgProcess.waitForFinished(settings.timeoutMilliSeconds())) { - hgProcess.terminate(); + QByteArray stdErr; + if (!Utils::SynchronousProcess::readDataFromProcess(hgProcess, settings.timeoutMilliSeconds(), + output, &stdErr)) { + Utils::SynchronousProcess::stopProcess(hgProcess); outputWindow->appendError(MercurialJobRunner::msgTimeout(settings.timeoutSeconds())); return false; } + if (!stdErr.isEmpty()) + outputWindow->append(QString::fromLocal8Bit(stdErr)); - if ((hgProcess.exitStatus() == QProcess::NormalExit) && (hgProcess.exitCode() == 0)) { - if (output) - *output = hgProcess.readAllStandardOutput(); - return true; - } - - return false; + return hgProcess.exitStatus() == QProcess::NormalExit && hgProcess.exitCode() == 0; } QString MercurialClient::branchQuerySync(const QString &repositoryRoot) diff --git a/src/plugins/mercurial/mercurialclient.h b/src/plugins/mercurial/mercurialclient.h index cd626adfb59d2a1d46032aaa4d14f3c8da497711..9e0daabc7eb48885cdeb07521bec17a2af8a0114 100644 --- a/src/plugins/mercurial/mercurialclient.h +++ b/src/plugins/mercurial/mercurialclient.h @@ -114,7 +114,7 @@ private slots: private: bool executeHgSynchronously(const QString &workingDir, const QStringList &args, - QByteArray *output=0) const; + QByteArray *output) const; void enqueueJob(const QSharedPointer<HgTask> &); void revert(const QString &workingDir, const QString &argument, const QString &revision, const QVariant &cookie); diff --git a/src/plugins/mercurial/mercurialjobrunner.cpp b/src/plugins/mercurial/mercurialjobrunner.cpp index eb41c54fc0a2c7f14b514b6c3d17b345bb723ab2..17e2ea4f111812c0cd430f67a6572f79cd6cbaf6 100644 --- a/src/plugins/mercurial/mercurialjobrunner.cpp +++ b/src/plugins/mercurial/mercurialjobrunner.cpp @@ -34,6 +34,7 @@ #include <vcsbase/vcsbaseoutputwindow.h> #include <vcsbase/vcsbaseeditor.h> +#include <utils/synchronousprocess.h> #include <QtCore/QProcess> #include <QtCore/QString> @@ -112,7 +113,7 @@ void MercurialJobRunner::getSettings() { const MercurialSettings &settings = MercurialPlugin::instance()->settings(); binary = settings.binary(); - timeout = settings.timeoutMilliSeconds(); + m_timeoutMS = settings.timeoutMilliSeconds(); standardArguments = settings.standardArguments(); } @@ -203,25 +204,27 @@ void MercurialJobRunner::task(const QSharedPointer<HgTask> &job) hgProcess.closeWriteChannel(); - if (!hgProcess.waitForFinished(timeout)) { - hgProcess.terminate(); - emit error(msgTimeout(timeout / 1000)); + QByteArray stdOutput; + QByteArray stdErr; + + if (!Utils::SynchronousProcess::readDataFromProcess(hgProcess, m_timeoutMS, &stdOutput, &stdErr)) { + Utils::SynchronousProcess::stopProcess(hgProcess); + emit error(msgTimeout(m_timeoutMS / 1000)); return; } - if ((hgProcess.exitStatus() == QProcess::NormalExit) && (hgProcess.exitCode() == 0)) { - QByteArray stdOutput= hgProcess.readAllStandardOutput(); + if (hgProcess.exitStatus() == QProcess::NormalExit && hgProcess.exitCode() == 0) { /* * sometimes success means output is actually on error channel (stderr) * e.g. "hg revert" outputs "no changes needed to 'file'" on stderr if file has not changed * from revision specified */ if (stdOutput.isEmpty()) - stdOutput = hgProcess.readAllStandardError(); + stdOutput = stdErr; emit output(stdOutput); taskData->emitSucceeded(); } else { - emit error(QString::fromLocal8Bit(hgProcess.readAllStandardError())); + emit error(QString::fromLocal8Bit(stdErr)); } hgProcess.close(); diff --git a/src/plugins/mercurial/mercurialjobrunner.h b/src/plugins/mercurial/mercurialjobrunner.h index cdc484d4bfeda39273a4ece06339160ef0f749d6..8d124e2457e083b13ba2f548bc1b38dff4d76811 100644 --- a/src/plugins/mercurial/mercurialjobrunner.h +++ b/src/plugins/mercurial/mercurialjobrunner.h @@ -115,7 +115,7 @@ private: bool keepRunning; QString binary; QStringList standardArguments; - int timeout; + int m_timeoutMS; }; } //namespace Internal diff --git a/src/plugins/perforce/perforcechecker.cpp b/src/plugins/perforce/perforcechecker.cpp index 9a80170faf458c1e0a342c4d2aadd86522f65f26..f42407bc739c06cd0add61c5b38a182bbe657d3e 100644 --- a/src/plugins/perforce/perforcechecker.cpp +++ b/src/plugins/perforce/perforcechecker.cpp @@ -30,6 +30,7 @@ #include "perforcechecker.h" #include <utils/qtcassert.h> +#include <utils/synchronousprocess.h> #include <QtCore/QRegExp> #include <QtCore/QTimer> @@ -101,23 +102,12 @@ void PerforceChecker::start(const QString &binary, } } -bool PerforceChecker::ensureProcessStopped(QProcess &p) -{ - if (p.state() != QProcess::Running) - return true; - p.terminate(); - if (p.waitForFinished(300)) - return true; - p.kill(); - return p.waitForFinished(300); -} - void PerforceChecker::slotTimeOut() { if (!isRunning()) return; m_timedOut = true; - ensureProcessStopped(m_process); + Utils::SynchronousProcess::stopProcess(m_process); emitFailed(tr("\"%1\" timed out after %2ms.").arg(m_binary).arg(m_timeOutMS)); } @@ -135,7 +125,7 @@ void PerforceChecker::slotError(QProcess::ProcessError error) case QProcess::ReadError: case QProcess::WriteError: case QProcess::UnknownError: - ensureProcessStopped(m_process); + Utils::SynchronousProcess::stopProcess(m_process); break; } } diff --git a/src/plugins/perforce/perforcechecker.h b/src/plugins/perforce/perforcechecker.h index 906ecd5c29d327b45096d45bb1b5f4963e5cf8ed..679ac2ec47c39b7c748e112e155571f483544b3f 100644 --- a/src/plugins/perforce/perforcechecker.h +++ b/src/plugins/perforce/perforcechecker.h @@ -57,8 +57,6 @@ public slots: bool useOverideCursor() const; void setUseOverideCursor(bool v); - static bool ensureProcessStopped(QProcess &p); - signals: void succeeded(const QString &repositoryRoot); void failed(const QString &errorMessage); diff --git a/src/plugins/perforce/perforceplugin.cpp b/src/plugins/perforce/perforceplugin.cpp index 8648f2c4a5ae4d3692595090d757b66878520c28..40146c6bd02c7386631f784ea4b69429afa4d3e8 100644 --- a/src/plugins/perforce/perforceplugin.cpp +++ b/src/plugins/perforce/perforceplugin.cpp @@ -1075,7 +1075,7 @@ PerforceResponse PerforcePlugin::fullySynchronousProcess(const QString &workingD } if (!stdInput.isEmpty()) { if (process.write(stdInput) == -1) { - PerforceChecker::ensureProcessStopped(process); + Utils::SynchronousProcess::stopProcess(process); response.error = true; response.message = tr("Unable to write input data to process %1: %2").arg(m_settings.p4Command(), process.errorString()); return response; @@ -1083,9 +1083,11 @@ PerforceResponse PerforcePlugin::fullySynchronousProcess(const QString &workingD process.closeWriteChannel(); } + QByteArray stdOut; + QByteArray stdErr; const int timeOut = (flags & LongTimeOut) ? m_settings.longTimeOutMS() : m_settings.timeOutMS(); - if (!process.waitForFinished(timeOut)) { - PerforceChecker::ensureProcessStopped(process); + if (!Utils::SynchronousProcess::readDataFromProcess(process, timeOut, &stdOut, &stdErr)) { + Utils::SynchronousProcess::stopProcess(process); response.error = true; response.message = msgTimeout(timeOut); return response; @@ -1097,8 +1099,7 @@ PerforceResponse PerforcePlugin::fullySynchronousProcess(const QString &workingD } response.exitCode = process.exitCode(); response.error = response.exitCode ? !(flags & IgnoreExitCode) : false; - response.stdErr = QString::fromLocal8Bit(process.readAllStandardError()); - const QByteArray stdOut = process.readAllStandardOutput(); + response.stdErr = QString::fromLocal8Bit(stdErr); response.stdOut = outputCodec ? outputCodec->toUnicode(stdOut.constData(), stdOut.size()) : QString::fromLocal8Bit(stdOut); const QChar cr = QLatin1Char('\r'); diff --git a/src/plugins/vcsbase/checkoutjobs.cpp b/src/plugins/vcsbase/checkoutjobs.cpp index 257068d5b50158d00b310c695ff9b510be019f6f..39b6cd232c891b974834b8cc164336ba58eeeb8e 100644 --- a/src/plugins/vcsbase/checkoutjobs.cpp +++ b/src/plugins/vcsbase/checkoutjobs.cpp @@ -30,6 +30,7 @@ #include "checkoutjobs.h" #include <QtCore/QDebug> +#include <utils/synchronousprocess.h> enum { debug = 0 }; namespace VCSBase { @@ -137,9 +138,7 @@ void ProcessCheckoutJob::cancel() qDebug() << "ProcessCheckoutJob::start"; emit output(tr("Stopping...")); - d->process.terminate(); - if (!d->process.waitForFinished(5000)) - d->process.kill(); + Utils::SynchronousProcess::stopProcess(d->process); } } // namespace VCSBase diff --git a/src/plugins/vcsbase/vcsbasesubmiteditor.cpp b/src/plugins/vcsbase/vcsbasesubmiteditor.cpp index 8632873c8ec2611222e805502cecdbcc47138cac..9c7ea317b15a119448b0a8ce28df500d5ed8b54b 100644 --- a/src/plugins/vcsbase/vcsbasesubmiteditor.cpp +++ b/src/plugins/vcsbase/vcsbasesubmiteditor.cpp @@ -42,6 +42,7 @@ #include <coreplugin/actionmanager/actionmanager.h> #include <utils/submiteditorwidget.h> #include <utils/checkablemessagebox.h> +#include <utils/synchronousprocess.h> #include <utils/submitfieldwidget.h> #include <find/basetextfind.h> #include <texteditor/fontsettings.h> @@ -580,18 +581,20 @@ bool VCSBaseSubmitEditor::runSubmitMessageCheckScript(const QString &checkScript *errorMessage = tr("The check script '%1' could not be started: %2").arg(checkScript, checkProcess.errorString()); return false; } - if (!checkProcess.waitForFinished()) { - *errorMessage = tr("The check script '%1' could not be run: %2").arg(checkScript, checkProcess.errorString()); + QByteArray stdOutData; + QByteArray stdErrData; + if (!Utils::SynchronousProcess::readDataFromProcess(checkProcess, 30000, &stdOutData, &stdErrData)) { + Utils::SynchronousProcess::stopProcess(checkProcess); + *errorMessage = tr("The check script '%1' timed out.").arg(checkScript); return false; } if (checkProcess.exitStatus() != QProcess::NormalExit) { *errorMessage = tr("The check script '%1' crashed").arg(checkScript); return false; } - const QString stdOut = QString::fromLocal8Bit(checkProcess.readAllStandardOutput()); - if (!stdOut.isEmpty()) - outputWindow->appendSilently(stdOut); - const QString stdErr = QString::fromLocal8Bit(checkProcess.readAllStandardError()); + if (!stdOutData.isEmpty()) + outputWindow->appendSilently(QString::fromLocal8Bit(stdOutData)); + const QString stdErr = QString::fromLocal8Bit(stdErrData); if (!stdErr.isEmpty()) outputWindow->appendSilently(stdErr); const int exitCode = checkProcess.exitCode();