Fix crash when queueing multiple (de)installations for the same package5.12

The Package pointer could be long gone, when the Deinstallation task is
finally scheduled.

Change-Id: I00186572f44ff43a8fcb2de60ef2b427c4b41690
Fixes: AUTOSUITE-1643
Reviewed-by: Dominik Holland <dominik.holland@qt.io>
(cherry picked from commit ed9428330b6f43acd2817bcdb44c6f4ca7506d28)

3 files changed, 26 insertions, 19 deletions

diff --git a/src/installer-lib/applicationinstaller.cpp b/src/installer-lib/applicationinstaller.cpp
index eecc5b6e..506373f0 100644
--- a/src/installer-lib/applicationinstaller.cpp
+++ b/src/installer-lib/applicationinstaller.cpp
@@ -798,7 +798,7 @@ QString ApplicationInstaller::removePackage(const QString &id, bool keepDocument
             const InstallationLocation &il = installationLocationFromId(report->installationLocationId());
 
             if (il.isValid() && (il.id() == report->installationLocationId()))
-                return enqueueTask(new DeinstallationTask(a->nonAliasedInfo(), il, force, keepDocuments));
+                return enqueueTask(new DeinstallationTask(id, il, force, keepDocuments));
         }
     }
     return QString();
diff --git a/src/installer-lib/deinstallationtask.cpp b/src/installer-lib/deinstallationtask.cpp
index cb03cb83..05c71ea0 100644
--- a/src/installer-lib/deinstallationtask.cpp
+++ b/src/installer-lib/deinstallationtask.cpp
@@ -52,15 +52,14 @@
 
 QT_BEGIN_NAMESPACE_AM
 
