Fix race condition for client buffer integration initialization

This race happened because QWaylandIntegration::clientBufferIntegration (which
lazily initializes the integration) was called from numerous places including
code that may run on different threads without any kind of syncrhonization.

An example of this is Qt3D, which indirectly and simultaneously calls
createPlatformWindow (from the GUI thread) and createPlatformOpenGLContext
(from its render thread), both of which needs to use the client buffer
integration.

In this patch, we fix it by first checking if the integration is initialized.
In that case, it's safe to use it. Otherwise we lock a mutex, re-check if
initialization has happened on another thread in the meantime, and then finally
we initialize it.

This way we should avoid the expense of mutex locking after initialization is
complete, while still staying race free during initialization.

Fixes: QTBUG-76504
Change-Id: I327840ebf41e014882cb659bc3e4fafb7bdb7a98
Reviewed-by: David Edmundson <davidedmundson@kde.org>
Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>

src/plugins/platforms/wayland/qwaylandintegration.cpp

@@ -301,11 +301,14 @@ QPlatformTheme *QWaylandIntegration::createPlatformTheme(const QString &name) co
     return GenericWaylandTheme::createUnixTheme(name);
 }
 
+// May be called from non-GUI threads
 QWaylandClientBufferIntegration *QWaylandIntegration::clientBufferIntegration() const
 {
-    if (!mClientBufferIntegrationInitialized)
+    // Do an inexpensive check first to avoid locking whenever possible
+    if (Q_UNLIKELY(!mClientBufferIntegrationInitialized))
         const_cast<QWaylandIntegration *>(this)->initializeClientBufferIntegration();
 
+    Q_ASSERT(mClientBufferIntegrationInitialized);
     return mClientBufferIntegration && mClientBufferIntegration->isValid() ? mClientBufferIntegration.data() : nullptr;
 }
 
@@ -325,9 +328,12 @@ QWaylandShellIntegration *QWaylandIntegration::shellIntegration() const
     return mShellIntegration.data();
 }
 
+// May be called from non-GUI threads
 void QWaylandIntegration::initializeClientBufferIntegration()
 {
-    mClientBufferIntegrationInitialized = true;
+    QMutexLocker lock(&mClientBufferInitLock);
+    if (mClientBufferIntegrationInitialized)
+        return;
 
     QString targetKey;
     bool disableHardwareIntegration = qEnvironmentVariableIsSet("QT_WAYLAND_DISABLE_HW_INTEGRATION");
@@ -345,17 +351,20 @@ void QWaylandIntegration::initializeClientBufferIntegration()
 
     if (targetKey.isEmpty()) {
         qWarning("Failed to determine what client buffer integration to use");
-        return;
+    } else {
+        QStringList keys = QWaylandClientBufferIntegrationFactory::keys();
+        if (keys.contains(targetKey)) {
+            mClientBufferIntegration.reset(QWaylandClientBufferIntegrationFactory::create(targetKey, QStringList()));
+        }
+        if (mClientBufferIntegration)
+            mClientBufferIntegration->initialize(mDisplay.data());
+        else
+            qWarning("Failed to load client buffer integration: %s\n", qPrintable(targetKey));
     }
 
-    QStringList keys = QWaylandClientBufferIntegrationFactory::keys();
+    // This must be set last to make sure other threads don't use the
-    if (keys.contains(targetKey)) {
+    // integration before initialization is complete.
-        mClientBufferIntegration.reset(QWaylandClientBufferIntegrationFactory::create(targetKey, QStringList()));
+    mClientBufferIntegrationInitialized = true;
-    }
-    if (mClientBufferIntegration)
-        mClientBufferIntegration->initialize(mDisplay.data());
-    else
-        qWarning("Failed to load client buffer integration: %s\n", qPrintable(targetKey));
 }
 
 void QWaylandIntegration::initializeServerBufferIntegration()

src/plugins/platforms/wayland/qwaylandintegration_p.h

@@ -54,6 +54,7 @@
 #include <QtWaylandClient/qtwaylandclientglobal.h>
 #include <qpa/qplatformintegration.h>
 #include <QtCore/QScopedPointer>
+#include <QtCore/QMutex>
 
 QT_BEGIN_NAMESPACE
 
@@ -148,6 +149,7 @@ private:
     QScopedPointer<QPlatformAccessibility> mAccessibility;
 #endif
     bool mFailed = false;
+    QMutex mClientBufferInitLock;
     bool mClientBufferIntegrationInitialized = false;
     bool mServerBufferIntegrationInitialized = false;
     bool mShellIntegrationInitialized = false;