Metadata cache: clear cache in a separate thread

This removes the visible blocked UI while clearing cache items, which
takes some time. Disable controls on the dialog and show a basic
progress bar while the clearing is in progress.

Also add basic access serialization to GenericDataCache<T> class
public methods. It is still possible that the user closes the settings
dialog with other means before the clearing in a separate thread was
completed, and we want to avoid crashing to race conditions in such
case. If the user manages to navigate forward in installer pages, to
fetch metadata again, it wouldn't still work correctly though.

Task-number: QTIFW-2815
Change-Id: Ic2c0feac44aff69c458105cf0f559e8693fc0d69
Reviewed-by: Iikka Eklund <iikka.eklund@qt.io>
Reviewed-by: Katja Marttila <katja.marttila@qt.io>

6 files changed, 121 insertions, 23 deletions

diff --git a/src/libs/installer/genericdatacache.cpp b/src/libs/installer/genericdatacache.cpp
index 1b33976c8..b70cbcb0b 100644
--- a/src/libs/installer/genericdatacache.cpp
+++ b/src/libs/installer/genericdatacache.cpp
@@ -174,6 +174,7 @@ GenericDataCache<T>::~GenericDataCache()
 template<typename T>
 void GenericDataCache<T>::setType(const QString &type)
 {
+    QMutexLocker _(&m_mutex);
     m_type = type;
 }
 
@@ -186,6 +187,7 @@ void GenericDataCache<T>::setType(const QString &type)
 template<typename T>
 void GenericDataCache<T>::setVersion(const QString &version)
 {
+    QMutexLocker _(&m_mutex);
     m_version = version;
 }
 