-DeinstallationTask::DeinstallationTask(ApplicationInfo *app, const InstallationLocation &installationLocation,
+DeinstallationTask::DeinstallationTask(const QString &appId, const InstallationLocation &installationLocation,
                                        bool forceDeinstallation, bool keepDocuments, QObject *parent)
     : AsynchronousTask(parent)
-    , m_app(app)
     , m_installationLocation(installationLocation)
     , m_forceDeinstallation(forceDeinstallation)
     , m_keepDocuments(keepDocuments)
 {
-    m_applicationId = m_app->id(); // in base class
+    m_applicationId = appId; // in base class
 }
 
 bool DeinstallationTask::cancel()
@@ -72,28 +71,37 @@ bool DeinstallationTask::cancel()
 
 void DeinstallationTask::execute()
 {
-    // these have been checked in ApplicationInstaller::removePackage() already
-    Q_ASSERT(m_app);
-    Q_ASSERT(m_app->installationReport());
-    Q_ASSERT(m_app->installationReport()->installationLocationId() == m_installationLocation.id());
-    Q_ASSERT(m_installationLocation.isValid());
-
     bool managerApproval = false;
 
     try {
+        // we cannot rely on a check in ApplicationInstaller::removePackage()
+        // things might have changed in the meantime (e.g. multiple deinstallation request)
+        AbstractApplication *a = ApplicationManager::instance()->fromId(m_applicationId);
+        if (!a) {
+            // the package has already been deinstalled - nothing more to do here
+            throw Exception(Error::NotInstalled, "Cannot remove package %1 because it is not installed")
+                    .arg(m_applicationId);
+        }
+        ApplicationInfo *app = a->nonAliasedInfo();
+
+        Q_ASSERT(app);
+        Q_ASSERT(app->installationReport());
+        Q_ASSERT(app->installationReport()->installationLocationId() == m_installationLocation.id());
+        Q_ASSERT(m_installationLocation.isValid());
+
         // we need to call those ApplicationManager methods in the correct thread
         // this will also exclusively lock the application for us
         QMetaObject::invokeMethod(ApplicationManager::instance(),
                                   "startingApplicationRemoval",
                                   Qt::BlockingQueuedConnection,
                                   Q_RETURN_ARG(bool, managerApproval),
-                                  Q_ARG(QString, m_app->id()));
+                                  Q_ARG(QString, m_applicationId));
         if (!managerApproval)
-            throw Exception("ApplicationManager rejected the removal of app %1").arg(m_app->id());
+            throw Exception("ApplicationManager rejected the removal of app %1").arg(m_applicationId);
 
         // if the app was running before, we now need to wait until is has actually stopped
         while (!m_canceled &&
-               (ApplicationManager::instance()->applicationRunState(m_app->id()) != Am::NotRunning)) {
+               (ApplicationManager::instance()->applicationRunState(m_applicationId) != Am::NotRunning)) {
             QThread::msleep(30);
         }
         // there's a small race condition here, but not doing a planned cancellation isn't harmful
@@ -107,14 +115,14 @@ void DeinstallationTask::execute()
         ScopedRenamer manifestRename;
 
         if (!m_keepDocuments) {
-            if (!docDirRename.rename(QDir(m_installationLocation.documentPath()).absoluteFilePath(m_app->id()),
+            if (!docDirRename.rename(QDir(m_installationLocation.documentPath()).absoluteFilePath(m_applicationId),
                                      ScopedRenamer::NameToNameMinus)) {
                 throw Exception(Error::IO, "could not rename %1 to %1-").arg(docDirRename.baseName());
             }
         }
 
         if (m_installationLocation.isRemovable()) {
-            QString imageFile = QDir(m_installationLocation.installationPath()).absoluteFilePath(m_app->id() + qSL(".appimg"));
+            QString imageFile = QDir(m_installationLocation.installationPath()).absoluteFilePath(m_applicationId + qSL(".appimg"));
 
             if (m_installationLocation.isMounted() && QFile::exists(imageFile)) {
                 // the correct medium is currently mounted
@@ -128,13 +136,13 @@ void DeinstallationTask::execute()
                     throw Exception(Error::MediumNotAvailable, "cannot delete application %1 without the removable medium it was installed on").arg(m_applicationId);
             }
         } else {
-            if (!appDirRename.rename(QDir(m_installationLocation.installationPath()).absoluteFilePath(m_app->id()),
+            if (!appDirRename.rename(QDir(m_installationLocation.installationPath()).absoluteFilePath(m_applicationId),
                                      ScopedRenamer::NameToNameMinus)) {
                 throw Exception(Error::IO, "could not rename %1 to %1-").arg(appDirRename.baseName());
             }
         }
 
-        if (!manifestRename.rename(ApplicationInstaller::instance()->manifestDirectory()->absoluteFilePath(m_app->id()),
+        if (!manifestRename.rename(ApplicationInstaller::instance()->manifestDirectory()->absoluteFilePath(m_applicationId),
                                    ScopedRenamer::NameToNameMinus)) {
             throw Exception(Error::IO, "could not rename %1 to %1-").arg(manifestRename.baseName());
         }
diff --git a/src/installer-lib/deinstallationtask.h b/src/installer-lib/deinstallationtask.h
index f990cee7..c6c88512 100644
--- a/src/installer-lib/deinstallationtask.h
+++ b/src/installer-lib/deinstallationtask.h
@@ -54,7 +54,7 @@ class DeinstallationTask : public AsynchronousTask
     Q_OBJECT
 
 public:
-    DeinstallationTask(ApplicationInfo *app, const InstallationLocation &installationLocation,
+    DeinstallationTask(const QString &appId, const InstallationLocation &installationLocation,
                        bool forceDeinstallation, bool keepDocuments, QObject *parent = nullptr);
 
     bool cancel() override;
@@ -63,7 +63,6 @@ protected:
     void execute() override;
 
 private:
-    ApplicationInfo *m_app;
     const InstallationLocation &m_installationLocation;
     bool m_forceDeinstallation;
     bool m_keepDocuments;