On Sat, Jun 13, 2015 at 9:26 PM, David Faure <fa...@kde.org> wrote:
> 3 bis:
> I assume the threads without an event loop have some way to get tasks, right?
> So when the gui thread gets notified about ksycoca changes, it should post a
> task to these threads-with-no-eventloop which says "sycoca has changed".
> It's ok if they keep using an old instance meanwhile (the mmap'ed file uses
> refcounting in the kernel).
> When the thread finally gets the task it can call
> KSycoca::notifyDatabaseChanged  (it's a private slot, use
> QMetaObject::invokeMethod for now, if it works we can make it public).

I've tried a hackish way to communicate with the other threads. See
attached patch.

This doesn't quite work since KSysCocaPrivate::checkDatabase isn't
always called before finding an entry. Resetting the db in findEntry
leads of crashes cause the state of the stream isn't as expected by
the factory.

Any tips?

--
Vishesh Handa
commit 3f7fab8756db5e1773af862eda6dd9322b49af22
Author: Vishesh Handa <m...@vhanda.in>
Date:   Fri Jun 19 21:44:15 2015 +0200

    Hookup databaseChanged signals to other threads
    
    When the db changes, we are notified via dbus. This calls a slot, which
    which closes the db. This works out well for when each of those threads
    has an event loop so that the slot can be called, but otherwise, the db
    never gets updated.
    
    We now mark the db as invalid for all threads from the thread that
    receieves it.
    
    THIS DOES NOT WORK! I'm not sure how the db recreation logic works.

diff --git a/src/sycoca/ksycoca.cpp b/src/sycoca/ksycoca.cpp
index 32d1689..115a80c 100644
--- a/src/sycoca/ksycoca.cpp
+++ b/src/sycoca/ksycoca.cpp
@@ -34,6 +34,8 @@
 #include <QtCore/QFile>
 #include <QtCore/QFileInfo>
 #include <QtCore/QBuffer>
+#include <QMutex>
+#include <QMutexLocker>
 #include <QProcess>
 #include <QDBusConnection>
 #include <QDBusConnectionInterface>
@@ -89,6 +91,7 @@ KSycocaPrivate::KSycocaPrivate()
       updateSig(0),
       sycoca_size(0),
       sycoca_mmap(0),
+      dbInvalid(false),
       m_mmapFile(0),
       m_device(0)
 {
@@ -167,7 +170,15 @@ public:
     KSycoca *sycoca()
     {
         if (!m_threadSycocas.hasLocalData()) {
-            m_threadSycocas.setLocalData(new KSycoca);
+            KSycoca* sycoca = new KSycoca;
+            QObject::connect(sycoca, &QObject::destroyed, [&](QObject* obj) {
+                QMutexLocker lock(&s_allSycocasMutex);
+                s_allSycocas.removeOne(qobject_cast<KSycoca*>(obj));
+            });
+
+            m_threadSycocas.setLocalData(sycoca);
+            QMutexLocker lock(&s_allSycocasMutex);
+            s_allSycocas << sycoca;
         }
         return m_threadSycocas.localData();
     }
@@ -176,10 +187,15 @@ public:
         m_threadSycocas.setLocalData(s);
     }
 
+    static QList<KSycoca*> s_allSycocas;
+    static QMutex s_allSycocasMutex;
 private:
     QThreadStorage<KSycoca *> m_threadSycocas;
 };
 
+QList<KSycoca*> KSycocaSingleton::s_allSycocas;
+QMutex KSycocaSingleton::s_allSycocasMutex;
+
 Q_GLOBAL_STATIC(KSycocaSingleton, ksycocaInstance)
 
 // Read-only constructor
@@ -322,6 +338,7 @@ void KSycocaPrivate::closeDatabase()
 {
     delete m_device;
     m_device = 0;
+    dbInvalid = false;
 
     // It is very important to delete all factories here
     // since they cache information about the database file
@@ -359,6 +376,17 @@ bool KSycoca::isChanged(const char *type)
 
 void KSycoca::notifyDatabaseChanged(const QStringList &changeList)
 {
+    // We tell all the other threads that their db is invalid so it can be reloaded later
+    // This is important because the other threads might not have event loops running and
+    // therefore will get the dbus signal which calls this slot
+    QMutexLocker lock(&KSycocaSingleton::s_allSycocasMutex);
+    for (KSycoca* ksycoca : KSycocaSingleton::s_allSycocas) {
+        qDebug() << "MARKING";
+        ksycoca->d->dbInvalid = true;
+    }
+    qDebug() << "---";
+    lock.unlock();
+
     d->changeList = changeList;
     //qCDebug(SYCOCA) << QThread::currentThread() << "got a notifyDatabaseChanged signal" << changeList;
     // kbuildsycoca tells us the database file changed
@@ -417,12 +445,13 @@ KSERVICE_EXPORT bool kservice_require_kded = true;
 // and past the version number.
 bool KSycocaPrivate::checkDatabase(BehaviorsIfNotFound ifNotFound)
 {
-    if (databaseStatus == DatabaseOK) {
+    if (databaseStatus == DatabaseOK && !dbInvalid) {
         if (checkVersion()) { // we know the version is ok, but we must rewind the stream anyway
             return true;
         }
     }
 
+    qDebug() << "CLOSING DB ... ";
     closeDatabase(); // close the dummy one
 
     const QString KDED_SERVICE_NAME=QStringLiteral("org.kde.kded5");
@@ -434,9 +463,10 @@ bool KSycocaPrivate::checkDatabase(BehaviorsIfNotFound ifNotFound)
                              qAppName() == QLatin1String("kbuildsycoca5");
 
     // Check if new database already available
-    if ((kdedRunning || !kservice_require_kded) && openDatabase(ifNotFound & IfNotFoundOpenDummy)) {
+    if (!dbInvalid && (kdedRunning || !kservice_require_kded) && openDatabase(ifNotFound & IfNotFoundOpenDummy)) {
         if (checkVersion()) {
             // Database exists, and version is ok.
+            qDebug() << "DB Exists ...";
             return true;
         }
     }
diff --git a/src/sycoca/ksycoca_p.h b/src/sycoca/ksycoca_p.h
index 393cfcf..9f0a5b5 100644
--- a/src/sycoca/ksycoca_p.h
+++ b/src/sycoca/ksycoca_p.h
@@ -62,6 +62,7 @@ public:
     QString language;
     quint32 updateSig;
     QStringList allResourceDirs;
+    bool dbInvalid;
 
     void addFactory(KSycocaFactory *factory)
     {

Reply via email to