Fix potential race condition

The node lives in the render thread, while the item lives in the
GUI thread. They are only synchronized during updatePaintNode().
This patch fixes two race conditons: 1. the texture might be deleted
while the render thread is using it; 2. the item might be deleted before
WaylandSurfaceNode::preprocess() accesses it.

Change-Id: I19bcba3af8bca68c8b9bf42ae8c15537ce7cebcf
Reviewed-by: Gunnar Sletta <gunnar.sletta@nokia.com>

diff --git a/src/compositor/compositor_api/waylandsurfaceitem.cpp b/src/compositor/compositor_api/waylandsurfaceitem.cpp
index 4883811..c0fcc92 100644 (file)
--- a/src/compositor/compositor_api/waylandsurfaceitem.cpp
+++ b/src/compositor/compositor_api/waylandsurfaceitem.cpp
@@ -54,37 +54,76 @@
 #include <QtQuick/QSGSimpleRectNode>
 #include <QtQuick/QQuickCanvas>
 
+#include <QtCore/QMutexLocker>
+#include <QtCore/QMutex>
+
+class WaylandSurfaceNode : public QSGSimpleTextureNode
+{
+public:
+    WaylandSurfaceNode(WaylandSurfaceItem *item) : m_item(item), smooth(true), t(0) {
+        if (m_item)
+            m_item->m_node = this;
+        setFlag(UsePreprocess,true);
+    }
+    ~WaylandSurfaceNode() {
+        QMutexLocker locker(WaylandSurfaceItem::mutex);
+        delete t;
+        if (m_item)
+            m_item->m_node = 0;
+    }
+    void preprocess() {
+        QMutexLocker locker(WaylandSurfaceItem::mutex);
+        if (m_item && m_item->m_damaged) {
+            QSGTexture *newTexture = m_item->updateTexture(t);
+            updateTexture(newTexture);
+        }
+    }
+
+    void updateTexture(QSGTexture *texture) {
+        t = texture;
+        setTexture(texture);
+        setFiltering(smooth ? QSGTexture::Linear : QSGTexture::Nearest);
+    }
+
+    WaylandSurfaceItem *m_item;
+    bool smooth;
+    QSGTexture *t;
+};
+
 class WaylandSurfaceTextureProvider : public QSGTextureProvider
 {
 public:
-    WaylandSurfaceTextureProvider() : t(0) { }
+    WaylandSurfaceTextureProvider() : node(0) { }
 
     QSGTexture *texture() const {
-        if (t)
-            t->setFiltering(smooth ? QSGTexture::Linear : QSGTexture::Nearest);
-        return t;
+        if (node && node->t) {
+            node->t->setFiltering(node->smooth ? QSGTexture::Linear : QSGTexture::Nearest);
+            return node->t;
+        }
+        return 0;
     }
 
-    QSGTexture *t;
-    bool smooth;
+    WaylandSurfaceNode *node;
 };
 
+QMutex *WaylandSurfaceItem::mutex = 0;
+
 WaylandSurfaceItem::WaylandSurfaceItem(QQuickItem *parent)
     : QQuickItem(parent)
     , m_surface(0)
-    , m_texture(0)
     , m_provider(0)
     , m_paintEnabled(true)
     , m_useTextureAlpha(false)
     , m_clientRenderingEnabled(false)
     , m_touchEventsEnabled(false)
 {
+    if (!mutex)
+        mutex = new QMutex;
 }
 
 WaylandSurfaceItem::WaylandSurfaceItem(WaylandSurface *surface, QQuickItem *parent)
     : QQuickItem(parent)
     , m_surface(0)
-    , m_texture(0)
     , m_provider(0)
     , m_paintEnabled(true)
     , m_useTextureAlpha(false)
@@ -131,10 +170,12 @@ void WaylandSurfaceItem::init(WaylandSurface *surface)
 
 WaylandSurfaceItem::~WaylandSurfaceItem()
 {
+    QMutexLocker locker(mutex);
+    if (m_node)
+        m_node->m_item = 0;
     if (m_surface) {
         m_surface->setSurfaceItem(0);
     }
-    m_texture->deleteLater();
 }
 
 void WaylandSurfaceItem::setSurface(WaylandSurface *surface)
@@ -304,53 +345,30 @@ void WaylandSurfaceItem::setPaintEnabled(bool enabled)
     update();
 }
 
