Skip to content

Commit

Permalink
Change destruction order to avoid accessing deleted GL resources
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Kent Hansen authored and Qt by Nokia committed Nov 16, 2011
1 parent cb26606 commit ddf9883
Showing 1 changed file with 3 additions and 4 deletions.
7 changes: 3 additions & 4 deletions src/declarative/items/qquickcanvas.cpp
Expand Up @@ -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)
Expand All @@ -878,6 +875,8 @@ QQuickCanvas::~QQuickCanvas()

delete d->rootItem; d->rootItem = 0;
d->cleanupNodes();

delete d->thread; d->thread = 0;
}

/*!
Expand Down

0 comments on commit ddf9883

Please sign in to comment.