Avoid corruption in Q(Open)GLFunctions when used on multiple threads

Using Q(Open)GLFunctions concurrently on multiple threads had some issues
due to incorrect synchronization: The m_groups list in the
QOpenGLMultiGroupSharedResource, in which the Q(Open)GLFunctions instance
is stored, became corrupted under certain scenarios, for example in the
tst_qglthreads autotest and any two threads that happen to enter an
initializeOpenGLFunctions() or QOpenGLContext::functions() call concurrently.

Locking in value() has been introduced in 666c25c089acf7fcc9e9a6b7665074c6286d604e
to fix such issues, however using the context group's mutex is not enough: that still
allows two threads using two contexts with a different context group to concurrently
enter insert(). Instead, the MultiGroupSharedResource has to have its own mutex
to protect its own member variables.

Task-number: QTBUG-38771
Change-Id: If01c44c2084b95e487bc9146576ca180ed8044da
Reviewed-by: Jørgen Lind <jorgen.lind@digia.com>

2 files changed, 6 insertions, 2 deletions

diff --git a/src/gui/kernel/qopenglcontext.cpp b/src/gui/kernel/qopenglcontext.cpp
index 1af5c09fd2..be592153d2 100644
--- a/src/gui/kernel/qopenglcontext.cpp
+++ b/src/gui/kernel/qopenglcontext.cpp
@@ -1345,7 +1345,8 @@ void QOpenGLSharedResourceGuard::freeResource(QOpenGLContext *context)
     QOpenGLMultiGroupSharedResource instance.
 */
 QOpenGLMultiGroupSharedResource::QOpenGLMultiGroupSharedResource()
-    : active(0)
+    : active(0),
+      m_mutex(QMutex::Recursive)
 {
 #ifdef QT_GL_CONTEXT_RESOURCE_DEBUG
     qDebug("Creating context group resource object %p.", this);
diff --git a/src/gui/kernel/qopenglcontext_p.h b/src/gui/kernel/qopenglcontext_p.h
index aa1b3b6245..711a3b1b2f 100644
--- a/src/gui/kernel/qopenglcontext_p.h
+++ b/src/gui/kernel/qopenglcontext_p.h
@@ -171,7 +171,9 @@ public:
     template <typename T>
     T *value(QOpenGLContext *context) {
         QOpenGLContextGroup *group = context->shareGroup();
-        QMutexLocker locker(&group->d_func()->m_mutex);
+        // Have to use our own mutex here, not the group's, since
+        // m_groups has to be protected too against any concurrent access.
+        QMutexLocker locker(&m_mutex);
         T *resource = static_cast<T *>(group->d_func()->m_resources.value(this, 0));
         if (!resource) {
             resource = new T(context);
@@ -183,6 +185,7 @@ public:
 private:
     QAtomicInt active;
     QList<QOpenGLContextGroup *> m_groups;
+    QMutex m_mutex;
 };
 
 class QPaintEngineEx;