Fix occasional crash in install phase

Progress information is printed using same static instance of the
coordinator by different operations. The operation object pointer was saved as key to QHash so that progress could be tracked. This has caused problems in installer as QHash insert occasionally caused segmentation fault.
Fixed so that the progress information is no longer tracked based on
operation object pointer, instead object name is used.

Task-number: QTIFW-3314
Change-Id: Ic4007f226321e109109006c5c89415308920c614
Reviewed-by: Arttu Tarkiainen <arttu.tarkiainen@qt.io>

7 files changed, 32 insertions, 34 deletions

diff --git a/src/libs/installer/downloadarchivesjob.cpp b/src/libs/installer/downloadarchivesjob.cpp
index feb7d66da..65eead1f9 100644
--- a/src/libs/installer/downloadarchivesjob.cpp
+++ b/src/libs/installer/downloadarchivesjob.cpp
@@ -49,7 +49,7 @@ static constexpr uint scMaxRetries = 5;
 /*!
     Creates a new DownloadArchivesJob with parent \a core.
 */
-DownloadArchivesJob::DownloadArchivesJob(PackageManagerCore *core)
+DownloadArchivesJob::DownloadArchivesJob(PackageManagerCore *core, const QString &objectName)
     : Job(core)
     , m_core(core)
     , m_downloader(nullptr)
