Allow the QtQuick + Scene3D case to shutdown cleanly

...by having the Scene3D item exit the simulation loop before asking
the renderer to shutdown. This is necessary because if we let the
simulation loop keep running then the render aspect keeps creating and
executing jobs as we pull the rug out form under it by shutting down
the renderer. This exhibited quite regularly as a crash caused by a
race in the Shader::cleanup() function which tried to access the
GraphicsContext that was destroyed by the renderer shutdown.

This approach sequences things very similarly to the pure Qt 3D shutdown
method. The main difference is that we explicitly stop the simulation
loop whereas in the pure Qt 3D case, that is done for us by the
QAspectEngine::setRootEntity(null) call. This is still done in the
QAspectEngine dtor but by that time, the simulation loop is already
stopped. The exit simulation logic has been adapted to handle this
case.

Task-number: QTBUG-51035
Task-number: QTBUG-42353
Change-Id: I08d16e301eab421fa6eb99e558f38e832534c151
Reviewed-by: Kevin Ottens <kevin.ottens@kdab.com>

4 files changed, 29 insertions, 3 deletions

diff --git a/src/core/aspects/qaspectengine.cpp b/src/core/aspects/qaspectengine.cpp
index 9cacc3adf..b94c53fc0 100644
--- a/src/core/aspects/qaspectengine.cpp
+++ b/src/core/aspects/qaspectengine.cpp
@@ -63,6 +63,11 @@ QT_BEGIN_NAMESPACE
 
 namespace Qt3DCore {
 
+QAspectEnginePrivate *QAspectEnginePrivate::get(QAspectEngine *q)
+{
+    return q->d_func();
+}
+
 QAspectEnginePrivate::QAspectEnginePrivate()
     : QObjectPrivate()
     , m_postman(nullptr)
@@ -208,7 +213,7 @@ void QAspectEnginePrivate::shutdown()
 
     // Exit the simulation loop. Waits for this to be completed on the aspect
     // thread before returning
-    m_aspectThread->aspectManager()->exitSimulationLoop();
+    exitSimulationLoop();
 
     // Cleanup the scene before quitting the backend
     m_scene->setArbiter(nullptr);
@@ -217,6 +222,11 @@ void QAspectEnginePrivate::shutdown()
     m_initialized = false;
 }
 
+void QAspectEnginePrivate::exitSimulationLoop()
+{
+    m_aspectThread->aspectManager()->exitSimulationLoop();
+}
+
 /*!
  * Registers a new \a aspect to the AspectManager. The QAspectEngine takes
  * ownership of the aspect and will delete it when the aspect is unregistered.
diff --git a/src/core/aspects/qaspectengine_p.h b/src/core/aspects/qaspectengine_p.h
index 8ab604f58..c74e6dfbb 100644
--- a/src/core/aspects/qaspectengine_p.h
+++ b/src/core/aspects/qaspectengine_p.h
@@ -67,7 +67,7 @@ class QAspectThread;
 class QPostman;
 class QScene;
 
-class QAspectEnginePrivate : public QObjectPrivate
+class QT3DCORE_PRIVATE_EXPORT QAspectEnginePrivate : public QObjectPrivate
 {
 public:
     QAspectEnginePrivate();
@@ -87,12 +87,16 @@ public:
     void initialize();
     void shutdown();
 
+    void exitSimulationLoop();
+
     void initNodeTree(QNode *node);
     void initNode(QNode *node);
     void initEntity(QEntity *entity);
 
     void generateCreationChanges(QNode *rootNode);
     QVector<QNodeCreatedChangeBasePtr> m_creationChanges;
+
+    static QAspectEnginePrivate *get(QAspectEngine *engine);
 };
 
 } // Qt3D
diff --git a/src/core/aspects/qaspectmanager.cpp b/src/core/aspects/qaspectmanager.cpp
index 993b86e08..7a8c09d00 100644
--- a/src/core/aspects/qaspectmanager.cpp
+++ b/src/core/aspects/qaspectmanager.cpp
@@ -99,7 +99,13 @@ void QAspectManager::enterSimulationLoop()
 void QAspectManager::exitSimulationLoop()
 {
     qCDebug(Aspects) << Q_FUNC_INFO;
-    m_runSimulationLoop.fetchAndStoreOrdered(0);
+
+    // If this fails, simulation loop is already exited so nothing to do
+    if (!m_runSimulationLoop.testAndSetOrdered(1, 0)) {
+        qCDebug(Aspects) << "Simulation loop was not running. Nothing to do";
+        return;
+    }
+
     QAbstractFrameAdvanceService *frameAdvanceService =
             m_serviceLocator->service<QAbstractFrameAdvanceService>(QServiceLocator::FrameAdvanceService);
     if (frameAdvanceService)
diff --git a/src/quick3d/imports/scene3d/scene3drenderer.cpp b/src/quick3d/imports/scene3d/scene3drenderer.cpp
index 24b42e15d..44a57e039 100644
--- a/src/quick3d/imports/scene3d/scene3drenderer.cpp
+++ b/src/quick3d/imports/scene3d/scene3drenderer.cpp
@@ -46,6 +46,7 @@
 #include <Qt3DRender/qrenderaspect.h>
 #include <Qt3DRender/private/qrenderaspect_p.h>
 #include <Qt3DCore/qaspectengine.h>
+#include <Qt3DCore/private/qaspectengine_p.h>
 
 #include <QtQuick/qquickwindow.h>
 
@@ -177,6 +178,11 @@ void Scene3DRenderer::shutdown()
     // would return early
     m_item = nullptr;
 
+    // Exit the simulation loop so no more jobs are asked for. Once this
+    // returns it is safe to shutdown the renderer.
+    auto engineD = Qt3DCore::QAspectEnginePrivate::get(m_aspectEngine);
+    engineD->exitSimulationLoop();
+
     // Shutdown the Renderer Aspect while the OpenGL context
     // is still valid
     if (m_renderAspect)