Allow the backend chance to process any final changes during shutdown

The QAspectEngine now flushes any pending changes batched up before
telling the aspect manager to exit the simulation loop.

Furthermore, the call to QAspectManager::exitSimulationLoop() now
waits until the aspect thread has completely finished the simulation
loop and has called onEngineShutdown() on each of the aspects. This is
important to ensure that the main thread doesn't call
QAspectEngine::shutdown() too early as this deletes the change queue
which contains the final changes sent from the main thread during
shutdown.

For some reason the backend is unable to find which
QBackendNodeMapper corresponds to each destruction change. Will
investigate that next.

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

5 files changed, 28 insertions, 5 deletions

diff --git a/src/core/aspects/qaspectengine.cpp b/src/core/aspects/qaspectengine.cpp
index 08891e349..9cacc3adf 100644
--- a/src/core/aspects/qaspectengine.cpp
+++ b/src/core/aspects/qaspectengine.cpp
@@ -67,6 +67,7 @@ QAspectEnginePrivate::QAspectEnginePrivate()
     : QObjectPrivate()
     , m_postman(nullptr)
     , m_scene(nullptr)
+    , m_initialized(false)
 {
     qRegisterMetaType<Qt3DCore::QAbstractAspect *>();
     qRegisterMetaType<Qt3DCore::QObserverInterface *>();
@@ -186,6 +187,7 @@ void QAspectEnginePrivate::initialize()
     QMetaObject::invokeMethod(arbiter,
                               "setScene",
                               Q_ARG(Qt3DCore::QScene*, m_scene));
+    m_initialized = true;
 }
 
 /*!
@@ -199,10 +201,20 @@ void QAspectEnginePrivate::shutdown()
 {
     qCDebug(Aspects) << Q_FUNC_INFO;
 
+    // Flush any change batch waiting in the postman that may contain node
+    // destruction changes that the aspects should process before we exit
+    // the simulation loop
+    m_postman->submitChangeBatch();
+
+    // Exit the simulation loop. Waits for this to be completed on the aspect
+    // thread before returning
+    m_aspectThread->aspectManager()->exitSimulationLoop();
+
     // Cleanup the scene before quitting the backend
     m_scene->setArbiter(nullptr);
     QChangeArbiter *arbiter = m_aspectThread->aspectManager()->changeArbiter();
     QChangeArbiter::destroyUnmanagedThreadLocalChangeQueue(arbiter);
+    m_initialized = false;
 }
 
 /*!
@@ -350,17 +362,15 @@ void QAspectEngine::setRootEntity(QEntityPtr root)
     if (d->m_root == root)
         return;
 
-    const bool shutdownNeeded = d->m_root;
+    const bool shutdownNeeded = d->m_root && d->m_initialized;
 
     // Set the new root object. This will cause the old tree to be deleted
     // and the deletion of the old frontend tree will cause the backends to
     // free any related resources
     d->m_root = root;
 
-    if (shutdownNeeded) {
+    if (shutdownNeeded)
         d->shutdown();
-        d->m_aspectThread->aspectManager()->exitSimulationLoop();
-    }
 
     // Do we actually have a new scene?
     if (!d->m_root)
diff --git a/src/core/aspects/qaspectengine_p.h b/src/core/aspects/qaspectengine_p.h
index 3639a5f84..8ab604f58 100644
--- a/src/core/aspects/qaspectengine_p.h
+++ b/src/core/aspects/qaspectengine_p.h
@@ -82,6 +82,7 @@ public:
     QSharedPointer<QEntity> m_root;
     QVector<QAbstractAspect*> m_aspects;
     QHash<QString, QAbstractAspect *> m_namedAspects;
+    bool m_initialized;
 
     void initialize();
     void shutdown();
diff --git a/src/core/aspects/qaspectmanager.cpp b/src/core/aspects/qaspectmanager.cpp
index d565848a8..993b86e08 100644
--- a/src/core/aspects/qaspectmanager.cpp
+++ b/src/core/aspects/qaspectmanager.cpp
@@ -73,6 +73,7 @@ QAspectManager::QAspectManager(QObject *parent)
     , m_jobManager(new QAspectJobManager(this))
     , m_changeArbiter(new QChangeArbiter(this))
     , m_serviceLocator(new QServiceLocator())
+    , m_waitForEndOfSimulationLoop(0)
     , m_waitForEndOfExecLoop(0)
     , m_waitForQuit(0)
 {
@@ -115,6 +116,10 @@ void QAspectManager::exitSimulationLoop()
     // QLogicComponent::onFrameUpdate() callback.
     for (QAbstractAspect *aspect : qAsConst(m_aspects))
         aspect->d_func()->onEngineAboutToShutdown();
+
+    // Wait until the simulation loop is fully exited and the aspects are done
+    // processing any final changes and have had onEngineShutdown() called on them
+    m_waitForEndOfSimulationLoop.acquire(1);
 }
 
 bool QAspectManager::isShuttingDown() const
@@ -278,6 +283,9 @@ void QAspectManager::exec()
         } // End of simulation loop
 
         if (needsShutdown) {
+            // Process any pending changes from the frontend before we shut the aspects down
+            m_changeArbiter->syncChanges();
+
             // Give aspects a chance to perform any shutdown actions. This may include unqueuing
             // any blocking work on the main thread that could potentially deadlock during shutdown.
             qCDebug(Aspects) << "Calling onEngineShutdown() for each aspect";
@@ -286,6 +294,9 @@ void QAspectManager::exec()
                 aspect->onEngineShutdown();
             }
             qCDebug(Aspects) << "Done calling onEngineShutdown() for each aspect";
+
+            // Wake up the main thread which is waiting for us inside of exitSimulationLoop()
+            m_waitForEndOfSimulationLoop.release(1);
         }
     } // End of main loop
     qCDebug(Aspects) << Q_FUNC_INFO << "***** Exited main loop *****";
diff --git a/src/core/aspects/qaspectmanager_p.h b/src/core/aspects/qaspectmanager_p.h
index 5be415533..7f941b2bf 100644
--- a/src/core/aspects/qaspectmanager_p.h
+++ b/src/core/aspects/qaspectmanager_p.h
@@ -112,6 +112,7 @@ private:
     QAtomicInt m_runSimulationLoop;
     QAtomicInt m_runMainLoop;
     QScopedPointer<QServiceLocator> m_serviceLocator;
+    QSemaphore m_waitForEndOfSimulationLoop;
     QSemaphore m_waitForEndOfExecLoop;
     QSemaphore m_waitForQuit;
 };
diff --git a/src/core/qpostman_p.h b/src/core/qpostman_p.h
index 4c286e2f3..edfe69c29 100644
--- a/src/core/qpostman_p.h
+++ b/src/core/qpostman_p.h
@@ -81,7 +81,7 @@ public:
     void sceneChangeEvent(const QSceneChangePtr &e) Q_DECL_FINAL;
     void notifyBackend(const QSceneChangePtr &change) Q_DECL_FINAL;
 
-private Q_SLOTS:
+public Q_SLOTS:
     void submitChangeBatch();
 
 private: