From 624513810c997afb12b56a674b41e4af0a0a87cf Mon Sep 17 00:00:00 2001 From: Robert Griebl Date: Fri, 23 Apr 2021 23:22:00 +0200 Subject: Fix crash when queueing multiple (de)installations for the same package The Package pointer could be long gone, when the Deinstallation task is finally scheduled. Change-Id: I00186572f44ff43a8fcb2de60ef2b427c4b41690 Fixes: AUTOSUITE-1643 Reviewed-by: Dominik Holland (cherry picked from commit ed9428330b6f43acd2817bcdb44c6f4ca7506d28) --- src/installer-lib/applicationinstaller.cpp | 2 +- src/installer-lib/deinstallationtask.cpp | 40 ++++++++++++++++++------------ src/installer-lib/deinstallationtask.h | 3 +-- 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; -- cgit v1.2.1