diff options
author | Christian Kandeler <christian.kandeler@digia.com> | 2013-10-25 12:27:40 +0200 |
---|---|---|
committer | Christian Kandeler <christian.kandeler@digia.com> | 2013-10-28 16:21:26 +0100 |
commit | ac02788b396d4bc323729cadec52a29cbbd57eef (patch) | |
tree | 13f121fba076ff7a7e30bc30f39675fb989fdded | |
parent | 68514e8cb29bd805b30d393457ef282611364d37 (diff) |
Fix potential deadlock in Ctrl-C handling.
The current code calls into the Qt event system from the interrupt
handler, which will lead to a deadlock if the interrupt came in during
event handling. Change to an approach that does only minimal work in the
interrupt handler.
Change-Id: Ic4e615939305390efd819b47fa02c7fadb6acf16
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>
-rw-r--r-- | src/app/qbs/commandlinefrontend.cpp | 58 | ||||
-rw-r--r-- | src/app/qbs/commandlinefrontend.h | 14 |
2 files changed, 56 insertions, 16 deletions
diff --git a/src/app/qbs/commandlinefrontend.cpp b/src/app/qbs/commandlinefrontend.cpp index d3cedb93f..bc9b9762e 100644 --- a/src/app/qbs/commandlinefrontend.cpp +++ b/src/app/qbs/commandlinefrontend.cpp @@ -36,10 +36,12 @@ #include <qbs.h> #include <api/runenvironment.h> #include <logging/translator.h> +#include <tools/qbsassert.h> #include <QDir> #include <QMetaObject> #include <QProcessEnvironment> +#include <QTimer> #include <cstdlib> namespace qbs { @@ -47,24 +49,45 @@ using namespace Internal; CommandLineFrontend::CommandLineFrontend(const CommandLineParser &parser, Settings *settings, QObject *parent) - : QObject(parent), m_parser(parser), m_settings(settings), m_observer(0), m_canceled(false) + : QObject(parent) + , m_parser(parser) + , m_settings(settings) + , m_observer(0) + , m_cancelStatus(CancelStatusNone) + , m_cancelTimer(new QTimer(this)) { } +CommandLineFrontend::~CommandLineFrontend() +{ + m_cancelTimer->stop(); +} + +// Called from interrupt handler. Don't do anything non-trivial here. void CommandLineFrontend::cancel() { - m_canceled = true; - QMetaObject::invokeMethod(this, "doCancel", Qt::QueuedConnection); + m_cancelStatus = CancelStatusRequested; } -void CommandLineFrontend::doCancel() +void CommandLineFrontend::checkCancelStatus() { - if (m_resolveJobs.isEmpty() && m_buildJobs.isEmpty()) - std::exit(EXIT_FAILURE); - foreach (AbstractJob * const job, m_resolveJobs) - job->cancel(); - foreach (AbstractJob * const job, m_buildJobs) - job->cancel(); + switch (m_cancelStatus) { + case CancelStatusNone: + break; + case CancelStatusRequested: + m_cancelStatus = CancelStatusCanceling; + m_cancelTimer->stop(); + if (m_resolveJobs.isEmpty() && m_buildJobs.isEmpty()) + std::exit(EXIT_FAILURE); + foreach (AbstractJob * const job, m_resolveJobs) + job->cancel(); + foreach (AbstractJob * const job, m_buildJobs) + job->cancel(); + break; + case CancelStatusCanceling: + QBS_ASSERT(false, return); + break; + } } void CommandLineFrontend::start() @@ -129,12 +152,21 @@ void CommandLineFrontend::start() */ if (m_parser.showProgress() && resolvingMultipleProjects()) m_observer->initialize(tr("Setting up projects"), m_resolveJobs.count()); + + // Check every two seconds whether we received a cancel request. This value has been + // experimentally found to be acceptable. + // Note that this polling approach is not problematic here, since we are doing work anyway, + // so there's no danger of waking up the processor for no reason. + connect(m_cancelTimer, SIGNAL(timeout()), SLOT(checkCancelStatus())); + m_cancelTimer->start(2000); } catch (const ErrorInfo &error) { qbsError() << error.toString(); - if (m_buildJobs.isEmpty() && m_resolveJobs.isEmpty()) + if (m_buildJobs.isEmpty() && m_resolveJobs.isEmpty()) { qApp->exit(EXIT_FAILURE); - else + } else { cancel(); + checkCancelStatus(); + } } } @@ -283,7 +315,7 @@ CommandLineFrontend::ProductMap CommandLineFrontend::productsToUse() const void CommandLineFrontend::handleProjectsResolved() { try { - if (m_canceled) + if (m_cancelStatus != CancelStatusNone) throw ErrorInfo(Tr::tr("Execution canceled.")); switch (m_parser.command()) { case ResolveCommandType: diff --git a/src/app/qbs/commandlinefrontend.h b/src/app/qbs/commandlinefrontend.h index 5c913f957..f988c958b 100644 --- a/src/app/qbs/commandlinefrontend.h +++ b/src/app/qbs/commandlinefrontend.h @@ -37,6 +37,10 @@ #include <QList> #include <QObject> +QT_BEGIN_NAMESPACE +class QTimer; +QT_END_NAMESPACE + namespace qbs { class AbstractJob; class ConsoleProgressObserver; @@ -50,6 +54,7 @@ class CommandLineFrontend : public QObject public: explicit CommandLineFrontend(const CommandLineParser &parser, Settings *settings, QObject *parent = 0); + ~CommandLineFrontend(); void cancel(); @@ -61,6 +66,7 @@ private slots: void handleTotalEffortChanged(int totalEffort); void handleTaskProgress(int value, qbs::AbstractJob *job); void handleProcessResultReport(const qbs::ProcessResult &result); + void checkCancelStatus(); private: typedef QHash<Project, QList<ProductData> > ProductMap; @@ -81,8 +87,6 @@ private: void checkForExactlyOneProduct(); void install(); - Q_INVOKABLE void doCancel(); - const CommandLineParser &m_parser; Settings * const m_settings; QList<AbstractJob *> m_resolveJobs; @@ -90,7 +94,11 @@ private: QList<Project> m_projects; ConsoleProgressObserver *m_observer; - bool m_canceled; + + enum CancelStatus { CancelStatusNone, CancelStatusRequested, CancelStatusCanceling }; + CancelStatus m_cancelStatus; + QTimer * const m_cancelTimer; + int m_buildEffortsNeeded; int m_buildEffortsRetrieved; int m_totalBuildEffort; |