From 4f2eb122df6c68d0046a91cfe8c92974c13177f0 Mon Sep 17 00:00:00 2001
From: Jarek Kobus <jaroslaw.kobus@qt.io>
Date: Tue, 5 Nov 2024 09:48:12 +0100
Subject: [PATCH] QmlPreview: Fix resetting tasktree from its handler

It may happen that we are resetting the running task tree
directly from one of its handler, which may lead to crash.
The code path may look like:
1. onParserDone [qmlpreviewplugin.cpp : 508]
   task tree is still running
2. QmlPreviewPluginPrivate::triggerPreview
3. QmlPreviewPlugin::previewCurrentFile
4. QmlPreviewPlugin::setPreviewedFile
5. QmlPreviewPlugin::previewedFileChanged
6. QmlPreviewPluginPrivate::checkFile
7. QmlPreviewPluginPrivate::checkDocument
8. and now we are resetting the still running task tree.

Fix it by providing TaskTreeRunner's done handler,
which is safe to be used in such a scenario, as before
the handler is invoked, the finished task tree is already
scheduled for deletion by a call to deleteLater().

Add a test for TaskTreeRunner confirming such a scenario
is valid.

Amends bbed1896561b2f62449cd4f87ef95459c93d2fa1

Change-Id: I8671ffd4e82c505c6ed98687948db1273114e65a
Reviewed-by: Eike Ziller <eike.ziller@qt.io>
---
 src/plugins/qmlpreview/qmlpreviewplugin.cpp  |  7 +++++--
 tests/auto/solutions/tasking/tst_tasking.cpp | 17 +++++++++++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/plugins/qmlpreview/qmlpreviewplugin.cpp b/src/plugins/qmlpreview/qmlpreviewplugin.cpp
index 3b0f8c281f2..d87d0516a1e 100644
--- a/src/plugins/qmlpreview/qmlpreviewplugin.cpp
+++ b/src/plugins/qmlpreview/qmlpreviewplugin.cpp
@@ -505,8 +505,11 @@ void QmlPreviewPluginPrivate::checkDocument(const QString &name, const QByteArra
     const auto onParseSetup = [name, contents, dialect](Utils::Async<void> &async) {
         async.setConcurrentCallData(parse, name, contents, dialect);
     };
-    const auto onParseDone = [this, name, contents] { triggerPreview(name, contents); };
-    m_parseRunner.start({Utils::AsyncTask<void>(onParseSetup, onParseDone, CallDoneIf::Success)});
+    const auto onDone = [this, name, contents](DoneWith result) {
+        if (result == DoneWith::Success)
+            triggerPreview(name, contents);
+    };
+    m_parseRunner.start({Utils::AsyncTask<void>(onParseSetup)}, {}, onDone);
 }
 
 } // namespace QmlPreview
diff --git a/tests/auto/solutions/tasking/tst_tasking.cpp b/tests/auto/solutions/tasking/tst_tasking.cpp
index e1f37bbc001..80d581ac6b3 100644
--- a/tests/auto/solutions/tasking/tst_tasking.cpp
+++ b/tests/auto/solutions/tasking/tst_tasking.cpp
@@ -4,6 +4,7 @@
 #include <tasking/barrier.h>
 #include <tasking/concurrentcall.h>
 #include <tasking/conditional.h>
+#include <tasking/tasktreerunner.h>
 
 #include <QtTest>
 #include <QHash>
@@ -124,6 +125,7 @@ private slots:
     void nestedBrokenStorage();
     void restart();
     void destructorOfTaskEmittingDone();
+    void restartTaskTreeRunnerFromDoneHandler();
     void validConditionalConstructs();
 };
 
@@ -4251,6 +4253,21 @@ void tst_Tasking::destructorOfTaskEmittingDone()
     taskTree.start();
 }
 
+void tst_Tasking::restartTaskTreeRunnerFromDoneHandler()
+{
+    TaskTreeRunner runner;
+    QStringList log;
+    QStringList expectedLog{"1", "2"};
+
+    const auto onFirstDone = [&runner, &log](DoneWith) {
+        log.append("1");
+        runner.start({TestTask()}, {}, [&log](DoneWith) { log.append("2"); });
+    };
+    runner.start({TestTask()}, {}, onFirstDone);
+    QTRY_VERIFY(!runner.isRunning());
+    QCOMPARE(log, expectedLog);
+}
+
 void tst_Tasking::validConditionalConstructs()
 {
     const TestTask condition;