From 202127f860208c21145e05685bc54219e1655dbd Mon Sep 17 00:00:00 2001
From: Kent Hansen <kent.hansen@nokia.com>
Date: Tue, 15 Nov 2011 16:39:16 +0100
Subject: [PATCH] Ensure that QOpenGLMultiGroupSharedResources are cleaned up
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When a GL context group is destroyed, all multi-group shared
resources associated with the group should be cleaned up.
Otherwise we could get a double deletion in the resource's
destructor, because it still retained a pointer to the
deleted group.

The missing cleanup resulted in a crash when the global
static qt_gl_functions_resource was destroyed, first seen
in the tst_examples autotest in qtdeclarative. It possibly
didn't manifest before because it's event loop-dependent
(the contexts are deleted via deleteLater()).

Change-Id: I6b1e0bfdfbbb2bff8e795f545e680fcdfa094768
Reviewed-by: Samuel Rødal <samuel.rodal@nokia.com>
---
 src/gui/kernel/qopenglcontext.cpp      | 29 +++++++++++---------------
 src/gui/kernel/qopenglcontext_p.h      |  7 +------
 tests/auto/gui/qopengl/tst_qopengl.cpp | 23 ++++++++++++++++++++
 3 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/src/gui/kernel/qopenglcontext.cpp b/src/gui/kernel/qopenglcontext.cpp
index 2b2f13e4bd..88925df606 100644
--- a/src/gui/kernel/qopenglcontext.cpp
+++ b/src/gui/kernel/qopenglcontext.cpp
@@ -404,6 +404,15 @@ void QOpenGLContextGroupPrivate::removeContext(QOpenGLContext *ctx)
 
 void QOpenGLContextGroupPrivate::cleanup()
 {
+    Q_Q(QOpenGLContextGroup);
+    {
+        QHash<QOpenGLMultiGroupSharedResource *, QOpenGLSharedResource *>::const_iterator it, end;
+        end = m_resources.constEnd();
+        for (it = m_resources.constBegin(); it != end; ++it)
+            it.key()->cleanup(q, it.value());
+        m_resources.clear();
+    }
+
     QList<QOpenGLSharedResource *>::iterator it = m_sharedResources.begin();
     QList<QOpenGLSharedResource *>::iterator end = m_sharedResources.end();
 
@@ -581,29 +590,15 @@ QList<QOpenGLSharedResource *> QOpenGLMultiGroupSharedResource::resources() cons
     return result;
 }
 
-void QOpenGLMultiGroupSharedResource::cleanup(QOpenGLContext *ctx)
-{
-    QOpenGLSharedResource *resource = value(ctx);
-
-    if (resource != 0) {
-        resource->free();
-
-        QOpenGLContextGroup *group = ctx->shareGroup();
-        group->d_func()->m_resources.remove(this);
-        m_groups.removeOne(group);
-        active.deref();
-    }
-}
-
-void QOpenGLMultiGroupSharedResource::cleanup(QOpenGLContext *ctx, QOpenGLSharedResource *value)
+void QOpenGLMultiGroupSharedResource::cleanup(QOpenGLContextGroup *group, QOpenGLSharedResource *value)
 {
 #ifdef QT_GL_CONTEXT_RESOURCE_DEBUG
-    qDebug("Cleaning up context group resource %p, for context %p in thread %p.", this, ctx, QThread::currentThread());
+    qDebug("Cleaning up context group resource %p, for group %p in thread %p.", this, group, QThread::currentThread());
 #endif
     value->free();
     active.deref();
 
-    QOpenGLContextGroup *group = ctx->shareGroup();
+    Q_ASSERT(m_groups.contains(group));
     m_groups.removeOne(group);
 }
 
diff --git a/src/gui/kernel/qopenglcontext_p.h b/src/gui/kernel/qopenglcontext_p.h
index 059872c09f..1a47d293e5 100644
--- a/src/gui/kernel/qopenglcontext_p.h
+++ b/src/gui/kernel/qopenglcontext_p.h
@@ -138,8 +138,6 @@ public:
 
     QList<QOpenGLSharedResource *> m_sharedResources;
     QList<QOpenGLSharedResource *> m_pendingDeletion;
-
-    void cleanupResources(QOpenGLContext *ctx);
 };
 
 class Q_GUI_EXPORT QOpenGLMultiGroupSharedResource
@@ -149,8 +147,7 @@ public:
     ~QOpenGLMultiGroupSharedResource();
 
     void insert(QOpenGLContext *context, QOpenGLSharedResource *value);
-    void cleanup(QOpenGLContext *context);
-    void cleanup(QOpenGLContext *context, QOpenGLSharedResource *value);
+    void cleanup(QOpenGLContextGroup *group, QOpenGLSharedResource *value);
 
     QOpenGLSharedResource *value(QOpenGLContext *context);
 
@@ -220,8 +217,6 @@ public:
 
     QPaintEngineEx *active_engine;
 
-    QHash<QOpenGLMultiGroupSharedResource *, void *> m_resources;
-
     static void setCurrentContext(QOpenGLContext *context);
 
     int maxTextureSize() const { return 1024; }
diff --git a/tests/auto/gui/qopengl/tst_qopengl.cpp b/tests/auto/gui/qopengl/tst_qopengl.cpp
index 479c1c8e2e..a3e0cf4fba 100644
--- a/tests/auto/gui/qopengl/tst_qopengl.cpp
+++ b/tests/auto/gui/qopengl/tst_qopengl.cpp
@@ -42,6 +42,7 @@
 
 #include <QtGui/private/qopenglcontext_p.h>
 #include <QtGui/QOpenGLFramebufferObject>
+#include <QtGui/QOpenGLFunctions>
 #include <QtGui/QOpenGLPaintDevice>
 #include <QtGui/QPainter>
 #include <QtGui/QScreen>
@@ -55,6 +56,7 @@ Q_OBJECT
 
 private slots:
     void sharedResourceCleanup();
+    void multiGroupSharedResourceCleanup();
     void fboSimpleRendering();
     void fboRendering();
     void fboHandleNulledAfterContextDestroyed();
@@ -172,6 +174,27 @@ void tst_QOpenGL::sharedResourceCleanup()
     QCOMPARE(tracker.destructorCalls, 1);
 }
 
+void tst_QOpenGL::multiGroupSharedResourceCleanup()
+{
+    QWindow window;
+    window.setGeometry(0, 0, 10, 10);
+    window.create();
+
+    for (int i = 0; i < 10; ++i) {
+        QOpenGLContext *gl = new QOpenGLContext();
+        gl->create();
+        gl->makeCurrent(&window);
+        {
+            // Cause QOpenGLMultiGroupSharedResource instantiation.
+            QOpenGLFunctions func(gl);
+        }
+        delete gl;
+        // Cause context group's deleteLater() to be processed.
+        QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
+    }
+    // Shouldn't crash when application exits.
+}
+
 static bool fuzzyComparePixels(const QRgb testPixel, const QRgb refPixel, const char* file, int line, int x = -1, int y = -1)
 {
     static int maxFuzz = 1;