File: 0061-Fix-frame-sync-related-to-unprotected-multithread-ac.patch

package info (click to toggle)
qtwayland-opensource-src 5.15.17-1
  • links: PTS, VCS
  • area: main
  • in suites: experimental
  • size: 11,108 kB
  • sloc: cpp: 53,691; xml: 9,462; ansic: 187; makefile: 29; sh: 5
file content (158 lines) | stat: -rw-r--r-- 6,011 bytes parent folder | download | duplicates (2)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
From 241486c4a90b222b9127c316c821090131057933 Mon Sep 17 00:00:00 2001
From: Weng Xuetian <wengxt@gmail.com>
Date: Sun, 27 Nov 2022 12:44:40 -0800
Subject: [PATCH] Fix frame sync related to unprotected multithread access

There is a few crashes happens in real life that frame callback is
double-free'd and hit an assertion in wayland-client. e.g.
https://bugs.kde.org/show_bug.cgi?id=450003

This is due to the WaylandEventThread and calls to QWaylandWindow::reset
may free and unset the mFrameCallback at the same time. mFrameSyncMutex
should be used to protect such access.

Pick-to: 6.4
Change-Id: Ie01d08d07a2f10f70606ed1935caac09cb4f0382
(cherry picked from commit b6cbb5e323822d6e3ef5ed4dd5a4c4cc1ea85038)
---
 src/client/qwaylandwindow.cpp | 64 ++++++++++++++++++++---------------
 src/client/qwaylandwindow_p.h | 11 +++---
 2 files changed, 43 insertions(+), 32 deletions(-)

--- a/src/client/qwaylandwindow.cpp
+++ b/src/client/qwaylandwindow.cpp
@@ -252,13 +252,16 @@ void QWaylandWindow::reset()
         mSurface.reset();
     }
 
-    if (mFrameCallback) {
-        wl_callback_destroy(mFrameCallback);
-        mFrameCallback = nullptr;
-    }
+    {
+        QMutexLocker lock(&mFrameSyncMutex);
+        if (mFrameCallback) {
+            wl_callback_destroy(mFrameCallback);
+            mFrameCallback = nullptr;
+        }
 
-    mFrameCallbackElapsedTimer.invalidate();
-    mWaitingForFrameCallback = false;
+        mFrameCallbackElapsedTimer.invalidate();
+        mWaitingForFrameCallback = false;
+    }
     mFrameCallbackTimedOut = false;
 
     mMask = QRegion();
@@ -633,18 +636,21 @@ const wl_callback_listener QWaylandWindow::callbackListener = {
     [](void *data, wl_callback *callback, uint32_t time) {
         Q_UNUSED(time);
         auto *window = static_cast<QWaylandWindow*>(data);
-
-        Q_ASSERT(callback == window->mFrameCallback);
-        wl_callback_destroy(callback);
-        window->mFrameCallback = nullptr;
-
-        window->handleFrameCallback();
+        window->handleFrameCallback(callback);
     }
 };
 
