Fix Race Condition in NodePostConstructorInit::processNodes

This fixes QTBUG-116770. This moves m_aspectEngine to the correct thread
when it's created in Scene3DItem. And prevents processNodes from
being called on the wrong thread.

[ChangeLog] Fix Race Condition in NodePostConstructorInit::processNodes

Fixes: QTBUG-116770
Pick-to: 6.5
Change-Id: Iaf47ffd99ab6f920559b596a9baa8c253c135e40
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>
(cherry picked from commit e275b1c286d223463409c57a7300b7ddc56df061)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

4 files changed, 12 insertions, 6 deletions

diff --git a/src/core/nodes/qnode.cpp b/src/core/nodes/qnode.cpp
index 5ba46a7c5..7858a1c54 100644
--- a/src/core/nodes/qnode.cpp
+++ b/src/core/nodes/qnode.cpp
@@ -12,6 +12,7 @@
 #include <QtCore/QEvent>
 #include <QtCore/QMetaObject>
 #include <QtCore/QMetaProperty>
+#include <QtCore/QThread>
 
 #include <Qt3DCore/private/corelogging_p.h>
 #include <Qt3DCore/private/qdestructionidandtypecollector_p.h>
@@ -877,6 +878,7 @@ void NodePostConstructorInit::removeNode(QNode *node)
  */
 void NodePostConstructorInit::processNodes()
 {
+    Q_ASSERT(thread() == QThread::currentThread());
     m_requestedProcessing = false;
     while (!m_nodesToConstruct.empty()) {
         auto node = m_nodesToConstruct.takeFirst();
diff --git a/src/core/qscene.cpp b/src/core/qscene.cpp
index 3068eeda3..d4a0a1529 100644
--- a/src/core/qscene.cpp
+++ b/src/core/qscene.cpp
@@ -8,6 +8,7 @@
 #include <QtCore/QReadLocker>
 
 #include <Qt3DCore/private/qnode_p.h>
+#include <Qt3DCore/qaspectengine.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -19,7 +20,8 @@ public:
     QScenePrivate(QAspectEngine *engine)
         : m_engine(engine)
         , m_arbiter(nullptr)
-        , m_postConstructorInit(new NodePostConstructorInit)
+        //m_postConstructorInit needs the parent set correctly for QObject::moveToThread() to work correctly
+        , m_postConstructorInit(new NodePostConstructorInit(engine))
         , m_rootNode(nullptr)
     {
     }
@@ -28,7 +30,7 @@ public:
     QHash<QNodeId, QNode *> m_nodeLookupTable;
     QMultiHash<QNodeId, QNodeId> m_componentToEntities;
     QChangeArbiter *m_arbiter;
-    QScopedPointer<NodePostConstructorInit> m_postConstructorInit;
+    QScopedPointer<NodePostConstructorInit, QScopedPointerDeleteLater> m_postConstructorInit;
     mutable QReadWriteLock m_lock;
     mutable QReadWriteLock m_nodePropertyTrackModeLock;
     QNode *m_rootNode;
diff --git a/src/quick3d/imports/scene3d/scene3ditem.cpp b/src/quick3d/imports/scene3d/scene3ditem.cpp
index 53f238927..ef0ae6409 100644
--- a/src/quick3d/imports/scene3d/scene3ditem.cpp
+++ b/src/quick3d/imports/scene3d/scene3ditem.cpp
@@ -769,6 +769,7 @@ QSGNode *Scene3DItem::updatePaintNode(QSGNode *node, QQuickItem::UpdatePaintNode
             // Needs to belong in the same thread as the item which is the same as
             // the original QAspectEngine
             m_aspectEngineDestroyer->moveToThread(thread());
+            m_aspectEngine->moveToThread(thread());
 
             // To destroy AspectEngine
             m_aspectEngineDestroyer->reset(2);
diff --git a/tests/auto/render/qmesh/tst_qmesh.cpp b/tests/auto/render/qmesh/tst_qmesh.cpp
index 6dad93483..c10890872 100644
--- a/tests/auto/render/qmesh/tst_qmesh.cpp
+++ b/tests/auto/render/qmesh/tst_qmesh.cpp
@@ -8,6 +8,7 @@ QT_WARNING_DISABLE_DEPRECATED
 #include <QtTest/QTest>
 #include <Qt3DRender/qmesh.h>
 #include <Qt3DRender/private/qmesh_p.h>
+#include <Qt3DCore/qaspectengine.h>
 #include <QObject>
 #include <QSignalSpy>
 #include <Qt3DCore/private/qscene_p.h>
@@ -91,8 +92,8 @@ private Q_SLOTS:
         Qt3DRender::QMesh mesh;
         arbiter.setArbiterOnNode(&mesh);
 
-        Qt3DCore::QAspectEngine *engine = reinterpret_cast<Qt3DCore::QAspectEngine*>(0xdeadbeefL);
-        Qt3DCore::QScene *scene = new Qt3DCore::QScene(engine);
+        Qt3DCore::QAspectEngine engine;
+        Qt3DCore::QScene *scene = new Qt3DCore::QScene(&engine);
         Qt3DCore::QNodePrivate *meshd = Qt3DCore::QNodePrivate::get(&mesh);
         meshd->setScene(scene);
         QCoreApplication::processEvents();
@@ -128,8 +129,8 @@ private Q_SLOTS:
         Qt3DRender::QMesh mesh;
         arbiter.setArbiterOnNode(&mesh);
 
-        Qt3DCore::QAspectEngine *engine = reinterpret_cast<Qt3DCore::QAspectEngine*>(0xdeadbeefL);
-        Qt3DCore::QScene *scene = new Qt3DCore::QScene(engine);
+        Qt3DCore::QAspectEngine engine;
+        Qt3DCore::QScene *scene = new Qt3DCore::QScene(&engine);
         Qt3DCore::QNodePrivate *meshd = Qt3DCore::QNodePrivate::get(&mesh);
         meshd->setScene(scene);
         QCoreApplication::processEvents();