1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix frame sync related to unprotected multithread access

Browse Source 
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
Reviewed-by: David Edmundson <davidedmundson@kde.org>
This commit is contained in:
Weng Xuetian 2022-11-27 12:44:40 -08:00
parent 27e59681ac
commit 2b49cf04a6
2 changed files with 43 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
src/plugins/platforms/wayland/qwaylandwindow.cpp
View File

@ -228,13 +228,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;
mWaitingToApplyConfigure = false;
@ -672,18 +675,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();
@ -1382,19 +1388,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()
@ -1437,15 +1448,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());

11
src/plugins/platforms/wayland/qwaylandwindow_p.h
View File

@ -261,12 +261,13 @@ protected:
#endif
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;
@ -323,7 +324,7 @@ private:
QRect mLastExposeGeometry;
static const wl_callback_listener callbackListener;
void handleFrameCallback();
void handleFrameCallback(struct ::wl_callback* callback);
static QWaylandWindow *mMouseGrab;