@@ -200,6 +202,7 @@ void GenericDataCache<T>::setVersion(const QString &version)
 template <typename T>
 bool GenericDataCache<T>::initialize()
 {
+    QMutexLocker _(&m_mutex);
     Q_ASSERT(m_items.isEmpty());
 
     if (m_path.isEmpty()) {
@@ -247,6 +250,7 @@ bool GenericDataCache<T>::initialize()
 template <typename T>
 bool GenericDataCache<T>::clear()
 {
+    QMutexLocker _(&m_mutex);
     if (m_invalidated) {
         setErrorString(QCoreApplication::translate("GenericDataCache",
             "Cannot clear invalidated cache."));
@@ -286,6 +290,7 @@ bool GenericDataCache<T>::clear()
 template<typename T>
 bool GenericDataCache<T>::sync()
 {
+    QMutexLocker _(&m_mutex);
     if (m_invalidated) {
         setErrorString(QCoreApplication::translate("GenericDataCache",
             "Cannot synchronize invalidated cache."));
@@ -303,6 +308,7 @@ bool GenericDataCache<T>::sync()
 template<typename T>
 bool GenericDataCache<T>::isValid() const
 {
+    QMutexLocker _(&m_mutex);
     return !m_invalidated;
 }
 
@@ -314,6 +320,7 @@ bool GenericDataCache<T>::isValid() const
 template<typename T>
 QString GenericDataCache<T>::errorString() const
 {
+    QMutexLocker _(&m_mutex);
     return m_error;
 }
 
@@ -325,6 +332,7 @@ QString GenericDataCache<T>::errorString() const
 template <typename T>
 QString GenericDataCache<T>::path() const
 {
+    QMutexLocker _(&m_mutex);
     return m_path;
 }
 
@@ -337,6 +345,7 @@ QString GenericDataCache<T>::path() const
 template <typename T>
 void GenericDataCache<T>::setPath(const QString &path)
 {
+    QMutexLocker _(&m_mutex);
     if (!m_invalidated)
         toDisk();
 
@@ -352,6 +361,7 @@ void GenericDataCache<T>::setPath(const QString &path)
 template <typename T>
 QList<T *> GenericDataCache<T>::items() const
 {
+    QMutexLocker _(&m_mutex);
     if (m_invalidated) {
         setErrorString(QCoreApplication::translate("GenericDataCache",
             "Cannot retrieve items from invalidated cache."));
@@ -369,6 +379,7 @@ QList<T *> GenericDataCache<T>::items() const
 template <typename T>
 T *GenericDataCache<T>::itemByChecksum(const QByteArray &checksum) const
 {
+    QMutexLocker _(&m_mutex);
     if (m_invalidated) {
         setErrorString(QCoreApplication::translate("GenericDataCache",
             "Cannot retrieve item from invalidated cache."));
@@ -387,6 +398,7 @@ T *GenericDataCache<T>::itemByChecksum(const QByteArray &checksum) const
 template <typename T>
 T *GenericDataCache<T>::itemByPath(const QString &path) const
 {
+    QMutexLocker _(&m_mutex);
     auto it = std::find_if(m_items.constBegin(), m_items.constEnd(),
         [&](T *item) {
             return (QDir::fromNativeSeparators(path) == QDir::fromNativeSeparators(item->path()));
@@ -413,6 +425,7 @@ T *GenericDataCache<T>::itemByPath(const QString &path) const
 template <typename T>
 bool GenericDataCache<T>::registerItem(T *item, bool replace)
 {
+    QMutexLocker _(&m_mutex);
     if (m_invalidated) {
         setErrorString(QCoreApplication::translate("GenericDataCache",
             "Cannot register item to invalidated cache."));
@@ -430,7 +443,7 @@ bool GenericDataCache<T>::registerItem(T *item, bool replace)
     }
     if (m_items.contains(item->checksum())) {
         if (replace) {// replace existing item including contents on disk
-            removeItem(item->checksum());
+            remove(item->checksum());
         } else {
             setErrorString(QCoreApplication::translate("GenericDataCache",
                 "Cannot register item with checksum %1. An item with the same checksum "
@@ -470,26 +483,8 @@ bool GenericDataCache<T>::registerItem(T *item, bool replace)
 template <typename T>
 bool GenericDataCache<T>::removeItem(const QByteArray &checksum)
 {
-    if (m_invalidated) {
-        setErrorString(QCoreApplication::translate("GenericDataCache",
-            "Cannot remove item from invalidated cache."));
-        return false;
-    }
-    QScopedPointer<T> item(m_items.take(checksum));
-    if (!item) {
-        setErrorString(QCoreApplication::translate("GenericDataCache",
-            "Cannot remove item specified by checksum %1: no such item exists.").arg(QLatin1String(checksum)));
-        return false;
-    }
-
-    try {
-        QInstaller::removeDirectory(item->path());
-    } catch (const Error &e) {
-        setErrorString(QCoreApplication::translate("GenericDataCache",
-            "Error while removing directory \"%1\": %2").arg(item->path(), e.message()));
-        return false;
-    }
-    return true;
+    QMutexLocker _(&m_mutex);
+    return remove(checksum);
 }
 
 /*!
@@ -500,6 +495,7 @@ bool GenericDataCache<T>::removeItem(const QByteArray &checksum)
 template<typename T>
 QList<T *> GenericDataCache<T>::obsoleteItems() const
 {
+    QMutexLocker _(&m_mutex);
     const QList<T *> obsoletes = QtConcurrent::blockingFiltered(m_items.values(),
         [&](T *item1) {
             if (item1->isActive()) // We can skip the iteration for active entries
@@ -638,6 +634,36 @@ bool GenericDataCache<T>::toDisk()
     return true;
 }
 
+/*!
+    \fn template <typename T> QInstaller::GenericDataCache<T>::remove(const QByteArray &checksum)
+
+    \internal
+*/
+template<typename T>
+bool GenericDataCache<T>::remove(const QByteArray &checksum)
+{
+    if (m_invalidated) {
+        setErrorString(QCoreApplication::translate("GenericDataCache",
+            "Cannot remove item from invalidated cache."));
+        return false;
+    }
+    QScopedPointer<T> item(m_items.take(checksum));
+    if (!item) {
+        setErrorString(QCoreApplication::translate("GenericDataCache",
+            "Cannot remove item specified by checksum %1: no such item exists.").arg(QLatin1String(checksum)));
+        return false;
+    }
+
+    try {
+        QInstaller::removeDirectory(item->path());
+    } catch (const Error &e) {
+        setErrorString(QCoreApplication::translate("GenericDataCache",
+            "Error while removing directory \"%1\": %2").arg(item->path(), e.message()));
+        return false;
+    }
+    return true;
+}
+
 template class GenericDataCache<Metadata>;
 
 } // namespace QInstaller
diff --git a/src/libs/installer/genericdatacache.h b/src/libs/installer/genericdatacache.h
index 5042ed9a4..2bdf6697e 100644
--- a/src/libs/installer/genericdatacache.h
+++ b/src/libs/installer/genericdatacache.h
@@ -33,6 +33,7 @@
 #include "lockfile.h"
 
 #include <QHash>
+#include <QMutex>
 #include <QScopedPointer>
 
 namespace QInstaller {
@@ -96,8 +97,12 @@ private:
     bool fromDisk();
     bool toDisk();
 
+    bool remove(const QByteArray &checksum);
+
 private:
     QScopedPointer<KDUpdater::LockFile> m_lock;
+    mutable QMutex m_mutex;
+
     QHash<QByteArray, T *> m_items;
     QString m_path;
     QString m_type;
diff --git a/src/sdk/settingsdialog.cpp b/src/sdk/settingsdialog.cpp
index b807deccd..acd364b2f 100644
--- a/src/sdk/settingsdialog.cpp
+++ b/src/sdk/settingsdialog.cpp
@@ -257,6 +257,13 @@ SettingsDialog::SettingsDialog(PackageManagerCore *core, QWidget *parent)
     }
 
    m_ui->m_cachePathLineEdit->setText(settings.localCachePath());
+   showClearCacheProgress(false);
+}
+
+void SettingsDialog::showClearCacheProgress(bool show)
+{
+    m_ui->m_clearCacheProgressLabel->setVisible(show);
+    m_ui->m_clearCacheProgressBar->setVisible(show);
 }
 
 void SettingsDialog::accept()
diff --git a/src/sdk/settingsdialog.h b/src/sdk/settingsdialog.h
index bc618d6b7..5f5c017a8 100644
--- a/src/sdk/settingsdialog.h
+++ b/src/sdk/settingsdialog.h
@@ -104,6 +104,8 @@ class SettingsDialog : public QDialog
 public:
     explicit SettingsDialog(QInstaller::PackageManagerCore *core, QWidget *parent = 0);
 
+    void showClearCacheProgress(bool show);
+
 public slots:
     void accept();
 
diff --git a/src/sdk/settingsdialog.ui b/src/sdk/settingsdialog.ui
index 0254abd98..9d4ba9577 100644
--- a/src/sdk/settingsdialog.ui
+++ b/src/sdk/settingsdialog.ui
@@ -304,6 +304,42 @@
         </layout>
        </item>
        <item>
+        <spacer name="verticalSpacer_3">
+         <property name="orientation">
+          <enum>Qt::Vertical</enum>
+         </property>
+         <property name="sizeHint" stdset="0">
+          <size>
+           <width>20</width>
+           <height>40</height>
+          </size>
+         </property>
+        </spacer>
+       </item>
+       <item>
+        <widget class="QLabel" name="m_clearCacheProgressLabel">
+         <property name="text">
+          <string>Clearing cache...</string>
+         </property>
+        </widget>
+       </item>
+       <item>
+        <widget class="QProgressBar" name="m_clearCacheProgressBar">
+         <property name="enabled">
+          <bool>true</bool>
+         </property>
+         <property name="maximum">
+          <number>0</number>
+         </property>
+         <property name="value">
+          <number>-1</number>
+         </property>
+         <property name="format">
+          <string>%p%</string>
+         </property>
+        </widget>
+       </item>
+       <item>
         <spacer name="verticalSpacer">
          <property name="orientation">
           <enum>Qt::Vertical</enum>
diff --git a/src/sdk/tabcontroller.cpp b/src/sdk/tabcontroller.cpp
index ae8fdbc25..c40ceed5f 100644
--- a/src/sdk/tabcontroller.cpp
+++ b/src/sdk/tabcontroller.cpp
@@ -38,6 +38,7 @@
 
 #include <QtCore/QTimer>
 #include <QtWidgets/QMessageBox>
+#include <QtConcurrent>
 
 using namespace QInstaller;
 
@@ -198,10 +199,31 @@ void TabController::onAboutApplicationClicked()
 
 void TabController::onClearCacheClicked()
 {
-    QDialog *settingsDialog = static_cast<QDialog *>(sender());
+    SettingsDialog *settingsDialog = static_cast<SettingsDialog *>(sender());
+    settingsDialog->setEnabled(false);
+    settingsDialog->showClearCacheProgress(true);
 
     QString errorMessage;
-    const bool success = d->m_core->clearLocalCache(&errorMessage);
+    bool success = true;
+
+    // Clearing might take some time, run in a separate thread
+    QEventLoop loop;
+    QFutureWatcher<bool> futureWatcher;
+
+    connect(&futureWatcher, &QFutureWatcher<bool>::finished, this, [&]() {
+        success = futureWatcher.future().result();
+        if (loop.isRunning())
+            loop.quit();
+    });
+
+    futureWatcher.setFuture(QtConcurrent::run(d->m_core,
+        &PackageManagerCore::clearLocalCache, &errorMessage));
+
+    if (!futureWatcher.isFinished())
+        loop.exec();
+
+    settingsDialog->setEnabled(true);
+    settingsDialog->showClearCacheProgress(false);
 
     QMessageBox msgBox(settingsDialog);
     msgBox.setWindowModality(Qt::WindowModal);