summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohan Klokkhammer Helsing <johan.helsing@qt.io>2017-08-30 14:00:51 +0200
committerJohan Helsing <johan.helsing@qt.io>2017-08-31 09:50:51 +0000
commit3e745ea053e42ba087438203df3bd76b30a08eb2 (patch)
treea290f8a2aff959de025dcae3268a5d78ed0bbd1d
parent97683fd367d9a42aeb6caadad5daa8e07412469e (diff)
downloadqtwayland-3e745ea053e42ba087438203df3bd76b30a08eb2.tar.gz
Fix crash when window attach is called without waiting for frame callbacks
If QWaylandWindow::attach was called before getting the frame callback, it would overwrite mFrameCallback. Hence, all but the last frame callback would still be alive after the QWaylandWindow destructor. When the dangling callbacks got invoked the data pointer was statically casted to the deleted QWaylandWindow, resulting in undefined behavior. In this change we only delete frame callbacks from the render thread, avoiding a race condition we fixed earlier. And we always destroy the frame callback when adding a new one, ensuring that the destructor will clean up the only remaining callback. There's a test confirming that the crash has been fixed. This fixes the flakiness of many of the qtbase auto tests. Change-Id: Iecb08ab48216eac61b1ebc5c0e0664d4aac900c0 Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>
-rw-r--r--src/client/qwaylandwindow.cpp13
-rw-r--r--src/client/qwaylandwindow_p.h3
-rw-r--r--tests/auto/client/client/tst_client.cpp27
3 files changed, 34 insertions, 9 deletions
diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp
index 4a60a661..eeba6c85 100644
--- a/src/client/qwaylandwindow.cpp
+++ b/src/client/qwaylandwindow.cpp
@@ -86,7 +86,6 @@ QWaylandWindow::QWaylandWindow(QWindow *window)
, mMouseEventsInContentArea(false)
, mMousePressedInContentArea(Qt::NoButton)
, mWaitingForFrameSync(false)
- , mFrameCallback(nullptr)
, mRequestResizeSent(false)
, mCanResize(true)
, mResizeDirty(false)
@@ -475,12 +474,14 @@ void QWaylandWindow::requestResize()
void QWaylandWindow::attach(QWaylandBuffer *buffer, int x, int y)
{
- mFrameCallback = nullptr;
+ if (mFrameCallback) {
+ wl_callback_destroy(mFrameCallback);
+ mFrameCallback = nullptr;
+ }
if (buffer) {
- auto callback = frame();
- wl_callback_add_listener(callback, &QWaylandWindow::callbackListener, this);
- mFrameCallback = callback;
+ mFrameCallback = frame();
+ wl_callback_add_listener(mFrameCallback, &QWaylandWindow::callbackListener, this);
mWaitingForFrameSync = true;
buffer->setBusy();
@@ -523,8 +524,6 @@ void QWaylandWindow::frameCallback(void *data, struct wl_callback *callback, uin
QWaylandWindow *self = static_cast<QWaylandWindow*>(data);
self->mWaitingForFrameSync = false;
- wl_callback_destroy(callback);
- self->mFrameCallback.testAndSetRelaxed(callback, nullptr);
if (self->mUpdateRequested) {
QWindowPrivate *w = QWindowPrivate::get(self->window());
self->mUpdateRequested = false;
diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h
index 29eb6c59..fa213d07 100644
--- a/src/client/qwaylandwindow_p.h
+++ b/src/client/qwaylandwindow_p.h
@@ -53,7 +53,6 @@
#include <QtCore/QWaitCondition>
#include <QtCore/QMutex>
-#include <QtCore/QAtomicPointer>
#include <QtGui/QIcon>
#include <QtCore/QVariant>
@@ -222,7 +221,7 @@ protected:
WId mWindowId;
bool mWaitingForFrameSync;
- QAtomicPointer<struct wl_callback> mFrameCallback;
+ struct ::wl_callback *mFrameCallback = nullptr;
QWaitCondition mFrameSyncWait;
QMutex mResizeLock;
diff --git a/tests/auto/client/client/tst_client.cpp b/tests/auto/client/client/tst_client.cpp
index 0c9c007a..8acddfbe 100644
--- a/tests/auto/client/client/tst_client.cpp
+++ b/tests/auto/client/client/tst_client.cpp
@@ -142,6 +142,7 @@ private slots:
void backingStore();
void touchDrag();
void mouseDrag();
+ void dontCrashOnMultipleCommits();
private:
MockCompositor *compositor;
@@ -333,6 +334,32 @@ void tst_WaylandClient::mouseDrag()
QTRY_VERIFY(window.dragStarted);
}
+void tst_WaylandClient::dontCrashOnMultipleCommits()
+{
+ auto window = new TestWindow();
+ window->show();
+
+ QRect rect(QPoint(), window->size());
+
+ QBackingStore backingStore(window);
+ backingStore.resize(rect.size());
+ backingStore.beginPaint(rect);
+ QPainter p(backingStore.paintDevice());
+ p.fillRect(rect, Qt::magenta);
+ p.end();
+ backingStore.endPaint();
+
+ backingStore.flush(rect);
+ backingStore.flush(rect);
+ backingStore.flush(rect);
+
+ compositor->processWaylandEvents();
+
+ delete window;
+
+ QTRY_VERIFY(!compositor->surface());
+}
+
int main(int argc, char **argv)
{
setenv("XDG_RUNTIME_DIR", ".", 1);