From 19692eed8e7411298b2e2ac0afdf94bb9e1d0336 Mon Sep 17 00:00:00 2001 From: Robert Griebl Date: Tue, 11 Oct 2022 18:25:05 +0200 Subject: 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 Reviewed-by: Dominik Holland (cherry picked from commit 5ae261864728678df3dd72f156efc6688dcf695d) Reviewed-by: Qt Cherry-pick Bot --- src/manager-lib/applicationmanager.cpp | 14 +++++++++----- src/manager-lib/nativeruntime.cpp | 4 +--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/manager-lib/applicationmanager.cpp b/src/manager-lib/applicationmanager.cpp index 5305ebef..9b27c8e1 100644 --- a/src/manager-lib/applicationmanager.cpp +++ b/src/manager-lib/applicationmanager.cpp @@ -685,6 +685,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; } } @@ -806,12 +808,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 @@ -823,7 +827,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 31d842a2..e8b9d635 100644 --- a/src/manager-lib/nativeruntime.cpp +++ b/src/manager-lib/nativeruntime.cpp @@ -167,9 +167,7 @@ bool NativeRuntime::attachApplicationToQuickLauncher(Application *app) ret = startApplicationViaLauncher(); } - if (ret) - setState(Am::Running); - + setState(ret ? Am::Running : Am::NotRunning); return ret; } -- cgit v1.2.1