From d49cd790d8525071f6613066caf5e8dd2f5ca980 Mon Sep 17 00:00:00 2001 From: Giulio Camuffo Date: Fri, 23 Oct 2015 13:29:41 +0300 Subject: [PATCH] client: Remove the event thread If the compositor sends events to us while the main thread is blocked the socket notifier in the events thread would keep sending out the activated() signal, but no events would actually be read until the main thread starts to run again. That causes the event thread to keep queueing new events, and so allocating memory, potentially forever. This patch fixes the issue in maybe a bit radical way, that is by removing the event thread. The socket notifier now runs in the main thread so it will block if the events are not being read. Nowadays there is no real reason to keep the event thread around, as every thread that needs to receive wayland events can dispatch them on its own, we don't need a central dispatcher thread anymore. Change-Id: Ib7885e4b038b82719d78d193f465618a72cbe6af Reviewed-by: Pier Luigi Fiorini --- src/plugins/platforms/wayland/client.pro | 2 - .../platforms/wayland/qwaylanddisplay.cpp | 42 ++++--- .../platforms/wayland/qwaylanddisplay_p.h | 3 +- .../platforms/wayland/qwaylandeventthread.cpp | 112 ------------------ .../platforms/wayland/qwaylandeventthread_p.h | 97 --------------- .../platforms/wayland/qwaylandintegration.cpp | 5 + 6 files changed, 29 insertions(+), 232 deletions(-) delete mode 100644 src/plugins/platforms/wayland/qwaylandeventthread.cpp delete mode 100644 src/plugins/platforms/wayland/qwaylandeventthread_p.h diff --git a/src/plugins/platforms/wayland/client.pro b/src/plugins/platforms/wayland/client.pro index 7182f3203d8..8bd9e1dc83a 100644 --- a/src/plugins/platforms/wayland/client.pro +++ b/src/plugins/platforms/wayland/client.pro @@ -78,7 +78,6 @@ SOURCES += qwaylandintegration.cpp \ qwaylandabstractdecoration.cpp \ qwaylanddecorationfactory.cpp \ qwaylanddecorationplugin.cpp \ - qwaylandeventthread.cpp\ qwaylandwindowmanagerintegration.cpp \ qwaylandinputcontext.cpp \ qwaylanddatadevice.cpp \ @@ -111,7 +110,6 @@ HEADERS += qwaylandintegration_p.h \ qwaylandabstractdecoration_p.h \ qwaylanddecorationfactory_p.h \ qwaylanddecorationplugin_p.h \ - qwaylandeventthread_p.h \ qwaylandwindowmanagerintegration_p.h \ qwaylandinputcontext_p.h \ qwaylanddatadevice_p.h \ diff --git a/src/plugins/platforms/wayland/qwaylanddisplay.cpp b/src/plugins/platforms/wayland/qwaylanddisplay.cpp index dbb8e12dd07..a6f52bc2432 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay.cpp +++ b/src/plugins/platforms/wayland/qwaylanddisplay.cpp @@ -33,7 +33,6 @@ #include "qwaylanddisplay_p.h" -#include "qwaylandeventthread_p.h" #include "qwaylandintegration_p.h" #include "qwaylandwindow_p.h" #include "qwaylandscreen_p.h" @@ -145,21 +144,15 @@ QWaylandDisplay::QWaylandDisplay(QWaylandIntegration *waylandIntegration) { qRegisterMetaType("uint32_t"); - mEventThreadObject = new QWaylandEventThread(0); - mEventThread = new QThread(this); - mEventThread->setObjectName(QStringLiteral("QtWayland")); - mEventThreadObject->moveToThread(mEventThread); - mEventThread->start(); - - mEventThreadObject->displayConnect(); - mDisplay = mEventThreadObject->display(); //blocks until display is available + mDisplay = wl_display_connect(NULL); + if (mDisplay == NULL) { + qErrnoWarning(errno, "Failed to create display"); + ::exit(1); + } struct ::wl_registry *registry = wl_display_get_registry(mDisplay); init(registry); - connect(mEventThreadObject, SIGNAL(newEventsRead()), this, SLOT(flushRequests())); - connect(mEventThreadObject, &QWaylandEventThread::fatalError, this, &QWaylandDisplay::exitWithError); - mWindowManagerIntegration.reset(new QWaylandWindowManagerIntegration(this)); forceRoundTrip(); @@ -175,15 +168,28 @@ QWaylandDisplay::~QWaylandDisplay(void) } mScreens.clear(); delete mDndSelectionHandler.take(); - mEventThread->quit(); - mEventThread->wait(); - delete mEventThreadObject; + wl_display_disconnect(mDisplay); +} + +void QWaylandDisplay::checkError() const +{ + int ecode = wl_display_get_error(mDisplay); + if ((ecode == EPIPE || ecode == ECONNRESET)) { + // special case this to provide a nicer error + qWarning("The Wayland connection broke. Did the Wayland compositor die?"); + } else { + qErrnoWarning(ecode, "The Wayland connection experienced a fatal error"); + } } void QWaylandDisplay::flushRequests() { + if (wl_display_prepare_read(mDisplay) == 0) { + wl_display_read_events(mDisplay); + } + if (wl_display_dispatch_pending(mDisplay) < 0) { - mEventThreadObject->checkError(); + checkError(); exitWithError(); } @@ -194,15 +200,13 @@ void QWaylandDisplay::flushRequests() void QWaylandDisplay::blockingReadEvents() { if (wl_display_dispatch(mDisplay) < 0) { - mEventThreadObject->checkError(); + checkError(); exitWithError(); } } void QWaylandDisplay::exitWithError() { - mEventThread->quit(); - mEventThread->wait(); ::exit(1); } diff --git a/src/plugins/platforms/wayland/qwaylanddisplay_p.h b/src/plugins/platforms/wayland/qwaylanddisplay_p.h index bf48c212304..8262e72be46 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay_p.h +++ b/src/plugins/platforms/wayland/qwaylanddisplay_p.h @@ -177,6 +177,7 @@ public slots: private: void waitForScreens(); void exitWithError(); + void checkError() const; struct Listener { RegistryListener listener; @@ -186,8 +187,6 @@ private: struct wl_display *mDisplay; QtWayland::wl_compositor mCompositor; struct wl_shm *mShm; - QThread *mEventThread; - QWaylandEventThread *mEventThreadObject; QScopedPointer mShell; QScopedPointer mShellXdg; QList mScreens; diff --git a/src/plugins/platforms/wayland/qwaylandeventthread.cpp b/src/plugins/platforms/wayland/qwaylandeventthread.cpp deleted file mode 100644 index e0a3edcb81f..00000000000 --- a/src/plugins/platforms/wayland/qwaylandeventthread.cpp +++ /dev/null @@ -1,112 +0,0 @@ -/**************************************************************************** -** -** Copyright (C) 2015 The Qt Company Ltd. -** Contact: http://www.qt.io/licensing/ -** -** This file is part of the plugins of the Qt Toolkit. -** -** $QT_BEGIN_LICENSE:LGPL21$ -** Commercial License Usage -** Licensees holding valid commercial Qt licenses may use this file in -** accordance with the commercial license agreement provided with the -** Software or, alternatively, in accordance with the terms contained in -** a written agreement between you and The Qt Company. For licensing terms -** and conditions see http://www.qt.io/terms-conditions. For further -** information use the contact form at http://www.qt.io/contact-us. -** -** GNU Lesser General Public License Usage -** Alternatively, this file may be used under the terms of the GNU Lesser -** General Public License version 2.1 or version 3 as published by the Free -** Software Foundation and appearing in the file LICENSE.LGPLv21 and -** LICENSE.LGPLv3 included in the packaging of this file. Please review the -** following information to ensure the GNU Lesser General Public License -** requirements will be met: https://www.gnu.org/licenses/lgpl.html and -** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. -** -** As a special exception, The Qt Company gives you certain additional -** rights. These rights are described in The Qt Company LGPL Exception -** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. -** -** $QT_END_LICENSE$ -** -****************************************************************************/ - -#include "qwaylandeventthread_p.h" -#include -#include - -#include -#include -#include -#include - -QT_BEGIN_NAMESPACE - -namespace QtWaylandClient { - -QWaylandEventThread::QWaylandEventThread(QObject *parent) - : QObject(parent) - , m_display(0) - , m_fileDescriptor(-1) - , m_readNotifier(0) - , m_displayLock(new QMutex) -{ -} - -QWaylandEventThread::~QWaylandEventThread() -{ - delete m_displayLock; - wl_display_disconnect(m_display); -} - -void QWaylandEventThread::displayConnect() -{ - m_displayLock->lock(); - QMetaObject::invokeMethod(this, "waylandDisplayConnect", Qt::QueuedConnection); -} - -// ### be careful what you do, this function may also be called from other -// threads to clean up & exit. -void QWaylandEventThread::checkError() const -{ - int ecode = wl_display_get_error(m_display); - if ((ecode == EPIPE || ecode == ECONNRESET)) { - // special case this to provide a nicer error - qWarning("The Wayland connection broke. Did the Wayland compositor die?"); - } else { - qErrnoWarning(ecode, "The Wayland connection experienced a fatal error"); - } -} - -void QWaylandEventThread::readWaylandEvents() -{ - if (wl_display_prepare_read(m_display) == 0) { - wl_display_read_events(m_display); - } - emit newEventsRead(); -} - -void QWaylandEventThread::waylandDisplayConnect() -{ - m_display = wl_display_connect(NULL); - if (m_display == NULL) { - qErrnoWarning(errno, "Failed to create display"); - ::exit(1); - } - m_displayLock->unlock(); - - m_fileDescriptor = wl_display_get_fd(m_display); - - m_readNotifier = new QSocketNotifier(m_fileDescriptor, QSocketNotifier::Read, this); - connect(m_readNotifier, SIGNAL(activated(int)), this, SLOT(readWaylandEvents())); -} - -wl_display *QWaylandEventThread::display() const -{ - QMutexLocker displayLock(m_displayLock); - return m_display; -} - -} - -QT_END_NAMESPACE diff --git a/src/plugins/platforms/wayland/qwaylandeventthread_p.h b/src/plugins/platforms/wayland/qwaylandeventthread_p.h deleted file mode 100644 index 0920d403e42..00000000000 --- a/src/plugins/platforms/wayland/qwaylandeventthread_p.h +++ /dev/null @@ -1,97 +0,0 @@ -/**************************************************************************** -** -** Copyright (C) 2015 The Qt Company Ltd. -** Contact: http://www.qt.io/licensing/ -** -** This file is part of the plugins of the Qt Toolkit. -** -** $QT_BEGIN_LICENSE:LGPL21$ -** Commercial License Usage -** Licensees holding valid commercial Qt licenses may use this file in -** accordance with the commercial license agreement provided with the -** Software or, alternatively, in accordance with the terms contained in -** a written agreement between you and The Qt Company. For licensing terms -** and conditions see http://www.qt.io/terms-conditions. For further -** information use the contact form at http://www.qt.io/contact-us. -** -** GNU Lesser General Public License Usage -** Alternatively, this file may be used under the terms of the GNU Lesser -** General Public License version 2.1 or version 3 as published by the Free -** Software Foundation and appearing in the file LICENSE.LGPLv21 and -** LICENSE.LGPLv3 included in the packaging of this file. Please review the -** following information to ensure the GNU Lesser General Public License -** requirements will be met: https://www.gnu.org/licenses/lgpl.html and -** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. -** -** As a special exception, The Qt Company gives you certain additional -** rights. These rights are described in The Qt Company LGPL Exception -** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. -** -** $QT_END_LICENSE$ -** -****************************************************************************/ - -#ifndef QWAYLANDEVENTTHREAD_H -#define QWAYLANDEVENTTHREAD_H - -// -// W A R N I N G -// ------------- -// -// This file is not part of the Qt API. It exists purely as an -// implementation detail. This header file may change from version to -// version without notice, or even be removed. -// -// We mean it. -// - -#include -#include -#include - -#include - -QT_BEGIN_NAMESPACE - -class QSocketNotifier; - -namespace QtWaylandClient { - -class Q_WAYLAND_CLIENT_EXPORT QWaylandEventThread : public QObject -{ - Q_OBJECT -public: - explicit QWaylandEventThread(QObject *parent = 0); - ~QWaylandEventThread(); - - void displayConnect(); - - wl_display *display() const; - - void checkError() const; - -private slots: - void readWaylandEvents(); - - void waylandDisplayConnect(); - -signals: - void newEventsRead(); - void fatalError(); - -private: - - struct wl_display *m_display; - int m_fileDescriptor; - - QSocketNotifier *m_readNotifier; - - QMutex *m_displayLock; - -}; - -} - -QT_END_NAMESPACE - -#endif // QWAYLANDEVENTTHREAD_H diff --git a/src/plugins/platforms/wayland/qwaylandintegration.cpp b/src/plugins/platforms/wayland/qwaylandintegration.cpp index 82df8a30a41..39fff533d32 100644 --- a/src/plugins/platforms/wayland/qwaylandintegration.cpp +++ b/src/plugins/platforms/wayland/qwaylandintegration.cpp @@ -54,6 +54,7 @@ #include #include #include +#include #include #include @@ -201,6 +202,10 @@ void QWaylandIntegration::initialize() QAbstractEventDispatcher *dispatcher = QGuiApplicationPrivate::eventDispatcher; QObject::connect(dispatcher, SIGNAL(aboutToBlock()), mDisplay, SLOT(flushRequests())); QObject::connect(dispatcher, SIGNAL(awake()), mDisplay, SLOT(flushRequests())); + + int fd = wl_display_get_fd(mDisplay->wl_display()); + QSocketNotifier *sn = new QSocketNotifier(fd, QSocketNotifier::Read, mDisplay); + QObject::connect(sn, SIGNAL(activated(int)), mDisplay, SLOT(flushRequests())); } QPlatformFontDatabase *QWaylandIntegration::fontDatabase() const