@@ -63,6 +63,7 @@ DownloadArchivesJob::DownloadArchivesJob(PackageManagerCore *core)
     , m_retryCount(scMaxRetries)
 {
     setCapabilities(Cancelable);
+    setObjectName(objectName);
 }
 
 /*!
diff --git a/src/libs/installer/downloadarchivesjob.h b/src/libs/installer/downloadarchivesjob.h
index 122af2831..5155c881a 100644
--- a/src/libs/installer/downloadarchivesjob.h
+++ b/src/libs/installer/downloadarchivesjob.h
@@ -51,7 +51,7 @@ class DownloadArchivesJob : public Job
     Q_OBJECT
 
 public:
-    explicit DownloadArchivesJob(PackageManagerCore *core);
+    explicit DownloadArchivesJob(PackageManagerCore *core, const QString &objectName);
     ~DownloadArchivesJob();
 
     int numberOfDownloads() const { return m_archivesDownloaded; }
diff --git a/src/libs/installer/packagemanagercore.cpp b/src/libs/installer/packagemanagercore.cpp
index 509b8a757..2fb39a4a8 100644
--- a/src/libs/installer/packagemanagercore.cpp
+++ b/src/libs/installer/packagemanagercore.cpp
@@ -853,7 +853,7 @@ int PackageManagerCore::downloadNeededArchives(double partProgressSize)
     ProgressCoordinator::instance()->emitLabelAndDetailTextChanged(QLatin1Char('\n')
         + tr("Downloading packages..."));
 
-    DownloadArchivesJob archivesJob(this);
+    DownloadArchivesJob archivesJob(this, QLatin1String("downloadArchiveJob"));
     archivesJob.setAutoDelete(false);
     archivesJob.setArchivesToDownload(archivesToDownload);
     archivesJob.setExpectedTotalSize(archivesToDownloadTotalSize);
diff --git a/src/libs/installer/packagemanagercore_p.cpp b/src/libs/installer/packagemanagercore_p.cpp
index 7777fd644..858698a7a 100644
--- a/src/libs/installer/packagemanagercore_p.cpp
+++ b/src/libs/installer/packagemanagercore_p.cpp
@@ -220,6 +220,7 @@ PackageManagerCorePrivate::PackageManagerCorePrivate(PackageManagerCore *core)
 #else
     , m_allowCompressedRepositoryInstall(false)
 #endif
+    , m_connectedOperations(0)
 {
 }
 
@@ -266,6 +267,7 @@ PackageManagerCorePrivate::PackageManagerCorePrivate(PackageManagerCore *core, q
 #else
     , m_allowCompressedRepositoryInstall(false)
 #endif
+    , m_connectedOperations(0)
 {
     foreach (const OperationBlob &operation, performedOperations) {
         std::unique_ptr<QInstaller::Operation> op(KDUpdater::UpdateOperationFactory::instance()
@@ -1208,8 +1210,11 @@ void PackageManagerCorePrivate::connectOperationToInstaller(Operation *const ope
             connect(m_core, SIGNAL(installationInterrupted()), operationObject, SLOT(cancelOperation()));
 
         if (mo->indexOfSignal(QMetaObject::normalizedSignature("progressChanged(double)")) > -1) {
+            // create unique object names for progress information track
+            operationObject->setObjectName(QLatin1String("operation_%1").arg(QString::number(m_connectedOperations)));
             ProgressCoordinator::instance()->registerPartProgress(operationObject,
                 SIGNAL(progressChanged(double)), operationPartSize);
+            m_connectedOperations++;
         }
     }
 }
diff --git a/src/libs/installer/packagemanagercore_p.h b/src/libs/installer/packagemanagercore_p.h
index 61a7013fd..c0c55c4cc 100644
--- a/src/libs/installer/packagemanagercore_p.h
+++ b/src/libs/installer/packagemanagercore_p.h
@@ -341,6 +341,7 @@ private:
 
     QString m_datFileName;
     bool m_allowCompressedRepositoryInstall;
+    int m_connectedOperations;
 };
 
 } // namespace QInstaller
diff --git a/src/libs/installer/progresscoordinator.cpp b/src/libs/installer/progresscoordinator.cpp
index afcec1908..6413efe28 100644
--- a/src/libs/installer/progresscoordinator.cpp
+++ b/src/libs/installer/progresscoordinator.cpp
@@ -76,7 +76,8 @@ ProgressCoordinator *ProgressCoordinator::instance()
 
 void ProgressCoordinator::reset()
 {
-    disconnectAllSenders();
+    m_senderPartProgressSizeHash.clear();
+    m_senderPendingCalculatedPercentageHash.clear();
     m_installationLabelText.clear();
     m_currentCompletePercentage = 0;
     m_currentBasePercentage = 0;
@@ -90,10 +91,11 @@ void ProgressCoordinator::reset()
 void ProgressCoordinator::registerPartProgress(QObject *sender, const char *signal, double partProgressSize)
 {
     Q_ASSERT(sender);
+    Q_ASSERT(!sender->objectName().isEmpty());
     Q_ASSERT(QString::fromLatin1(signal).contains(QLatin1String("(double)")));
     Q_ASSERT(partProgressSize <= 1);
 
-    m_senderPartProgressSizeHash.insert(sender, partProgressSize);
+    m_senderPartProgressSizeHash.insert(sender->objectName(), partProgressSize);
     bool isConnected = connect(sender, signal, this, SLOT(partProgressChanged(double)));
     Q_UNUSED(isConnected);
     Q_ASSERT(isConnected);
@@ -116,16 +118,17 @@ void ProgressCoordinator::partProgressChanged(double fraction)
     }
 
     // no fraction no change
-    if (fraction == 0)
+    if (fraction == 0 || !sender())
         return;
+    QString senderObjectName = sender()->objectName();
 
     // ignore senders sending 100% multiple times
-    if (fraction == 1 && m_senderPendingCalculatedPercentageHash.contains(sender())
-        && m_senderPendingCalculatedPercentageHash.value(sender()) == 0) {
+    if (fraction == 1 && m_senderPendingCalculatedPercentageHash.contains(senderObjectName)
+        && m_senderPendingCalculatedPercentageHash.value(senderObjectName) == 0) {
         return;
     }
 
-    double partProgressSize = m_senderPartProgressSizeHash.value(sender(), 0);
+    double partProgressSize = m_senderPartProgressSizeHash.value(senderObjectName, 0);
     if (partProgressSize == 0) {
         qCWarning(QInstaller::lcInstallerInstallLog) << "It seems that this sender was not registered "
             "in the right way:" << sender();
@@ -138,7 +141,7 @@ void ProgressCoordinator::partProgressChanged(double fraction)
 
          // allPendingCalculatedPartPercentages has negative values
         double newCurrentCompletePercentage = m_currentBasePercentage - pendingCalculatedPartPercentage
-            + allPendingCalculatedPartPercentages(sender());
+            + allPendingCalculatedPartPercentages(senderObjectName);
 
         //we can't check this here, because some round issues can make it little bit under 0 or over 100
         //Q_ASSERT(newCurrentCompletePercentage >= 0);
@@ -163,9 +166,9 @@ void ProgressCoordinator::partProgressChanged(double fraction)
         m_currentCompletePercentage = newCurrentCompletePercentage;
         if (fraction == 1) {
             m_currentBasePercentage = m_currentBasePercentage - pendingCalculatedPartPercentage;
-            m_senderPendingCalculatedPercentageHash.insert(sender(), 0);
+            m_senderPendingCalculatedPercentageHash.insert(senderObjectName, 0);
         } else {
-            m_senderPendingCalculatedPercentageHash.insert(sender(), pendingCalculatedPartPercentage);
+            m_senderPendingCalculatedPercentageHash.insert(senderObjectName, pendingCalculatedPartPercentage);
         }
 
     } else { //if (m_undoMode)
@@ -174,7 +177,7 @@ void ProgressCoordinator::partProgressChanged(double fraction)
         //double checkValue = allPendingCalculatedPartPercentages(sender());
 
         double newCurrentCompletePercentage = m_manualAddedPercentage + m_currentBasePercentage
-            + pendingCalculatedPartPercentage + allPendingCalculatedPartPercentages(sender());
+            + pendingCalculatedPartPercentage + allPendingCalculatedPartPercentages(senderObjectName);
 
         //we can't check this here, because some round issues can make it little bit under 0 or over 100
         //Q_ASSERT(newCurrentCompletePercentage >= 0);
@@ -199,9 +202,9 @@ void ProgressCoordinator::partProgressChanged(double fraction)
 
         if (fraction == 1) {
             m_currentBasePercentage = m_currentBasePercentage + pendingCalculatedPartPercentage;
-            m_senderPendingCalculatedPercentageHash.insert(sender(), 0);
+            m_senderPendingCalculatedPercentageHash.insert(senderObjectName, 0);
         } else {
-            m_senderPendingCalculatedPercentageHash.insert(sender(), pendingCalculatedPartPercentage);
+            m_senderPendingCalculatedPercentageHash.insert(senderObjectName, pendingCalculatedPartPercentage);
         }
     } //if (m_undoMode)
     printProgressPercentage(progressInPercentage());
@@ -219,25 +222,13 @@ int ProgressCoordinator::progressInPercentage() const
     return currentValue;
 }
 
-void ProgressCoordinator::disconnectAllSenders()
-{
-    foreach (QPointer<QObject> sender, m_senderPartProgressSizeHash.keys()) {
-        if (!sender.isNull()) {
-            bool isDisconnected = sender->disconnect(this);
-            Q_UNUSED(isDisconnected);
-            Q_ASSERT(isDisconnected);
-        }
-    }
-    m_senderPartProgressSizeHash.clear();
-    m_senderPendingCalculatedPercentageHash.clear();
-}
-
 void ProgressCoordinator::setUndoMode()
 {
     Q_ASSERT(!m_undoMode);
     m_undoMode = true;
 
-    disconnectAllSenders();
+    m_senderPartProgressSizeHash.clear();
+    m_senderPendingCalculatedPercentageHash.clear();
     m_reachedPercentageBeforeUndo = progressInPercentage();
     m_currentBasePercentage = m_reachedPercentageBeforeUndo;
 }
@@ -294,10 +285,10 @@ void ProgressCoordinator::emitLabelAndDetailTextChanged(const QString &text)
     qApp->processEvents(); //makes the result available in the ui
 }
 
-double ProgressCoordinator::allPendingCalculatedPartPercentages(QObject *excludeKeyObject)
+double ProgressCoordinator::allPendingCalculatedPartPercentages(const QString &excludeKeyObject)
 {
     double result = 0;
-    QHash<QPointer<QObject>, double>::iterator it = m_senderPendingCalculatedPercentageHash.begin();
+    QHash<QString, double>::iterator it = m_senderPendingCalculatedPercentageHash.begin();
     while (it != m_senderPendingCalculatedPercentageHash.end()) {
         if (it.key() != excludeKeyObject)
             result += it.value();
diff --git a/src/libs/installer/progresscoordinator.h b/src/libs/installer/progresscoordinator.h
index 3540b5d16..75d5a5d30 100644
--- a/src/libs/installer/progresscoordinator.h
+++ b/src/libs/installer/progresscoordinator.h
@@ -85,12 +85,12 @@ protected:
     explicit ProgressCoordinator(QObject *parent);
 
 private:
-    double allPendingCalculatedPartPercentages(QObject *excludeKeyObject = 0);
+    double allPendingCalculatedPartPercentages(const QString &excludeKeyObject = QString());
     void disconnectAllSenders();
 
 private:
-    QHash<QPointer<QObject>, double> m_senderPendingCalculatedPercentageHash;
-    QHash<QPointer<QObject>, double> m_senderPartProgressSizeHash;
+    QHash<QString, double> m_senderPendingCalculatedPercentageHash;
+    QHash<QString, double> m_senderPartProgressSizeHash;
     ProgressSpinner *m_progressSpinner;
     QString m_installationLabelText;
     double m_currentCompletePercentage;