CtfVisualizer: Fix multithreading issues

Simplify the CtfVisualizerTool::loadJson(). Don't create a QThread
manually, but use TaskTree with AsyncTask instead.
Move pure parsing into the separate thread, and leave the
nlohmann::json event handling in the main thread.
Avoid moving m_modelAggregator between threads.

Fixes: QTCREATORBUG-29657
Change-Id: I0c6a9a4ea8298dbbdbafcddd338d39ad73c3f82b
Reviewed-by: Alessandro Portale <alessandro.portale@qt.io>

src/plugins/ctfvisualizer/ctftracemanager.cpp

@@ -8,17 +8,11 @@
 #include "ctfvisualizertr.h"
 
 #include <coreplugin/icore.h>
+
 #include <tracing/timelinemodelaggregator.h>
 
-#include <QByteArray>
-#include <QDebug>
-#include <QFile>
-#include <QList>
 #include <QMessageBox>
 
-#include <fstream>
-
-
 namespace CtfVisualizer {
 namespace Internal {
 
@@ -26,48 +20,6 @@ using json = nlohmann::json;
 
 using namespace Constants;
 
-
-class CtfJsonParserCallback
-{
-
-public:
-
-    explicit CtfJsonParserCallback(CtfTraceManager *traceManager)
-        : m_traceManager(traceManager)
-    {}
-
-    bool callback(int depth, nlohmann::json::parse_event_t event, nlohmann::json &parsed)
-    {
-        if ((event == json::parse_event_t::array_start && depth == 0)
-                || (event == json::parse_event_t::key && depth == 1 && parsed == json(CtfTraceEventsKey))) {
-            m_isInTraceArray = true;
-            m_traceArrayDepth = depth;
-            return true;
-        }
-        if (m_isInTraceArray && event == json::parse_event_t::array_end && depth == m_traceArrayDepth) {
-            m_isInTraceArray = false;
-            return false;
-        }
-        if (m_isInTraceArray && event == json::parse_event_t::object_end && depth == m_traceArrayDepth + 1) {
-            m_traceManager->addEvent(parsed);
-            return false;
-        }
-        if (m_isInTraceArray || (event == json::parse_event_t::object_start && depth == 0)) {
-            // keep outer object and values in trace objects:
-            return true;
-        }
-        // discard any objects outside of trace array:
-        // TODO: parse other data, e.g. stack frames
-        return false;
-    }
-
-protected:
-    CtfTraceManager *m_traceManager;
-
-    bool m_isInTraceArray = false;
-    int m_traceArrayDepth = 0;
-};
-
 CtfTraceManager::CtfTraceManager(QObject *parent,
                                  Timeline::TimelineModelAggregator *modelAggregator,
                                  CtfStatisticsModel *statisticsModel)
@@ -75,7 +27,6 @@ CtfTraceManager::CtfTraceManager(QObject *parent,
     , m_modelAggregator(modelAggregator)
     , m_statisticsModel(statisticsModel)
 {
-
 }
 
 qint64 CtfTraceManager::traceDuration() const
@@ -142,26 +93,6 @@ void CtfTraceManager::addEvent(const json &event)
     }
 }
 
-void CtfTraceManager::load(const QString &filename)
-{
-    clearAll();
-
-    std::ifstream file(filename.toStdString());
-    if (!file.is_open()) {
-        QMessageBox::warning(Core::ICore::dialogParent(),
-                             Tr::tr("CTF Visualizer"),
-                             Tr::tr("Cannot read the CTF file."));
-        return;
-    }
-    CtfJsonParserCallback ctfParser(this);
-    json::parser_callback_t callback = [&ctfParser](int depth, json::parse_event_t event, json &parsed) {
-        return ctfParser.callback(depth, event, parsed);
-    };
-    json unusedValues = json::parse(file, callback, /*allow_exceptions*/ false);
-    file.close();
-    updateStatistics();
-}
-
 void CtfTraceManager::finalize()
 {
     bool userConsentToIgnoreDeepTraces = false;

src/plugins/ctfvisualizer/ctftracemanager.h

@@ -5,13 +5,11 @@
 #include "json/json.hpp"
 
 #include <QHash>
+#include <QList>
 #include <QMap>
 #include <QObject>
-#include <QVector>
 
-namespace Timeline {
-class TimelineModelAggregator;
-}
+namespace Timeline { class TimelineModelAggregator; }
 
 namespace CtfVisualizer {
 namespace Internal {
@@ -34,7 +32,6 @@ public:
 
     void addEvent(const nlohmann::json &event);
 
-    void load(const QString &filename);
     void finalize();
 
     bool isEmpty() const;
@@ -46,6 +43,9 @@ public:
     void setThreadRestriction(const QString &tid, bool restrictToThisThread);
     bool isRestrictedTo(const QString &tid) const;
 
+    void updateStatistics();
+    void clearAll();
+
 signals:
     void detailsRequested(const QString &title);
 
@@ -53,10 +53,6 @@ protected:
     void addModelForThread(const QString &threadId, const QString &processId);
     void addModelsToAggregator();
 
-    void updateStatistics();
-
-    void clearAll();
-
     Timeline::TimelineModelAggregator *const m_modelAggregator;
     CtfStatisticsModel *const m_statisticsModel;
 

src/plugins/ctfvisualizer/ctfvisualizertool.cpp

@@ -13,30 +13,29 @@
 #include <coreplugin/actionmanager/actioncontainer.h>
 #include <coreplugin/actionmanager/actionmanager.h>
 #include <coreplugin/icore.h>
-#include <coreplugin/progressmanager/progressmanager.h>
+#include <coreplugin/progressmanager/taskprogress.h>
+
 #include <debugger/analyzer/analyzerconstants.h>
 
+#include <utils/async.h>
 #include <utils/stylehelper.h>
 #include <utils/utilsicons.h>
 
-#include <QAction>
-#include <QApplication>
 #include <QFileDialog>
-#include <QFutureInterface>
 #include <QMenu>
 #include <QMessageBox>
-#include <QThread>
+
+#include <fstream>
 
 using namespace Core;
 using namespace CtfVisualizer::Constants;
-
+using namespace Utils;
 
 namespace CtfVisualizer {
 namespace Internal {
 
 CtfVisualizerTool::CtfVisualizerTool()
     : QObject (nullptr)
-    , m_isLoading(false)
     , m_loadJson(nullptr)
     , m_traceView(nullptr)
     , m_modelAggregator(new Timeline::TimelineModelAggregator(this))
@@ -150,34 +149,84 @@ Timeline::TimelineZoomControl *CtfVisualizerTool::zoomControl() const
     return m_zoomControl.get();
 }
 
-void CtfVisualizerTool::loadJson(const QString &filename)
+class CtfJsonParserFunctor
 {
-    if (m_isLoading)
-        return;
+public:
+    CtfJsonParserFunctor(QPromise<nlohmann::json> &promise)
+        : m_promise(promise) {}
 
-    if (filename.isEmpty()) {
-        m_isLoading = false;
+    bool operator()(int depth, nlohmann::json::parse_event_t event, nlohmann::json &parsed)
+    {
+        using json = nlohmann::json;
+        if ((event == json::parse_event_t::array_start && depth == 0)
+            || (event == json::parse_event_t::key && depth == 1 && parsed == json(CtfTraceEventsKey))) {
+            m_isInTraceArray = true;
+            m_traceArrayDepth = depth;
+            return true;
+        }
+        if (m_isInTraceArray && event == json::parse_event_t::array_end && depth == m_traceArrayDepth) {
+            m_isInTraceArray = false;
+            return false;
+        }
+        if (m_isInTraceArray && event == json::parse_event_t::object_end && depth == m_traceArrayDepth + 1) {
+            m_promise.addResult(parsed);
+            return false;
+        }
+        if (m_isInTraceArray || (event == json::parse_event_t::object_start && depth == 0)) {
+            // keep outer object and values in trace objects:
+            return true;
+        }
+        // discard any objects outside of trace array:
+        // TODO: parse other data, e.g. stack frames
+        return false;
+    }
+
+protected:
+    QPromise<nlohmann::json> &m_promise;
+    bool m_isInTraceArray = false;
+    int m_traceArrayDepth = 0;
+};
+
+static void load(QPromise<nlohmann::json> &promise, const QString &fileName)
+{
+    using json = nlohmann::json;
+
+    std::ifstream file(fileName.toStdString());
+    if (!file.is_open()) {
+        promise.future().cancel();
         return;
     }
 
-    m_isLoading = true;
+    CtfJsonParserFunctor functor(promise);
+    json::parser_callback_t callback = [&functor](int depth, json::parse_event_t event, json &parsed) {
+        return functor(depth, event, parsed);
+    };
 
-    auto *futureInterface = new QFutureInterface<void>();
-    auto *task = new QFuture<void>(futureInterface);
+    try {
+        json unusedValues = json::parse(file, callback, /*allow_exceptions*/ false);
+    } catch (...) {
+        // nlohmann::json can throw exceptions when requesting type that is wrong
+    }
 
-    QThread *thread = QThread::create([this, filename, futureInterface]() {
-        try {
-            m_traceManager->load(filename);
-        } catch (...) {
-            // nlohmann::json can throw exceptions when requesting type that is wrong
-        }
-        m_modelAggregator->moveToThread(QApplication::instance()->thread());
-        m_modelAggregator->setParent(this);
-        futureInterface->reportFinished();
-    });
+    file.close();
+}
 
-    connect(thread, &QThread::finished, this, [this, thread, task, futureInterface]() {
-        // in main thread:
+void CtfVisualizerTool::loadJson(const QString &fileName)
+{
+    using namespace Tasking;
+
+    if (m_loader || fileName.isEmpty())
+        return;
+
+    const auto onSetup = [this, fileName](Async<nlohmann::json> &async) {
+        m_traceManager->clearAll();
+        async.setConcurrentCallData(load, fileName);
+        connect(&async, &AsyncBase::resultReadyAt, this, [this, asyncPtr = &async](int index) {
+            m_traceManager->addEvent(asyncPtr->resultAt(index));
+        });
+    };
+    const auto onDone = [this] {
+        m_traceManager->updateStatistics();
         if (m_traceManager->isEmpty()) {
             QMessageBox::warning(Core::ICore::dialogParent(),
                                  Tr::tr("CTF Visualizer"),
@@ -189,17 +238,22 @@ void CtfVisualizerTool::loadJson(const QString &filename)
             zoomControl()->setRange(m_traceManager->traceBegin(), m_traceManager->traceEnd() + m_traceManager->traceDuration() / 20);
         }
         setAvailableThreads(m_traceManager->getSortedThreads());
-        thread->deleteLater();
-        delete task;
-        delete futureInterface;
-        m_isLoading = false;
-    }, Qt::QueuedConnection);
+        m_loader.release()->deleteLater();
+    };
+    const auto onError = [this] {
+        QMessageBox::warning(Core::ICore::dialogParent(),
+                             Tr::tr("CTF Visualizer"),
+                             Tr::tr("Cannot read the CTF file."));
+        m_loader.release()->deleteLater();
+    };
 
-    m_modelAggregator->setParent(nullptr);
-    m_modelAggregator->moveToThread(thread);
-
-    thread->start();
-    Core::ProgressManager::addTask(*task, Tr::tr("Loading CTF File"), CtfVisualizerTaskLoadJson);
+    const Group recipe { AsyncTask<nlohmann::json>(onSetup) };
+    m_loader.reset(new TaskTree(recipe));
+    connect(m_loader.get(), &TaskTree::done, this, onDone);
+    connect(m_loader.get(), &TaskTree::errorOccurred, this, onError);
+    auto progress = new TaskProgress(m_loader.get());
+    progress->setDisplayName(Tr::tr("Loading CTF File"));
+    m_loader->start();
 }
 
 }  // namespace Internal

src/plugins/ctfvisualizer/ctfvisualizertool.h

@@ -6,12 +6,15 @@
 #include "ctfvisualizerconstants.h"
 
 #include <debugger/debuggermainwindow.h>
+
 #include <tracing/timelinemodelaggregator.h>
 #include <tracing/timelinezoomcontrol.h>
 
 #include <QCoreApplication>
 #include <QScopedPointer>
 
+namespace Tasking { class TaskTree; }
+
 namespace CtfVisualizer {
 namespace Internal {
 
@@ -21,7 +24,6 @@ class CtfStatisticsView;
 class CtfTimelineModel;
 class CtfVisualizerTraceView;
 
-
 class CtfVisualizerTool  : public QObject
 {
     Q_OBJECT
@@ -34,7 +36,7 @@ public:
     CtfTraceManager *traceManager() const;
     Timeline::TimelineZoomControl *zoomControl() const;
 
-    void loadJson(const QString &filename);
+    void loadJson(const QString &fileName);
 
 private:
     void createViews();
@@ -45,11 +47,11 @@ private:
     void setAvailableThreads(const QList<CtfTimelineModel *> &threads);
     void toggleThreadRestriction(QAction *action);
 
-    Utils::Perspective m_perspective{Constants::CtfVisualizerPerspectiveId,
+    Utils::Perspective m_perspective{CtfVisualizer::Constants::CtfVisualizerPerspectiveId,
                                      QCoreApplication::translate("QtC::CtfVisualizer",
                                                                  "Chrome Trace Format Visualizer")};
 
-    bool m_isLoading;
+    std::unique_ptr<Tasking::TaskTree> m_loader;
     QScopedPointer<QAction> m_loadJson;
 
     CtfVisualizerTraceView *m_traceView;