Fix a potential crash when trying to start applications

If you were calling startApplication() and the runtime failed to
execute the app's binary in a container, it would call deleteLater() on
the failed runtime object. The problem was that if you immediately
called startApplication() once more in the same event loop iteration,
the Application object still had the to-be-deleted runtime attached,
which in turn prevents the allocation of a new container, so the
AM tried to start the application again, but this time with
"container" set to nullptr.

The actual fix is to NOT call deleteLater, but directly delete the
runtime object.

In addition, when startApplication() is called for an application,
we have to make sure that the application's runtime object can not
be in the state NotRunning: the AM could still end up in this state,
if a Container plugin is either not reporting shutdown correctly, or
if the container's "ready" state is delayed.

Change-Id: Ied1baec8c90d4e0a980c296cbc7cd87b12629524
Pick-to: 6.4 6.3 6.2 5.15
Reviewed-by: Dominik Holland <dominik.holland@qt.io>

2 files changed, 10 insertions, 8 deletions

diff --git a/src/manager-lib/applicationmanager.cpp b/src/manager-lib/applicationmanager.cpp
index 6a991a4f..c64e4761 100644
--- a/src/manager-lib/applicationmanager.cpp
+++ b/src/manager-lib/applicationmanager.cpp
@@ -673,6 +673,8 @@ bool ApplicationManager::startApplicationInternal(const QString &appId, const QS
             return false;
 
         case Am::NotRunning:
+            throw Exception("Application %1 is not running, but still has a Runtime object attached")
+                    .arg(app->id());
             break;
         }
     }
@@ -810,12 +812,14 @@ bool ApplicationManager::startApplicationInternal(const QString &appId, const QS
         qCDebug(LogSystem) << "  documentUrl:" << documentUrl;
 
     if (inProcess) {
-        bool ok = runtime->start();
-        if (ok)
+        bool successfullyStarted = runtime->start();
+
+        if (successfullyStarted)
             emitActivated(app);
         else
-            runtime->deleteLater();
-        return ok;
+            delete runtime;
+
+        return successfullyStarted;
     } else {
         // We can only start the app when both the container and the windowmanager are ready.
         // Using a state-machine would be one option, but then we would need that state-machine
@@ -827,7 +831,7 @@ bool ApplicationManager::startApplicationInternal(const QString &appId, const QS
             if (successfullyStarted)
                 emitActivated(app);
             else
-                runtime->deleteLater(); // ~Runtime() will clean app->nonAliased()->m_runtime
+                delete runtime; // ~Runtime() will clean up app->m_runtime
 
             return successfullyStarted;
         };
diff --git a/src/manager-lib/nativeruntime.cpp b/src/manager-lib/nativeruntime.cpp
index f7d6cf22..d583856a 100644
--- a/src/manager-lib/nativeruntime.cpp
+++ b/src/manager-lib/nativeruntime.cpp
@@ -145,9 +145,7 @@ bool NativeRuntime::attachApplicationToQuickLauncher(Application *app)
         ret = startApplicationViaLauncher();
     }
 
-    if (ret)
-        setState(Am::Running);
-
+    setState(ret ? Am::Running : Am::NotRunning);
     return ret;
 }