Shared node bookkeeping

Any time a property references a QNode there is a risk that the node gets
destroyed and then the property is left pointing to a dangling pointer.

To handle such cases, setters of such properties are able to use a helper
that internally connect QObject::destroyed signal to a setter removal method.

Change-Id: I42428c851d0e3d2d88ab0cf6a5b75605334ec648
Task-number: QTBUG-53456
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

1 files changed, 25 insertions, 8 deletions

diff --git a/src/render/materialsystem/qrenderpass.cpp b/src/render/materialsystem/qrenderpass.cpp
index 69098f9ec..9ec7a452b 100644
--- a/src/render/materialsystem/qrenderpass.cpp
+++ b/src/render/materialsystem/qrenderpass.cpp
@@ -92,21 +92,23 @@ void QRenderPass::setShaderProgram(QShaderProgram *shaderProgram)
             d->notifyObservers(change);
         }
 
-        d->m_shader = shaderProgram;
-        emit shaderProgramChanged(shaderProgram);
+        if (d->m_shader)
+            d->unregisterDestructionHelper(d->m_shader);
 
         // We need to add it as a child of the current node if it has been declared inline
         // Or not previously added as a child of the current node so that
         // 1) The backend gets notified about it's creation
         // 2) When the current node is destroyed, it gets destroyed as well
-        if (!shaderProgram->parent())
+        if (shaderProgram && !shaderProgram->parent())
             shaderProgram->setParent(this);
 
-        if (d->m_shader && d->m_changeArbiter != nullptr) {
-            const auto change = QPropertyNodeAddedChangePtr::create(id(), d->m_shader);
-            change->setPropertyName("shaderProgram");
-            d->notifyObservers(change);
-        }
+        d->m_shader = shaderProgram;
+
+        // Ensures proper bookkeeping
+        if (d->m_shader)
+            d->registerDestructionHelper(d->m_shader, &QRenderPass::setShaderProgram, d->m_shader);
+
+        emit shaderProgramChanged(shaderProgram);
     }
 }
 
@@ -123,6 +125,9 @@ void QRenderPass::addFilterKey(QFilterKey *filterKey)
     if (!d->m_filterKeyList.contains(filterKey)) {
         d->m_filterKeyList.append(filterKey);
 
+        // Ensures proper bookkeeping
+        d->registerDestructionHelper(filterKey, &QRenderPass::removeFilterKey, d->m_filterKeyList);
+
         // We need to add it as a child of the current node if it has been declared inline
         // Or not previously added as a child of the current node so that
         // 1) The backend gets notified about it's creation
@@ -148,6 +153,8 @@ void QRenderPass::removeFilterKey(QFilterKey *filterKey)
         d->notifyObservers(change);
     }
     d->m_filterKeyList.removeOne(filterKey);
+    // Remove bookkeeping connection
+    d->unregisterDestructionHelper(filterKey);
 }
 
 QVector<QFilterKey *> QRenderPass::filterKeys() const
@@ -171,6 +178,9 @@ void QRenderPass::addRenderState(QRenderState *state)
     if (!d->m_renderStates.contains(state)) {
         d->m_renderStates.append(state);
 
+        // Ensures proper bookkeeping
+        d->registerDestructionHelper(state, &QRenderPass::removeRenderState, d->m_renderStates);
+
         if (!state->parent())
             state->setParent(this);
 
@@ -195,6 +205,8 @@ void QRenderPass::removeRenderState(QRenderState *state)
         d->notifyObservers(change);
     }
     d->m_renderStates.removeOne(state);
+    // Remove bookkeeping connection
+    d->unregisterDestructionHelper(state);
 }
 
 /*!
@@ -214,6 +226,9 @@ void QRenderPass::addParameter(QParameter *parameter)
     if (!d->m_parameters.contains(parameter)) {
         d->m_parameters.append(parameter);
 
+        // Ensures proper bookkeeping
+        d->registerDestructionHelper(parameter, &QRenderPass::removeParameter, d->m_parameters);
+
         // We need to add it as a child of the current node if it has been declared inline
         // Or not previously added as a child of the current node so that
         // 1) The backend gets notified about it's creation
@@ -239,6 +254,8 @@ void QRenderPass::removeParameter(QParameter *parameter)
         d->notifyObservers(change);
     }
     d->m_parameters.removeOne(parameter);
+    // Remove bookkeeping connection
+    d->unregisterDestructionHelper(parameter);
 }