From b45e9bdf3af71b497a2e314488a69f4a1bff381c Mon Sep 17 00:00:00 2001
From: Jarek Kobus <jaroslaw.kobus@qt.io>
Date: Wed, 26 Jun 2024 08:36:01 +0200
Subject: [PATCH] Android: Start avd emulator via detached process

Address the TODO about the process leak.

Before, the process was potentially started in a separate thread.
In this case the done handler of the process could work only
when the thread was still executing, otherwise it couldn't be
invoked because of the missing event loop in a separate thread.
Thus, it was only serving for start process failures which are
raised synchronously.

After the successful start the process thread finished soon,
and then we were losing a handle to the running process.
Later, on shutdown, the process was still running (so a possible
assert from process laucher could have been triggered), and
the emulator process kept running after the Creator shutdown.

This patch executes the emulator process as a detached process.
This makes it possible to continue running the process after
the Creator shutdown without leaking a Process instance.
We also handle the detached process start failure and execute
a message box.

Change-Id: I855d280c257a0cfaca7722a3b1e14d1ead9021f7
Reviewed-by: Alessandro Portale <alessandro.portale@qt.io>
---
 src/plugins/android/androidavdmanager.cpp | 40 ++++++++---------------
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/src/plugins/android/androidavdmanager.cpp b/src/plugins/android/androidavdmanager.cpp
index c9453bb9084..d05c4e0bb51 100644
--- a/src/plugins/android/androidavdmanager.cpp
+++ b/src/plugins/android/androidavdmanager.cpp
@@ -45,46 +45,32 @@ static bool is32BitUserSpace()
 
 bool startAvdAsync(const QString &avdName)
 {
-    const FilePath emulator = AndroidConfig::emulatorToolPath();
-    if (!emulator.exists()) {
-        QMetaObject::invokeMethod(Core::ICore::mainWindow(), [emulator] {
+    const FilePath emulatorPath = AndroidConfig::emulatorToolPath();
+    if (!emulatorPath.exists()) {
+        QMetaObject::invokeMethod(Core::ICore::mainWindow(), [emulatorPath] {
             QMessageBox::critical(Core::ICore::dialogParent(),
                                   Tr::tr("Emulator Tool Is Missing"),
                                   Tr::tr("Install the missing emulator tool (%1) to the"
                                          " installed Android SDK.")
-                                  .arg(emulator.displayName()));
+                                  .arg(emulatorPath.displayName()));
         });
         return false;
     }
 
-    // TODO: Here we are potentially leaking Process instance in case when shutdown happens
-    // after the avdProcess has started and before it has finished. Giving a parent object here
-    // should solve the issue. However, AndroidAvdManager is not a QObject, so no clue what parent
-    // would be the most appropriate. Preferably some object taken form android plugin...
-    Process *avdProcess = new Process;
-    avdProcess->setProcessChannelMode(QProcess::MergedChannels);
-    QObject::connect(avdProcess, &Process::done, avdProcess, [avdProcess] {
-        if (avdProcess->exitCode()) {
-            const QString errorOutput = QString::fromLatin1(avdProcess->rawStdOut());
-            QMetaObject::invokeMethod(Core::ICore::mainWindow(), [errorOutput] {
-                const QString title = Tr::tr("AVD Start Error");
-                QMessageBox::critical(Core::ICore::dialogParent(), title, errorOutput);
-            });
-        }
-        avdProcess->deleteLater();
-    });
-
-    // start the emulator
-    CommandLine cmd(emulator);
+    CommandLine cmd(emulatorPath);
     if (is32BitUserSpace())
         cmd.addArg("-force-32bit");
-
     cmd.addArgs(AndroidConfig::emulatorArgs(), CommandLine::Raw);
     cmd.addArgs({"-avd", avdName});
     qCDebug(avdManagerLog).noquote() << "Running command (startAvdAsync):" << cmd.toUserOutput();
-    avdProcess->setCommand(cmd);
-    avdProcess->start();
-    return avdProcess->waitForStarted(QDeadlineTimer::Forever);
+    if (Process::startDetached(cmd, {}, DetachedChannelMode::Discard))
+        return true;
+
+    QMetaObject::invokeMethod(Core::ICore::mainWindow(), [avdName] {
+        QMessageBox::critical(Core::ICore::dialogParent(), Tr::tr("AVD Start Error"),
+                              Tr::tr("Failed to start AVD emulator for \"%1\" device.").arg(avdName));
+    });
+    return false;
 }
 
 QString findAvd(const QString &avdName)