-void QWaylandWindow::handleFrameCallback()
+void QWaylandWindow::handleFrameCallback(wl_callback* callback)
 {
     QMutexLocker locker(&mFrameSyncMutex);
+    if (!mFrameCallback) {
+        // This means the callback is already unset by QWaylandWindow::reset.
+        // The wl_callback object will be destroyed there too.
+        return;
+    }
+    Q_ASSERT(callback == mFrameCallback);
+    wl_callback_destroy(callback);
+    mFrameCallback = nullptr;
 
     mWaitingForFrameCallback = false;
     mFrameCallbackElapsedTimer.invalidate();
@@ -1172,19 +1178,24 @@ void QWaylandWindow::timerEvent(QTimerEvent *event)
     if (event->timerId() != mFrameCallbackCheckIntervalTimerId)
         return;
 
-    bool callbackTimerExpired = mFrameCallbackElapsedTimer.hasExpired(mFrameCallbackTimeout);
-    if (!mFrameCallbackElapsedTimer.isValid() || callbackTimerExpired ) {
-        killTimer(mFrameCallbackCheckIntervalTimerId);
-        mFrameCallbackCheckIntervalTimerId = -1;
-    }
-    if (mFrameCallbackElapsedTimer.isValid() && callbackTimerExpired) {
-        mFrameCallbackElapsedTimer.invalidate();
+    {
+        QMutexLocker lock(&mFrameSyncMutex);
 
-        qCDebug(lcWaylandBackingstore) << "Didn't receive frame callback in time, window should now be inexposed";
-        mFrameCallbackTimedOut = true;
-        mWaitingForUpdate = false;
-        sendExposeEvent(QRect());
+        bool callbackTimerExpired = mFrameCallbackElapsedTimer.hasExpired(mFrameCallbackTimeout);
+        if (!mFrameCallbackElapsedTimer.isValid() || callbackTimerExpired ) {
+            killTimer(mFrameCallbackCheckIntervalTimerId);
+            mFrameCallbackCheckIntervalTimerId = -1;
+        }
+        if (!mFrameCallbackElapsedTimer.isValid() || !callbackTimerExpired) {
+            return;
+        }
+        mFrameCallbackElapsedTimer.invalidate();
     }
+
+    qCDebug(lcWaylandBackingstore) << "Didn't receive frame callback in time, window should now be inexposed";
+    mFrameCallbackTimedOut = true;
+    mWaitingForUpdate = false;
+    sendExposeEvent(QRect());
 }
 
 void QWaylandWindow::requestUpdate()
@@ -1227,15 +1238,14 @@ void QWaylandWindow::handleUpdate()
 {
     qCDebug(lcWaylandBackingstore) << "handleUpdate" << QThread::currentThread();
 
-    if (mWaitingForFrameCallback)
-        return;
-
     // TODO: Should sync subsurfaces avoid requesting frame callbacks?
     QReadLocker lock(&mSurfaceLock);
     if (!mSurface)
         return;
 
     QMutexLocker locker(&mFrameSyncMutex);
+    if (mWaitingForFrameCallback)
+        return;
 
     struct ::wl_surface *wrappedSurface = reinterpret_cast<struct ::wl_surface *>(wl_proxy_create_wrapper(mSurface->object()));
     wl_proxy_set_queue(reinterpret_cast<wl_proxy *>(wrappedSurface), mDisplay->frameEventQueue());
--- a/src/client/qwaylandwindow_p.h
+++ b/src/client/qwaylandwindow_p.h
@@ -237,12 +237,13 @@ protected:
     Qt::MouseButtons mMousePressedInContentArea = Qt::NoButton;
 
     WId mWindowId;
-    bool mWaitingForFrameCallback = false;
     bool mFrameCallbackTimedOut = false; // Whether the frame callback has timed out
-    QAtomicInt mWaitingForUpdateDelivery = false;
     int mFrameCallbackCheckIntervalTimerId = -1;
-    QElapsedTimer mFrameCallbackElapsedTimer;
-    struct ::wl_callback *mFrameCallback = nullptr;
+    QAtomicInt mWaitingForUpdateDelivery = false;
+
+    bool mWaitingForFrameCallback = false; // Protected by mFrameSyncMutex
+    QElapsedTimer mFrameCallbackElapsedTimer; // Protected by mFrameSyncMutex
+    struct ::wl_callback *mFrameCallback = nullptr; // Protected by mFrameSyncMutex
     QMutex mFrameSyncMutex;
     QWaitCondition mFrameSyncWait;
 
@@ -297,7 +298,7 @@ private:
     QRect mLastExposeGeometry;
 
     static const wl_callback_listener callbackListener;
-    void handleFrameCallback();
+    void handleFrameCallback(struct ::wl_callback* callback);
 
     static QWaylandWindow *mMouseGrab;