QSGDistanceFieldGlyphNode: Remove per-node QLinkedList of nodes to delete

We avoided deleting these nodes directly due to preprocess, but this is
a bit of a hack. QSGRenderer::preprocess already contains some work to
allow for modification of nodes at preprocess time, but it didn't allow
for detecting dead nodes.

By marking a pointer as not to be touched if it is removed during preprocess,
we can remove the per-node QLinkedList and delete directly, while at the same
time, still not touching deleted nodes later on in preprocess.

Change-Id: I99a1ea65d3fe0b73db73e4a1d10d999d56edcdc4
Reviewed-by: Gunnar Sletta <gunnar@crimson.no>

4 files changed, 25 insertions, 13 deletions

diff --git a/src/quick/scenegraph/coreapi/qsgrenderer.cpp b/src/quick/scenegraph/coreapi/qsgrenderer.cpp
index 9207fdbc55..bb2671f6c3 100644
--- a/src/quick/scenegraph/coreapi/qsgrenderer.cpp
+++ b/src/quick/scenegraph/coreapi/qsgrenderer.cpp
@@ -134,6 +134,7 @@ QSGRenderer::QSGRenderer(QSGRenderContext *context)
     , m_bindable(0)
     , m_changed_emitted(false)
     , m_is_rendering(false)
+    , m_is_preprocessing(false)
 {
 }
 
@@ -287,6 +288,8 @@ void QSGRenderer::nodeChanged(QSGNode *node, QSGNode::DirtyState state)
 
 void QSGRenderer::preprocess()
 {
+    m_is_preprocessing = true;
+
     QSGRootNode *root = rootNode();
     Q_ASSERT(root);
 
@@ -298,6 +301,11 @@ void QSGRenderer::preprocess()
     for (QSet<QSGNode *>::const_iterator it = items.constBegin();
          it != items.constEnd(); ++it) {
         QSGNode *n = *it;
+
+        // If we are currently preprocessing, check this node hasn't been
+        // deleted or something. we don't want a use-after-free!
+        if (m_nodes_dont_preprocess.contains(n)) // skip
+            continue;
         if (!nodeUpdater()->isNodeBlocked(n, root)) {
             n->preprocess();
         }
@@ -315,8 +323,13 @@ void QSGRenderer::preprocess()
         updatePassTime = frameTimer.nsecsElapsed();
     Q_QUICK_SG_PROFILE_RECORD(QQuickProfiler::SceneGraphRendererFrame,
                               QQuickProfiler::SceneGraphRendererUpdate);
+
+    m_is_preprocessing = false;
+    m_nodes_dont_preprocess.clear();
 }
 
+
+
 void QSGRenderer::addNodesToPreprocess(QSGNode *node)
 {
     for (QSGNode *c = node->firstChild(); c; c = c->nextSibling())
@@ -329,8 +342,13 @@ void QSGRenderer::removeNodesToPreprocess(QSGNode *node)
 {
     for (QSGNode *c = node->firstChild(); c; c = c->nextSibling())
         removeNodesToPreprocess(c);
-    if (node->flags() & QSGNode::UsePreprocess)
+    if (node->flags() & QSGNode::UsePreprocess) {
         m_nodes_to_preprocess.remove(node);
+
+        // If preprocessing *now*, mark the node as gone.
+        if (m_is_preprocessing)
+            m_nodes_dont_preprocess.insert(node);
+    }
 }
 
 
diff --git a/src/quick/scenegraph/coreapi/qsgrenderer_p.h b/src/quick/scenegraph/coreapi/qsgrenderer_p.h
index 4589685765..1ea2775e6f 100644
--- a/src/quick/scenegraph/coreapi/qsgrenderer_p.h
+++ b/src/quick/scenegraph/coreapi/qsgrenderer_p.h
@@ -118,11 +118,13 @@ private:
     QSGNodeUpdater *m_node_updater;
 
     QSet<QSGNode *> m_nodes_to_preprocess;
+    QSet<QSGNode *> m_nodes_dont_preprocess;
 
     const QSGBindable *m_bindable;
 
     uint m_changed_emitted : 1;
     uint m_is_rendering : 1;
+    uint m_is_preprocessing : 1;
 };
 
 class Q_QUICK_PRIVATE_EXPORT QSGBindable
diff --git a/src/quick/scenegraph/qsgdistancefieldglyphnode.cpp b/src/quick/scenegraph/qsgdistancefieldglyphnode.cpp
index 456a197ba1..11c5444cd2 100644
--- a/src/quick/scenegraph/qsgdistancefieldglyphnode.cpp
+++ b/src/quick/scenegraph/qsgdistancefieldglyphnode.cpp
@@ -76,9 +76,6 @@ QSGDistanceFieldGlyphNode::~QSGDistanceFieldGlyphNode()
         m_glyph_cache->unregisterGlyphNode(this);
         m_glyph_cache->unregisterOwnerElement(ownerElement());
     }
-
-    while (m_nodesToDelete.count())
-        delete m_nodesToDelete.takeLast();
 }
 
 void QSGDistanceFieldGlyphNode::setColor(const QColor &color)
@@ -158,9 +155,6 @@ void QSGDistanceFieldGlyphNode::preprocess()
 {
     Q_ASSERT(m_glyph_cache);
 
-    while (m_nodesToDelete.count())
-        delete m_nodesToDelete.takeLast();
-
     m_glyph_cache->processPendingGlyphs();
     m_glyph_cache->update();
 
@@ -188,13 +182,12 @@ void QSGDistanceFieldGlyphNode::updateGeometry()
     // Remove previously created sub glyph nodes
     // We assume all the children are sub glyph nodes
     QSGNode *subnode = firstChild();
+    QSGNode *nextNode = 0;
     while (subnode) {
-        // We can't delete the node now as it might be in the preprocess list
-        // It will be deleted in the next preprocess
-        m_nodesToDelete.append(subnode);
-        subnode = subnode->nextSibling();
+        nextNode = subnode->nextSibling();
+        delete subnode;
+        subnode = nextNode;
     }
-    removeAllChildNodes();
 
     QSGGeometry *g = geometry();
 
diff --git a/src/quick/scenegraph/qsgdistancefieldglyphnode_p.h b/src/quick/scenegraph/qsgdistancefieldglyphnode_p.h
index c0c6bda718..04446c7b2c 100644
--- a/src/quick/scenegraph/qsgdistancefieldglyphnode_p.h
+++ b/src/quick/scenegraph/qsgdistancefieldglyphnode_p.h
@@ -107,7 +107,6 @@ private:
     AntialiasingMode m_antialiasingMode;
     QRectF m_boundingRect;
     const QSGDistanceFieldGlyphCache::Texture *m_texture;
-    QLinkedList<QSGNode *> m_nodesToDelete;
 
     struct GlyphInfo {
         QVector<quint32> indexes;