From ee958e086847e2131f9b746e5c6de4491ae1f44b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Samuel=20R=C3=B8dal?= <samuel.rodal@nokia.com>
Date: Mon, 8 Nov 2010 10:05:28 +0100
Subject: Prevented race condition on texture destruction.

When texture destruction was triggered from a different thread, we
posted a signal to the QGLSignalProxy. However, before the signal is
delivered the corresponding QGLContext might be destroyed in the main
thread. We need to post a signal to a QObject which is destroyed when
the QGLContext is destroyed instead, to prevent trying to access an
invalid QGLContext pointer.

Task-number: QT-4238
Reviewed-by: Gunnar Sletta
---
 src/opengl/qgl.cpp         | 11 +++++++++++
 src/opengl/qgl.h           |  1 +
 src/opengl/qgl_p.h         | 30 ++++++++++++++++++++----------
 tests/auto/qgl/tst_qgl.cpp |  8 ++++++--
 4 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/src/opengl/qgl.cpp b/src/opengl/qgl.cpp
index 84cfa97..ed9753e 100644
--- a/src/opengl/qgl.cpp
+++ b/src/opengl/qgl.cpp
@@ -1642,12 +1642,23 @@ const QGLContext *qt_gl_transfer_context(const QGLContext *ctx)
         return 0;
 }
 
+QGLContextPrivate::QGLContextPrivate(QGLContext *context)
+    : internal_context(false)
+    , q_ptr(context)
+{
+    group = new QGLContextGroup(context);
+    texture_destroyer = new QGLTextureDestroyer;
+    texture_destroyer->moveToThread(qApp->thread());
+}
+
 QGLContextPrivate::~QGLContextPrivate()
 {
     if (!group->m_refs.deref()) {
         Q_ASSERT(group->context() == q_ptr);
         delete group;
     }
+
+    delete texture_destroyer;
 }
 
 void QGLContextPrivate::init(QPaintDevice *dev, const QGLFormat &format)
diff --git a/src/opengl/qgl.h b/src/opengl/qgl.h
index f85cad5..9ae619d 100644
--- a/src/opengl/qgl.h
+++ b/src/opengl/qgl.h
@@ -428,6 +428,7 @@ private:
     friend class QGLSharedResourceGuard;
     friend class QGLPixmapBlurFilter;
     friend class QGLExtensions;
+    friend class QGLTexture;
     friend QGLFormat::OpenGLVersionFlags QGLFormat::openGLVersionFlags();
 #ifdef Q_WS_MAC
 public:
diff --git a/src/opengl/qgl_p.h b/src/opengl/qgl_p.h
index 18b2765..4742bdb 100644
--- a/src/opengl/qgl_p.h
+++ b/src/opengl/qgl_p.h
@@ -317,6 +317,7 @@ private:
 };
 
 class QGLTexture;
+class QGLTextureDestroyer;
 
 // This probably needs to grow to GL_MAX_VERTEX_ATTRIBS, but 3 is ok for now as that's
 // all the GL2 engine uses:
@@ -326,7 +327,7 @@ class QGLContextPrivate
 {
     Q_DECLARE_PUBLIC(QGLContext)
 public:
-    explicit QGLContextPrivate(QGLContext *context) : internal_context(false), q_ptr(context) {group = new QGLContextGroup(context);}
+    explicit QGLContextPrivate(QGLContext *context);
     ~QGLContextPrivate();
     QGLTexture *bindTexture(const QImage &image, GLenum target, GLint format,
                             QGLContext::BindOptions options);
@@ -417,6 +418,7 @@ public:
     GLuint current_fbo;
     GLuint default_fbo;
     QPaintEngine *active_engine;
+    QGLTextureDestroyer *texture_destroyer;
 
     bool vertexAttributeArraysEnabledState[QT_GL_VERTEX_ARRAY_TRACKED_COUNT];
 
@@ -476,25 +478,20 @@ private:
     QGLContext *m_ctx;
 };
 
-// ### make QGLContext a QObject in 5.0 and remove the proxy stuff
-class Q_OPENGL_EXPORT QGLSignalProxy : public QObject
+class QGLTextureDestroyer : public QObject
 {
     Q_OBJECT
 public:
-    QGLSignalProxy() : QObject() {
+    QGLTextureDestroyer() : QObject() {
         qRegisterMetaType<GLuint>("GLuint");
         connect(this, SIGNAL(freeTexture(QGLContext *, QPixmapData *, GLuint)),
                 this, SLOT(freeTexture_slot(QGLContext *, QPixmapData *, GLuint)));
     }
-    void emitAboutToDestroyContext(const QGLContext *context) {
-        emit aboutToDestroyContext(context);
-    }
     void emitFreeTexture(QGLContext *context, QPixmapData *boundPixmap, GLuint id) {
         emit freeTexture(context, boundPixmap, id);
     }
-    static QGLSignalProxy *instance();
+
 Q_SIGNALS:
-    void aboutToDestroyContext(const QGLContext *context);
     void freeTexture(QGLContext *context, QPixmapData *boundPixmap, GLuint id);
 
 private slots:
@@ -518,6 +515,19 @@ private slots:
     }
 };
 
+// ### make QGLContext a QObject in 5.0 and remove the proxy stuff
+class Q_OPENGL_EXPORT QGLSignalProxy : public QObject
+{
+    Q_OBJECT
+public:
+    void emitAboutToDestroyContext(const QGLContext *context) {
+        emit aboutToDestroyContext(context);
+    }
+    static QGLSignalProxy *instance();
+Q_SIGNALS:
+    void aboutToDestroyContext(const QGLContext *context);
+};
+
 class QGLTexture {
 public:
     QGLTexture(QGLContext *ctx = 0, GLuint tx_id = 0, GLenum tx_target = GL_TEXTURE_2D,
@@ -537,7 +547,7 @@ public:
 #if !defined(Q_WS_X11)
             QPixmapData *boundPixmap = 0;
 #endif
-            QGLSignalProxy::instance()->emitFreeTexture(context, boundPixmap, id);
+            context->d_ptr->texture_destroyer->emitFreeTexture(context, boundPixmap, id);
         }
      }
 
diff --git a/tests/auto/qgl/tst_qgl.cpp b/tests/auto/qgl/tst_qgl.cpp
index 4220b45..e38bf42 100644
--- a/tests/auto/qgl/tst_qgl.cpp
+++ b/tests/auto/qgl/tst_qgl.cpp
@@ -2335,8 +2335,10 @@ public:
     Widget()
         : iterations(0)
         , display(0)
+        , producer(new Producer)
     {
         startTimer(400);
+        connect(this, SIGNAL(destroyed()), producer, SLOT(deleteLater()));
     }
 
     int iterations;
@@ -2348,15 +2350,15 @@ protected:
 
         delete display;
         display = new DisplayWidget(this);
-        connect(&producer, SIGNAL(imageReady(const QImage &)), display, SLOT(setImage(const QImage &)));
+        connect(producer, SIGNAL(imageReady(const QImage &)), display, SLOT(setImage(const QImage &)));
 
         display->setGeometry(rect());
         display->show();
     }
 
 private:
-    Producer producer;
     DisplayWidget *display;
+    Producer *producer;
 };
 
 }
@@ -2369,6 +2371,8 @@ void tst_QGL::threadImages()
     while (widget->iterations <= 5) {
         qApp->processEvents();
     }
+
+    delete widget;
 }
 
 class tst_QGLDummy : public QObject
-- 
cgit v0.12