Fix crash with AnimatedImage caused by race condition.
authorGunnar Sletta <gunnar.sletta@nokia.com>
Mon, 5 Mar 2012 08:44:46 +0000 (09:44 +0100)
committerQt by Nokia <qt-info@nokia.com>
Mon, 5 Mar 2012 09:06:08 +0000 (10:06 +0100)
In QDeclarativePixmap::setImage() we deleted a QDeclPixmapData
and recreated a new one in a very short timespan and the new
texture factory was the same pointer as the deleted one, yet a
queued destroyed signal was still emitted. Depending on when the
queued connection was handled in the rendering thread, this would
cause problems with the value returned from textureForFactory.

Change-Id: Ibd785ca12667c99efb88b92689ae7ac4fa87c7ee
Reviewed-by: Kim M. Kalland <kim.kalland@nokia.com>
src/quick/scenegraph/qsgcontext.cpp

index 10276c1..6eae5c3 100644 (file)
@@ -110,6 +110,7 @@ public:
     QOpenGLContext *gl;
 
     QHash<QSGMaterialType *, QSGMaterialShader *> materials;
+    QMutex textureMutex;
     QHash<QQuickTextureFactory *, QSGTexture *> textures;
 
     QSGDistanceFieldGlyphCacheManager *distanceFieldCacheManager;
@@ -119,6 +120,13 @@ public:
     bool distanceFieldDisabled;
 };
 
+class QSGTextureCleanupEvent : public QEvent
+{
+public:
+    QSGTextureCleanupEvent(QSGTexture *t) : QEvent(QEvent::User), texture(t) { }
+    ~QSGTextureCleanupEvent() { delete texture; }
+    QSGTexture *texture;
+};
 
 /*!
     \class QSGContext
@@ -147,8 +155,10 @@ QSGContext::~QSGContext()
 void QSGContext::invalidate()
 {
     Q_D(QSGContext);
+    d->textureMutex.lock();
     qDeleteAll(d->textures.values());
     d->textures.clear();
+    d->textureMutex.unlock();
     qDeleteAll(d->materials.values());
     d->materials.clear();
     delete d->distanceFieldCacheManager;
@@ -166,6 +176,7 @@ QSGTexture *QSGContext::textureForFactory(QQuickTextureFactory *factory, QQuickC
     if (!factory)
         return 0;
 
+    d->textureMutex.lock();
     QSGTexture *texture = d->textures.value(factory);
     if (!texture) {
         if (QQuickDefaultTextureFactory *dtf = qobject_cast<QQuickDefaultTextureFactory *>(factory))
@@ -173,8 +184,9 @@ QSGTexture *QSGContext::textureForFactory(QQuickTextureFactory *factory, QQuickC
         else
             texture = factory->createTexture(canvas);
         d->textures.insert(factory, texture);
-        connect(factory, SIGNAL(destroyed(QObject *)), this, SLOT(textureFactoryDestroyed(QObject *)));
+        connect(factory, SIGNAL(destroyed(QObject *)), this, SLOT(textureFactoryDestroyed(QObject *)), Qt::DirectConnection);
     }
+    d->textureMutex.unlock();
     return texture;
 }
 
@@ -184,9 +196,16 @@ void QSGContext::textureFactoryDestroyed(QObject *o)
     Q_D(QSGContext);
     QQuickTextureFactory *f = static_cast<QQuickTextureFactory *>(o);
 
-    // This function will only be called on the scene graph thread, so it is
-    // safe to directly delete the texture here.
-    delete d->textures.take(f);
+    d->textureMutex.lock();
+    QSGTexture *t = d->textures.take(f);
+    d->textureMutex.unlock();
+
+    if (t) {
+        if (t->thread() == thread())
+            t->deleteLater();
+        else
+            QCoreApplication::postEvent(this, new QSGTextureCleanupEvent(t));
+    }
 }