Change destruction order to avoid accessing deleted GL resources
authorKent Hansen <kent.hansen@nokia.com>
Wed, 16 Nov 2011 10:24:18 +0000 (11:24 +0100)
committerQt by Nokia <qt-info@nokia.com>
Wed, 16 Nov 2011 12:04:50 +0000 (13:04 +0100)
This commit fixes a crash when a QQuickCanvas containing a
Text item is destroyed.

202127f860208c21145e05685bc54219e1655dbd in qtbase fixed a
resource leak with QOpenGLMultiGroupSharedResource; resources
were not cleaned up correctly when the associated GL context
(group) was destroyed.

This exposed a bug where QSGDefaultDistanceFieldGlyphCache
(used for Text items) relied on that leakage. In particular,
the glyph cache stores a pointer to a GL resource that it
doesn't own (m_textureData).

If the GL context is deleted and there are no other contexts
in the group, the group and its resources will be deleted.
Subsequently, if the glyph cache accesses the resource pointer
when the Text node is destroyed (via
QSGDefaultDistanceFieldGlyphCache::releaseGlyphs()), we crash.

The GL context is deleted when QQuickCanvasPlainRenderLoop is
destroyed. By moving the deletion of the render loop object to
the end of QQuickCanvas destructor, the canvas nodes get a
chance to clean up the resources they use _before_ the
resources are deleted.

Task-number: QTBUG-22754
Change-Id: Ie8de19a139b4631a16203e63e731feed3d8d64cf
Reviewed-by: Aaron Kennedy <aaron.kennedy@nokia.com>
Reviewed-by: Samuel Rødal <samuel.rodal@nokia.com>
src/declarative/items/qquickcanvas.cpp

index b433e02..383b552 100644 (file)
@@ -863,11 +863,8 @@ QQuickCanvas::~QQuickCanvas()
        updatePaintNode(), so disconnect them before starting the shutdown
      */
     disconnect(d->context->renderer(), SIGNAL(sceneGraphChanged()), this, SLOT(maybeUpdate()));
-    if (d->thread->isRunning()) {
+    if (d->thread->isRunning())
         d->thread->stopRendering();
-        delete d->thread;
-        d->thread = 0;
-    }
 
     // ### should we change ~QQuickItem to handle this better?
     // manually cleanup for the root item (item destructor only handles these when an item is parented)
@@ -878,6 +875,8 @@ QQuickCanvas::~QQuickCanvas()
 
     delete d->rootItem; d->rootItem = 0;
     d->cleanupNodes();
+
+    delete d->thread; d->thread = 0;
 }
 
 /*!