From 4ecb10d3b1294d03578c28467c0f3759b3496e0b Mon Sep 17 00:00:00 2001
From: Christian Mollekopf <chrigi_1@fastmail.fm>
Date: Tue, 7 Mar 2017 12:04:47 +0100
Subject: Resolved potential deadlock

When trying to reply to a mail from kube we ran into a deadlock.
The initial result callback is called from the main thread, and that can
thus directly lead to destruction of the emitter.
---
 common/modelresult.cpp  | 18 +++++++++---------
 common/resultprovider.h | 10 +++++++++-
 2 files changed, 18 insertions(+), 10 deletions(-)

(limited to 'common')

diff --git a/common/modelresult.cpp b/common/modelresult.cpp
index f935419..904766d 100644
--- a/common/modelresult.cpp
+++ b/common/modelresult.cpp
@@ -296,15 +296,15 @@ void ModelResult<T, Ptr>::setEmitter(const typename Sink::ResultEmitter<Ptr>::Pt
     emitter->onInitialResultSetComplete([this, guard](const Ptr &parent, bool fetchedAll) {
         SinkTraceCtx(mLogCtx) << "Initial result set complete. Fetched all: " << fetchedAll;
         Q_ASSERT(guard);
-        threadBoundary.callInMainThread([=]() {
-            const qint64 parentId = parent ? qHash(*parent) : 0;
-            const auto parentIndex = createIndexFromId(parentId);
-            mEntityChildrenFetchComplete.insert(parentId);
-            if (fetchedAll) {
-                mEntityAllChildrenFetched.insert(parentId);
-            }
-            emit dataChanged(parentIndex, parentIndex, QVector<int>() << ChildrenFetchedRole);
-        });
+        Q_ASSERT(QThread::currentThread() == this->thread());
+
+        const qint64 parentId = parent ? qHash(*parent) : 0;
+        const auto parentIndex = createIndexFromId(parentId);
+        mEntityChildrenFetchComplete.insert(parentId);
+        if (fetchedAll) {
+            mEntityAllChildrenFetched.insert(parentId);
+        }
+        emit dataChanged(parentIndex, parentIndex, QVector<int>() << ChildrenFetchedRole);
     });
     mEmitter = emitter;
 }
diff --git a/common/resultprovider.h b/common/resultprovider.h
index a2ed0b5..d6feaf9 100644
--- a/common/resultprovider.h
+++ b/common/resultprovider.h
@@ -268,8 +268,9 @@ public:
 
     void initialResultSetComplete(const DomainType &parent, bool replayedAll)
     {
-        QMutexLocker locker{&mMutex};
+        //This callback is only ever called from the main thread, so we don't do any locking
         if (initialResultSetCompleteHandler && guardOk()) {
+            //This can directly lead to our destruction and thus waitForMethodExecutionEnd
             initialResultSetCompleteHandler(parent, replayedAll);
         }
     }
@@ -313,6 +314,13 @@ private:
     std::function<void(void)> clearHandler;
 
     std::function<void(const DomainType &parent)> mFetcher;
+    /*
+     * This mutex is here to protect the emitter from getting destroyed while the producer-thread (ResultProvider) is calling into it,
+     * and vice-verca, to protect the producer thread from calling into a destroyed emitter.
+     *
+     * This is necessary because Emitter and ResultProvider have lifetimes managed by two different threads.
+     * The emitter lives in the application thread, and the resultprovider in the query thread.
+     */
     QMutex mMutex;
     bool mDone = false;
 };
-- 
cgit v1.2.3