From e2e42e3b6f2b317f814aff2858a35a0993c94724 Mon Sep 17 00:00:00 2001
From: Christian Mollekopf <chrigi_1@fastmail.fm>
Date: Fri, 31 Jul 2015 00:51:18 +0200
Subject: Fixed possible race conditions.

* Ensure we always create the thread-local event loop before any objects
in the thread are created, and guarantee the done handler is
immediately registered before the query can execute.
* Call the callback on emitter destruction in the worker thread, where
the eventloop lives, instead of the main thread.

With this I can no longer reproduce any deadlocks or memory corruptions
that I used to get occasionally before.
---
 common/clientapi.cpp    | 14 ++++++--------
 common/clientapi.h      |  8 ++++----
 common/resultprovider.h |  4 +++-
 3 files changed, 13 insertions(+), 13 deletions(-)

(limited to 'common')

diff --git a/common/clientapi.cpp b/common/clientapi.cpp
index a98340c..8bb244c 100644
--- a/common/clientapi.cpp
+++ b/common/clientapi.cpp
@@ -5,26 +5,24 @@
 #include "resourcefacade.h"
 #include "log.h"
 #include <QtConcurrent/QtConcurrentRun>
-#define ASYNCINTHREAD
-#ifndef ASYNCINTHREAD
 #include <QTimer>
-#endif
+
+#define ASYNCINTHREAD
 
 namespace async
 {
     void run(const std::function<void()> &runner) {
-#ifndef ASYNCINTHREAD
         auto timer = new QTimer();
         timer->setSingleShot(true);
         QObject::connect(timer, &QTimer::timeout, [runner, timer]() {
             delete timer;
+#ifndef ASYNCINTHREAD
             runner();
-        });
-        timer->start(0);
 #else
-        //TODO use a job that runs in a thread?
-        QtConcurrent::run(runner);
+            QtConcurrent::run(runner);
 #endif
+        });
+        timer->start(0);
     };
 } // namespace async
 
diff --git a/common/clientapi.h b/common/clientapi.h
index d860305..3ff8472 100644
--- a/common/clientapi.h
+++ b/common/clientapi.h
@@ -137,6 +137,10 @@ public:
         //We must guarantee that the emitter is returned before the first result is emitted.
         //The result provider must be threadsafe.
         async::run([query, resultSet](){
+            QEventLoop eventLoop;
+            resultSet->onDone([&eventLoop](){
+                eventLoop.quit();
+            });
             // Query all resources and aggregate results
             KAsync::iterate(getResources(query.resources))
             .template each<void, QByteArray>([query, resultSet](const QByteArray &resource, KAsync::Future<void> &future) {
@@ -158,10 +162,6 @@ public:
 
             //Keep the thread alive until the result is ready
             if (!resultSet->isDone()) {
-                QEventLoop eventLoop;
-                resultSet->onDone([&eventLoop](){
-                    eventLoop.quit();
-                });
                 eventLoop.exec();
             }
         });
diff --git a/common/resultprovider.h b/common/resultprovider.h
index 4d985d9..a375815 100644
--- a/common/resultprovider.h
+++ b/common/resultprovider.h
@@ -100,7 +100,7 @@ public:
     {
         if (!mResultEmitter) {
             //We have to go over a separate var and return that, otherwise we'd delete the emitter immediately again
-            auto sharedPtr = QSharedPointer<ResultEmitter<T> >(new ResultEmitter<T>, [this](ResultEmitter<T> *emitter){ done(); delete emitter; });
+            auto sharedPtr = QSharedPointer<ResultEmitter<T> >(new ResultEmitter<T>, [this](ResultEmitter<T> *emitter){ mThreadBoundary->callInMainThread([this]() {done();}); delete emitter; });
             mResultEmitter = sharedPtr;
             return sharedPtr;
         }
@@ -128,6 +128,7 @@ public:
 
     void onDone(const std::function<void()> &callback)
     {
+        mThreadBoundary = QSharedPointer<ThreadBoundary>::create();
         mOnDoneCallback = callback;
     }
 
@@ -151,6 +152,7 @@ private:
     QSharedPointer<QObject> mQueryRunner;
     std::shared_ptr<void> mFacade;
     std::function<void()> mOnDoneCallback;
+    QSharedPointer<ThreadBoundary> mThreadBoundary;
 };
 
 /*
-- 
cgit v1.2.3