-class WaylandSurfaceNode : public QSGSimpleTextureNode
-{
-public:
-    WaylandSurfaceNode(WaylandSurfaceItem *item) : m_item(item) {
-        setFlag(UsePreprocess,true);
-    }
-    void preprocess() {
-        if (m_item->m_damaged) {
-            m_item->updateTexture();
-            m_item->updateNodeTexture(this);
-        }
-    }
-private:
-    WaylandSurfaceItem *m_item;
-};
-
 
-void WaylandSurfaceItem::updateTexture()
+QSGTexture *WaylandSurfaceItem::updateTexture(QSGTexture *oldTexture)
 {
+    QSGTexture *newTexture;
     if (m_damaged) {
         m_damaged = false;
-        QSGTexture *oldTexture = m_texture;
         if (m_surface->type() == WaylandSurface::Texture) {
             QOpenGLContext *context = QOpenGLContext::currentContext();
 
             QQuickCanvas::CreateTextureOptions opt = useTextureAlpha() ? QQuickCanvas::TextureHasAlphaChannel : QQuickCanvas::CreateTextureOptions(0);
-            m_texture = canvas()->createTextureFromId(m_surface->texture(context),
+            newTexture = canvas()->createTextureFromId(m_surface->texture(context),
                                                                           m_surface->size(),
                                                                           opt);
         } else {
-            m_texture = canvas()->createTextureFromImage(m_surface->image());
+            newTexture = canvas()->createTextureFromImage(m_surface->image());
         }
 
         delete oldTexture;
+    } else {
+        newTexture = oldTexture;
     }
-
-    if (m_provider) {
-        m_provider->t = m_texture;
-        m_provider->smooth = smooth();
-    }
+    return newTexture;
 }
 
-void WaylandSurfaceItem::updateNodeTexture(WaylandSurfaceNode *node)
-{
-    node->setTexture(m_texture);
-    node->setFiltering(smooth() ? QSGTexture::Linear : QSGTexture::Nearest);
-}
 
 QSGNode *WaylandSurfaceItem::updatePaintNode(QSGNode *oldNode, UpdatePaintNodeData *)
 {
@@ -359,19 +377,23 @@ QSGNode *WaylandSurfaceItem::updatePaintNode(QSGNode *oldNode, UpdatePaintNodeDa
         return 0;
     }
 
-    updateTexture();
-    if (!m_texture || !m_paintEnabled) {
+    WaylandSurfaceNode *node = static_cast<WaylandSurfaceNode *>(oldNode);
+
+    QSGTexture *oldTexture = node ? node->t : 0;
+    QSGTexture *newTexture = updateTexture(oldTexture);
+
+    if (!newTexture || !m_paintEnabled) {
         delete oldNode;
+        if (m_provider)
+            m_provider->node = 0;
         return 0;
     }
 
-    WaylandSurfaceNode *node = static_cast<WaylandSurfaceNode *>(oldNode);
-
     if (!node) {
         node = new WaylandSurfaceNode(this);
     }
 
-    updateNodeTexture(node);
+    node->updateTexture(newTexture);
 
     if (surface()->isYInverted()) {
         node->setRect(0, height(), width(), -height());
@@ -379,6 +401,10 @@ QSGNode *WaylandSurfaceItem::updatePaintNode(QSGNode *oldNode, UpdatePaintNodeDa
         node->setRect(0, 0, width(), height());
     }
 
+    node->smooth = smooth();
+    if (m_provider)
+        m_provider->node = node;
+
     return node;
 }
 

diff --git a/src/compositor/compositor_api/waylandsurfaceitem.h b/src/compositor/compositor_api/waylandsurfaceitem.h
index da07b00..34054e0 100644 (file)
--- a/src/compositor/compositor_api/waylandsurfaceitem.h
+++ b/src/compositor/compositor_api/waylandsurfaceitem.h
@@ -51,6 +51,7 @@
 
 class WaylandSurfaceTextureProvider;
 class WaylandSurfaceNode;
+class QMutex;
 
 Q_DECLARE_METATYPE(WaylandSurface*)
 
@@ -124,14 +125,15 @@ protected:
 
 private:
     friend class WaylandSurfaceNode;
-    void updateTexture();
-    void updateNodeTexture(WaylandSurfaceNode *newNode);
+    QSGTexture *updateTexture(QSGTexture *oldTexture);
     QPoint toSurface(const QPointF &pos) const;
     void init(WaylandSurface *);
 
+    static QMutex *mutex;
+
     WaylandSurface *m_surface;
-    QSGTexture *m_texture;
     mutable WaylandSurfaceTextureProvider *m_provider;
+    WaylandSurfaceNode *m_node;
     bool m_paintEnabled;
     bool m_useTextureAlpha;
     bool m_clientRenderingEnabled;