From 1628856d8af39d0eddded2374c7186a65799cadc Mon Sep 17 00:00:00 2001 From: Tobias Hunger Date: Fri, 11 Nov 2016 15:02:43 +0100 Subject: [PATCH] Core: Fix Reaper blocking shutdown The eventloop is blocked while the destructor of ReaperPrivate is running. So drive the ProcessReaper by hand instead. Change-Id: I691a28f27455f58ae5807540746ffa1aa2783fed Reviewed-by: Tim Jenssen --- src/plugins/coreplugin/reaper.cpp | 66 +++++++++++++++++++++++-------- src/plugins/coreplugin/reaper_p.h | 14 +++---- 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/src/plugins/coreplugin/reaper.cpp b/src/plugins/coreplugin/reaper.cpp index 9a03e308a2d..f7045f07677 100644 --- a/src/plugins/coreplugin/reaper.cpp +++ b/src/plugins/coreplugin/reaper.cpp @@ -29,6 +29,8 @@ #include #include +#include + namespace Core { namespace Internal { @@ -36,6 +38,8 @@ static ReaperPrivate *d = nullptr; ProcessReaper::ProcessReaper(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); @@ -44,11 +48,31 @@ ProcessReaper::ProcessReaper(QProcess *p, int timeoutMs) : m_process(p) m_futureInterface.reportStarted(); } +ProcessReaper::~ProcessReaper() +{ + d->m_reapers.removeOne(this); +} + +int ProcessReaper::timeoutMs() const +{ + const int remaining = m_iterationTimer.remainingTime(); + if (remaining < 0) + return m_iterationTimer.interval(); + m_iterationTimer.stop(); + return remaining; +} + +bool ProcessReaper::isFinished() const +{ + return !m_process; +} + void ProcessReaper::nextIteration() { QProcess::ProcessState state = m_process->state(); if (state == QProcess::NotRunning || m_emergencyCounter > 5) { delete m_process; + m_process = nullptr; m_futureInterface.reportFinished(); return; } @@ -76,6 +100,30 @@ ReaperPrivate::ReaperPrivate() ReaperPrivate::~ReaperPrivate() { + while (!m_reapers.isEmpty()) { + int alreadyWaited = 0; + QList toDelete; + + // push reapers along: + foreach (ProcessReaper *pr, m_reapers) { + const int timeoutMs = pr->timeoutMs(); + if (alreadyWaited < timeoutMs) { + const unsigned long toSleep = static_cast(timeoutMs - alreadyWaited); + QThread::msleep(toSleep); + alreadyWaited += toSleep; + } + + pr->nextIteration(); + + if (pr->isFinished()) + toDelete.append(pr); + } + + // Clean out reapers that finished in the meantime + qDeleteAll(toDelete); + toDelete.clear(); + } + d = nullptr; } @@ -90,23 +138,7 @@ void reap(QProcess *process, int timeoutMs) QTC_ASSERT(Internal::d, return); - auto reaper = new Internal::ProcessReaper(process, timeoutMs); - QFuture f = reaper->future(); - - Internal::d->m_synchronizer.addFuture(f); - auto watcher = new QFutureWatcher(); - - QObject::connect(watcher, &QFutureWatcher::finished, [watcher, reaper]() { - watcher->deleteLater(); - - const QList> futures = Utils::filtered(Internal::d->m_synchronizer.futures(), - [reaper](const QFuture &f) { return reaper->future() != f; }); - for (const QFuture &f : futures) - Internal::d->m_synchronizer.addFuture(f); - - delete reaper; - }); - watcher->setFuture(f); + new Internal::ProcessReaper(process, timeoutMs); } } // namespace Reaper diff --git a/src/plugins/coreplugin/reaper_p.h b/src/plugins/coreplugin/reaper_p.h index e0995f027e7..474dbc6e0c3 100644 --- a/src/plugins/coreplugin/reaper_p.h +++ b/src/plugins/coreplugin/reaper_p.h @@ -26,8 +26,7 @@ #pragma once #include -#include -#include +#include #include #include @@ -42,13 +41,14 @@ class ProcessReaper : public QObject public: ProcessReaper(QProcess *p, int timeoutMs); + ~ProcessReaper(); - QFuture future() { return m_futureInterface.future(); } - -private: + int timeoutMs() const; + bool isFinished() const; void nextIteration(); - QTimer m_iterationTimer; +private: + mutable QTimer m_iterationTimer; QFutureInterface m_futureInterface; QProcess *m_process; int m_emergencyCounter = 0; @@ -59,7 +59,7 @@ class ReaperPrivate { public: ~ReaperPrivate(); - QFutureSynchronizer m_synchronizer; + QList m_reapers; private: ReaperPrivate();