From 816746a06ddf95cba8575edf9664aee23eba07b1 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Mon, 14 Mar 2022 22:12:21 +0100 Subject: [PATCH] LauncherInterface: Wait for graceful finish of process launcher Give process launcher a chance to finish its processes gracefully before we finish process launcher's process in Creator. Otherwise, when Creator's process reaper kills the process launcher's process while the process launcher is still reaping its processes, the process launcher may leave zombies. Instantiate the ProcessReaper inside ProcessReaper::reap(), otherwise its destructor won't run at all inside process launcher. Reverts e45e16d904f704920bd457fdf3302f3ae7061459 Fixes: QTCREATORBUG-27118 Change-Id: I00290cda05538b5a7ecbeb08240d1e3772d43d62 Reviewed-by: Alessandro Portale Reviewed-by: Eike Ziller --- src/libs/utils/launcherinterface.cpp | 9 ++++---- src/libs/utils/processreaper.cpp | 1 + src/plugins/android/androiddevice.cpp | 31 ++++++--------------------- src/plugins/android/androiddevice.h | 7 +----- src/plugins/android/androidplugin.cpp | 13 +++-------- src/plugins/android/androidplugin.h | 2 -- 6 files changed, 16 insertions(+), 47 deletions(-) diff --git a/src/libs/utils/launcherinterface.cpp b/src/libs/utils/launcherinterface.cpp index a77650745a7..08c93124657 100644 --- a/src/libs/utils/launcherinterface.cpp +++ b/src/libs/utils/launcherinterface.cpp @@ -138,12 +138,11 @@ void LauncherInterfacePrivate::doStart() void LauncherInterfacePrivate::doStop() { m_server->close(); - if (!m_process) - return; - m_process->disconnect(); + QTC_ASSERT(m_process, return); m_socket->shutdown(); - m_process->waitForFinished(3000); - ProcessReaper::reap(m_process); + m_process->waitForFinished(-1); // Let the process interface finish so that it finishes + // reaping any possible processes it has started. + delete m_process; m_process = nullptr; } diff --git a/src/libs/utils/processreaper.cpp b/src/libs/utils/processreaper.cpp index 076981573d3..3ff060be548 100644 --- a/src/libs/utils/processreaper.cpp +++ b/src/libs/utils/processreaper.cpp @@ -195,6 +195,7 @@ void ProcessReaper::reap(QProcess *process, int timeoutMs) return; } + ProcessReaper::instance(); new Internal::Reaper(process, timeoutMs); } diff --git a/src/plugins/android/androiddevice.cpp b/src/plugins/android/androiddevice.cpp index dac30fd1446..1839d16f562 100644 --- a/src/plugins/android/androiddevice.cpp +++ b/src/plugins/android/androiddevice.cpp @@ -35,8 +35,6 @@ #include -#include - #include #include #include @@ -621,28 +619,6 @@ void AndroidDeviceManager::setupDevicesWatcher() updateAvdsList(); } -void AndroidDeviceManager::shutdownDevicesWatcher() -{ - m_avdsFutureWatcher.waitForFinished(); - m_removeAvdFutureWatcher.waitForFinished(); - - if (m_adbDeviceWatcherProcess) { - m_adbDeviceWatcherProcess->terminate(); - m_adbDeviceWatcherProcess->waitForFinished(); - m_adbDeviceWatcherProcess.reset(); - - // Despite terminate/waitForFinished, the process may still - // be around and remain if Qt Creator finishes too early. - QTimer::singleShot(1000, this, [this] { emit devicesWatcherShutdownFinished(); }); - } -} - -ExtensionSystem::IPlugin::ShutdownFlag AndroidDeviceManager::devicesShutdownFlag() const -{ - return m_adbDeviceWatcherProcess ? ExtensionSystem::IPlugin::AsynchronousShutdown - : ExtensionSystem::IPlugin::SynchronousShutdown; -} - void AndroidDeviceManager::HandleAvdsListChange() { DeviceManager *const devMgr = DeviceManager::instance(); @@ -706,6 +682,13 @@ void AndroidDeviceManager::HandleAvdsListChange() } } +void AndroidDeviceManager::shutdownDevicesWatcher() +{ + m_avdsFutureWatcher.waitForFinished(); + m_removeAvdFutureWatcher.waitForFinished(); + m_adbDeviceWatcherProcess.reset(); +} + void AndroidDeviceManager::HandleDevicesListChange(const QString &serialNumber) { DeviceManager *const devMgr = DeviceManager::instance(); diff --git a/src/plugins/android/androiddevice.h b/src/plugins/android/androiddevice.h index 5ecf8101e07..9800098fb16 100644 --- a/src/plugins/android/androiddevice.h +++ b/src/plugins/android/androiddevice.h @@ -30,8 +30,6 @@ #include "androidconfigurations.h" #include "androiddeviceinfo.h" -#include - #include #include @@ -106,7 +104,7 @@ public: static AndroidDeviceManager *instance(); void setupDevicesWatcher(); void shutdownDevicesWatcher(); - ExtensionSystem::IPlugin::ShutdownFlag devicesShutdownFlag() const; + void updateAvdsList(); IDevice::DeviceState getDeviceState(const QString &serial, IDevice::MachineType type) const; void updateDeviceState(const ProjectExplorer::IDevice::ConstPtr &device); @@ -118,9 +116,6 @@ public: QString getRunningAvdsSerialNumber(const QString &name) const; -signals: - void devicesWatcherShutdownFinished(); - private: AndroidDeviceManager(QObject *parent = nullptr); void HandleDevicesListChange(const QString &serialNumber); diff --git a/src/plugins/android/androidplugin.cpp b/src/plugins/android/androidplugin.cpp index bda2f80587e..7eb8e8c0d23 100644 --- a/src/plugins/android/androidplugin.cpp +++ b/src/plugins/android/androidplugin.cpp @@ -160,17 +160,10 @@ bool AndroidPlugin::initialize(const QStringList &arguments, QString *errorMessa return true; } -AndroidPlugin::ShutdownFlag AndroidPlugin::aboutToShutdown() +ExtensionSystem::IPlugin::ShutdownFlag AndroidPlugin::aboutToShutdown() { - AndroidDeviceManager *dm = AndroidDeviceManager::instance(); - const IPlugin::ShutdownFlag sf = dm->devicesShutdownFlag(); - - if (sf == AsynchronousShutdown) - connect(dm, &AndroidDeviceManager::devicesWatcherShutdownFinished, - this, &ExtensionSystem::IPlugin::asynchronousShutdownFinished); - - dm->shutdownDevicesWatcher(); - return sf; + AndroidDeviceManager::instance()->shutdownDevicesWatcher(); + return ExtensionSystem::IPlugin::SynchronousShutdown; } void AndroidPlugin::kitsRestored() diff --git a/src/plugins/android/androidplugin.h b/src/plugins/android/androidplugin.h index 3d46ca08674..e98a5a32cd9 100644 --- a/src/plugins/android/androidplugin.h +++ b/src/plugins/android/androidplugin.h @@ -44,9 +44,7 @@ class AndroidPlugin final : public ExtensionSystem::IPlugin class AndroidPluginPrivate *d = nullptr; -public: ShutdownFlag aboutToShutdown() final; - #ifdef WITH_TESTS private slots: void testAndroidSdkManagerProgressParser_data();