Fix nullpointer access and heap-use-after-free error

Task-number: QBS-1485
Change-Id: Id43e997a73ff55c3b438edb553806b61d45a8bdf
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

2 files changed, 19 insertions, 7 deletions

diff --git a/scripts/thread-sanitizer-suppressions.txt b/scripts/thread-sanitizer-suppressions.txt
new file mode 100644
index 000000000..9b2a9baa9
--- /dev/null
+++ b/scripts/thread-sanitizer-suppressions.txt
@@ -0,0 +1 @@
+called_from_lib:libtsan.so
diff --git a/src/lib/corelib/buildgraph/jscommandexecutor.cpp b/src/lib/corelib/buildgraph/jscommandexecutor.cpp
index 5c83b2056..aa72a7b5c 100644
--- a/src/lib/corelib/buildgraph/jscommandexecutor.cpp
+++ b/src/lib/corelib/buildgraph/jscommandexecutor.cpp
@@ -55,6 +55,7 @@
 #include <tools/qbsassert.h>
 
 #include <QtCore/qeventloop.h>
+#include <QtCore/qpointer.h>
 #include <QtCore/qthread.h>
 #include <QtCore/qtimer.h>
 
@@ -85,10 +86,11 @@ public:
 
     void cancel(const qbs::ErrorInfo &reason)
     {
-        QBS_ASSERT(m_scriptEngine, return);
         m_result.success = !reason.hasError();
         m_result.errorMessage = reason.toString();
-        m_scriptEngine->abortEvaluation();
+        if (m_scriptEngine)
+            m_scriptEngine->abortEvaluation();
+        m_cancelled = true;
     }
 
 signals:
@@ -97,6 +99,11 @@ signals:
 public:
     void start(const JavaScriptCommand *cmd, Transformer *transformer)
     {
+        if (m_cancelled) {
+            emit finished();
+            return;
+        }
+
         m_running = true;
         try {
             doStart(cmd, transformer);
@@ -184,6 +191,7 @@ private:
     ScriptEngine *m_scriptEngine;
     JavaScriptCommandResult m_result;
     bool m_running = false;
+    bool m_cancelled = false;
 };
 
 
@@ -203,9 +211,9 @@ JsCommandExecutor::JsCommandExecutor(const Logger &logger, QObject *parent)
 JsCommandExecutor::~JsCommandExecutor()
 {
     waitForFinished();
-    delete m_objectInThread;
     m_thread->quit();
     m_thread->wait();
+    delete m_objectInThread;
 }
 
 void JsCommandExecutor::doReportCommandDescription(const QString &productName)
@@ -225,20 +233,20 @@ void JsCommandExecutor::waitForFinished()
     if (!m_running)
         return;
     QEventLoop loop;
-    connect(m_objectInThread, &JsCommandExecutorThreadObject::finished, &loop, &QEventLoop::quit);
+    connect(this, &AbstractCommandExecutor::finished, &loop, &QEventLoop::quit);
     loop.exec();
 }
 
 bool JsCommandExecutor::doStart()
 {
     QBS_ASSERT(!m_running, return false);
-    m_thread->start();
 
     if (dryRun() && !command()->ignoreDryRun()) {
         QTimer::singleShot(0, this, [this] { emit finished(); }); // Don't call back on the caller.
         return false;
     }
 
+    m_thread->start();
     m_running = true;
     emit startRequested(jsCommand(), transformer());
     return true;
@@ -246,8 +254,11 @@ bool JsCommandExecutor::doStart()
 
 void JsCommandExecutor::cancel(const qbs::ErrorInfo &reason)
 {
-    if (m_running && !dryRun())
-        QTimer::singleShot(0, m_objectInThread, [objectInThread = m_objectInThread, reason] { objectInThread->cancel(reason); });
+    if (m_running && (!dryRun() || command()->ignoreDryRun()))
+        QTimer::singleShot(0, m_objectInThread, [objectInThread = QPointer<JsCommandExecutorThreadObject>{m_objectInThread}, reason] {
+            if (objectInThread)
+                objectInThread->cancel(reason);
+        });
 }
 
 void JsCommandExecutor::onJavaScriptCommandFinished()