summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJohan Klokkhammer Helsing <johan.helsing@qt.io>2019-07-08 10:22:03 +0200
committerJohan Klokkhammer Helsing <johan.helsing@qt.io>2019-08-20 12:07:09 +0200
commit28c852df3825be05bea9e3d17475c73c409ee534 (patch)
treebe738f2d966f542e8410f0ccfd3831dfce3d6d91
parent37a03693f22cba7a17a5e7bdc3c340e7646aa6ef (diff)
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>
-rw-r--r--src/client/qwaylandintegration.cpp31
-rw-r--r--src/client/qwaylandintegration_p.h2
2 files changed, 22 insertions, 11 deletions
diff --git a/src/client/qwaylandintegration.cpp b/src/client/qwaylandintegration.cpp
index 97e0203c..46bef294 100644
--- a/src/client/qwaylandintegration.cpp
+++ b/src/client/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();
- 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));
+ // This must be set last to make sure other threads don't use the
+ // integration before initialization is complete.
+ mClientBufferIntegrationInitialized = true;
}
void QWaylandIntegration::initializeServerBufferIntegration()
diff --git a/src/client/qwaylandintegration_p.h b/src/client/qwaylandintegration_p.h
index a5a3d7b6..7c1cb978 100644
--- a/src/client/qwaylandintegration_p.h
+++ b/src/client/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;