diff --git a/autotests/integration/kwin_wayland_test.cpp b/autotests/integration/kwin_wayland_test.cpp index dae6bf9f25..51e146d871 100644 --- a/autotests/integration/kwin_wayland_test.cpp +++ b/autotests/integration/kwin_wayland_test.cpp @@ -83,17 +83,13 @@ WaylandTestApplication::~WaylandTestApplication() if (effects) { static_cast(effects)->unloadAllEffects(); } - if (m_xwayland) { - // needs to be done before workspace gets destroyed - m_xwayland->prepareDestroy(); - } + delete m_xwayland; + m_xwayland = nullptr; destroyWorkspace(); waylandServer()->dispatch(); if (QStyle *s = style()) { s->unpolish(this); } - // kill Xwayland before terminating its connection - delete m_xwayland; waylandServer()->terminateClientConnections(); destroyCompositor(); } @@ -133,7 +129,7 @@ void WaylandTestApplication::continueStartupWithScreens() void WaylandTestApplication::finalizeStartup() { if (m_xwayland) { - disconnect(m_xwayland, &Xwl::Xwayland::initialized, this, &WaylandTestApplication::finalizeStartup); + disconnect(m_xwayland, &Xwl::Xwayland::started, this, &WaylandTestApplication::finalizeStartup); } notifyStarted(); } @@ -160,8 +156,8 @@ void WaylandTestApplication::continueStartupWithScene() std::cerr << "Xwayland had a critical error. Going to exit now." << std::endl; exit(code); }); - connect(m_xwayland, &Xwl::Xwayland::initialized, this, &WaylandTestApplication::finalizeStartup); - m_xwayland->init(); + connect(m_xwayland, &Xwl::Xwayland::started, this, &WaylandTestApplication::finalizeStartup); + m_xwayland->start(); } } diff --git a/composite.cpp b/composite.cpp index acd26e760a..cf744a3cb6 100644 --- a/composite.cpp +++ b/composite.cpp @@ -194,7 +194,7 @@ bool Compositor::setupStart() options->reloadCompositingSettings(true); - setupX11Support(); + initializeX11(); // There might still be a deleted around, needs to be cleared before // creating the scene (BUG 333275). @@ -306,8 +306,13 @@ bool Compositor::setupStart() return true; } -void Compositor::claimCompositorSelection() +void Compositor::initializeX11() { + xcb_connection_t *connection = kwinApp()->x11Connection(); + if (!connection) { + return; + } + if (!m_selectionOwner) { char selection_name[ 100 ]; sprintf(selection_name, "_NET_WM_CM_S%d", Application::x11ScreenNumber()); @@ -315,40 +320,34 @@ void Compositor::claimCompositorSelection() connect(m_selectionOwner, &CompositorSelectionOwner::lostOwnership, this, &Compositor::stop); } - - if (!m_selectionOwner) { - // No X11 yet. - return; - } if (!m_selectionOwner->owning()) { // Force claim ownership. m_selectionOwner->claim(true); m_selectionOwner->setOwning(true); } + + xcb_composite_redirect_subwindows(connection, kwinApp()->x11RootWindow(), + XCB_COMPOSITE_REDIRECT_MANUAL); } -void Compositor::setupX11Support() +void Compositor::cleanupX11() { - auto *con = kwinApp()->x11Connection(); - if (!con) { - delete m_selectionOwner; - m_selectionOwner = nullptr; - return; - } - claimCompositorSelection(); - xcb_composite_redirect_subwindows(con, kwinApp()->x11RootWindow(), - XCB_COMPOSITE_REDIRECT_MANUAL); + delete m_selectionOwner; + m_selectionOwner = nullptr; } void Compositor::startupWithWorkspace() { connect(kwinApp(), &Application::x11ConnectionChanged, - this, &Compositor::setupX11Support, Qt::UniqueConnection); + this, &Compositor::initializeX11, Qt::UniqueConnection); + connect(kwinApp(), &Application::x11ConnectionAboutToBeDestroyed, + this, &Compositor::cleanupX11, Qt::UniqueConnection); + initializeX11(); + Workspace::self()->markXStackingOrderAsDirty(); Q_ASSERT(m_scene); connect(workspace(), &Workspace::destroyed, this, [this] { compositeTimer.stop(); }); - setupX11Support(); fpsInterval = options->maxFpsInterval(); if (m_scene->syncsToVBlank()) { diff --git a/composite.h b/composite.h index 84783c8316..a0b210f530 100644 --- a/composite.h +++ b/composite.h @@ -141,9 +141,8 @@ protected: static Compositor *s_compositor; private: - void claimCompositorSelection(); - - void setupX11Support(); + void initializeX11(); + void cleanupX11(); void setCompositeTimer(); bool windowRepaintsPending() const; diff --git a/libkwineffects/kwinglobals.h b/libkwineffects/kwinglobals.h index 8bfcfe8ce8..04e92a1b08 100644 --- a/libkwineffects/kwinglobals.h +++ b/libkwineffects/kwinglobals.h @@ -152,22 +152,13 @@ Q_ENUM_NS(SessionState) inline KWIN_EXPORT xcb_connection_t *connection() { - static xcb_connection_t *s_con = nullptr; - if (!s_con) { - s_con = reinterpret_cast(qApp->property("x11Connection").value()); - } - Q_ASSERT(qApp); - return s_con; + return reinterpret_cast(qApp->property("x11Connection").value()); } inline KWIN_EXPORT xcb_window_t rootWindow() { - static xcb_window_t s_rootWindow = XCB_WINDOW_NONE; - if (s_rootWindow == XCB_WINDOW_NONE) { - s_rootWindow = qApp->property("x11RootWindow").value(); - } - return s_rootWindow; + return qApp->property("x11RootWindow").value(); } inline @@ -179,19 +170,19 @@ KWIN_EXPORT xcb_timestamp_t xTime() inline KWIN_EXPORT xcb_screen_t *defaultScreen() { - static xcb_screen_t *s_screen = nullptr; - if (s_screen) { - return s_screen; + xcb_connection_t *c = connection(); + if (!c) { + return nullptr; } int screen = qApp->property("x11ScreenNumber").toInt(); - for (xcb_screen_iterator_t it = xcb_setup_roots_iterator(xcb_get_setup(connection())); + for (xcb_screen_iterator_t it = xcb_setup_roots_iterator(xcb_get_setup(c)); it.rem; --screen, xcb_screen_next(&it)) { if (screen == 0) { - s_screen = it.data; + return it.data; } } - return s_screen; + return nullptr; } inline diff --git a/main.cpp b/main.cpp index 41c0418f80..fdb465c42c 100644 --- a/main.cpp +++ b/main.cpp @@ -309,11 +309,16 @@ void Application::createOptions() options = new Options; } -void Application::setupEventFilters() +void Application::installNativeX11EventFilter() { installNativeEventFilter(m_eventFilter.data()); } +void Application::removeNativeX11EventFilter() +{ + removeNativeEventFilter(m_eventFilter.data()); +} + void Application::destroyWorkspace() { delete Workspace::self(); diff --git a/main.h b/main.h index cbae0fbf89..1290b3671d 100644 --- a/main.h +++ b/main.h @@ -206,7 +206,8 @@ protected: void createWorkspace(); void createAtoms(); void createOptions(); - void setupEventFilters(); + void installNativeX11EventFilter(); + void removeNativeX11EventFilter(); void destroyWorkspace(); void destroyCompositor(); /** diff --git a/main_wayland.cpp b/main_wayland.cpp index b3dff99798..73c4f9e3d1 100644 --- a/main_wayland.cpp +++ b/main_wayland.cpp @@ -134,19 +134,14 @@ ApplicationWayland::~ApplicationWayland() if (effects) { static_cast(effects)->unloadAllEffects(); } - if (m_xwayland) { - // needs to be done before workspace gets destroyed - m_xwayland->prepareDestroy(); - } + delete m_xwayland; + m_xwayland = nullptr; destroyWorkspace(); waylandServer()->dispatch(); if (QStyle *s = style()) { s->unpolish(this); } - // kill Xwayland before terminating its connection - delete m_xwayland; - m_xwayland = nullptr; waylandServer()->terminateClientConnections(); destroyCompositor(); } @@ -196,7 +191,7 @@ void ApplicationWayland::continueStartupWithScreens() void ApplicationWayland::finalizeStartup() { if (m_xwayland) { - disconnect(m_xwayland, &Xwl::Xwayland::initialized, this, &ApplicationWayland::finalizeStartup); + disconnect(m_xwayland, &Xwl::Xwayland::started, this, &ApplicationWayland::finalizeStartup); } startSession(); notifyStarted(); @@ -225,8 +220,8 @@ void ApplicationWayland::continueStartupWithScene() std::cerr << "Xwayland had a critical error. Going to exit now." << std::endl; exit(code); }); - connect(m_xwayland, &Xwl::Xwayland::initialized, this, &ApplicationWayland::finalizeStartup); - m_xwayland->init(); + connect(m_xwayland, &Xwl::Xwayland::started, this, &ApplicationWayland::finalizeStartup); + m_xwayland->start(); } void ApplicationWayland::startSession() diff --git a/main_x11.cpp b/main_x11.cpp index 5897404563..051058ca0a 100644 --- a/main_x11.cpp +++ b/main_x11.cpp @@ -223,7 +223,7 @@ void ApplicationX11::performStartup() }); connect(owner.data(), SIGNAL(lostOwnership()), SLOT(lostSelection())); connect(owner.data(), &KSelectionOwner::claimedOwnership, [this]{ - setupEventFilters(); + installNativeX11EventFilter(); // first load options - done internally by a different thread createOptions(); diff --git a/rules.cpp b/rules.cpp index ed74841147..af8418006c 100644 --- a/rules.cpp +++ b/rules.cpp @@ -936,8 +936,9 @@ RuleBook::RuleBook(QObject *parent) , m_updatesDisabled(false) , m_temporaryRulesMessages() { - initWithX11(); - connect(kwinApp(), &Application::x11ConnectionChanged, this, &RuleBook::initWithX11); + initializeX11(); + connect(kwinApp(), &Application::x11ConnectionChanged, this, &RuleBook::initializeX11); + connect(kwinApp(), &Application::x11ConnectionAboutToBeDestroyed, this, &RuleBook::cleanupX11); connect(m_updateTimer, SIGNAL(timeout()), SLOT(save())); m_updateTimer->setInterval(1000); m_updateTimer->setSingleShot(true); @@ -949,17 +950,21 @@ RuleBook::~RuleBook() deleteAll(); } -void RuleBook::initWithX11() +void RuleBook::initializeX11() { auto c = kwinApp()->x11Connection(); if (!c) { - m_temporaryRulesMessages.reset(); return; } m_temporaryRulesMessages.reset(new KXMessages(c, kwinApp()->x11RootWindow(), "_KDE_NET_WM_TEMPORARY_RULES", nullptr)); connect(m_temporaryRulesMessages.data(), SIGNAL(gotMessage(QString)), SLOT(temporaryRulesMessage(QString))); } +void RuleBook::cleanupX11() +{ + m_temporaryRulesMessages.reset(); +} + void RuleBook::deleteAll() { qDeleteAll(m_rules); diff --git a/rules.h b/rules.h index c6f6061dbc..c5f8181807 100644 --- a/rules.h +++ b/rules.h @@ -312,7 +312,8 @@ private Q_SLOTS: private: void deleteAll(); - void initWithX11(); + void initializeX11(); + void cleanupX11(); QTimer *m_updateTimer; bool m_updatesDisabled; QList m_rules; diff --git a/unmanaged.cpp b/unmanaged.cpp index 3c3f9e2f8f..8e3695362c 100644 --- a/unmanaged.cpp +++ b/unmanaged.cpp @@ -109,9 +109,9 @@ void Unmanaged::release(ReleaseReason releaseReason) xcb_shape_select_input(connection(), window(), false); Xcb::selectInput(window(), XCB_EVENT_MASK_NO_EVENT); } + workspace()->removeUnmanaged(this); + addWorkspaceRepaint(visibleRect()); if (releaseReason != ReleaseReason::KWinShutsDown) { - workspace()->removeUnmanaged(this); - addWorkspaceRepaint(del->visibleRect()); disownDataPassedToDeleted(); del->unrefWindow(); } diff --git a/wayland_server.cpp b/wayland_server.cpp index a1fbc197c3..80204e982e 100644 --- a/wayland_server.cpp +++ b/wayland_server.cpp @@ -101,6 +101,11 @@ WaylandServer::~WaylandServer() destroyInputMethodConnection(); } +KWaylandServer::ClientConnection *WaylandServer::xWaylandConnection() const +{ + return m_xwaylandConnection; +} + void WaylandServer::destroyInternalConnection() { emit terminatingInternalClientConnection(); @@ -603,23 +608,17 @@ int WaylandServer::createXWaylandConnection() if (!socket.connection) { return -1; } - m_xwayland.client = socket.connection; - m_xwayland.destroyConnection = connect(m_xwayland.client, &KWaylandServer::ClientConnection::disconnected, this, - [] { - qFatal("Xwayland Connection died"); - } - ); + m_xwaylandConnection = socket.connection; return socket.fd; } void WaylandServer::destroyXWaylandConnection() { - if (!m_xwayland.client) { + if (!m_xwaylandConnection) { return; } - disconnect(m_xwayland.destroyConnection); - m_xwayland.client->destroy(); - m_xwayland.client = nullptr; + m_xwaylandConnection->destroy(); + m_xwaylandConnection = nullptr; } int WaylandServer::createInputMethodConnection() diff --git a/wayland_server.h b/wayland_server.h index 55c3798f02..97f71a4183 100644 --- a/wayland_server.h +++ b/wayland_server.h @@ -184,9 +184,7 @@ public: void createInternalConnection(); void initWorkspace(); - KWaylandServer::ClientConnection *xWaylandConnection() const { - return m_xwayland.client; - } + KWaylandServer::ClientConnection *xWaylandConnection() const; KWaylandServer::ClientConnection *inputMethodConnection() const { return m_inputMethodServerConnection; } @@ -281,10 +279,7 @@ private: KWaylandServer::LinuxDmabufUnstableV1Interface *m_linuxDmabuf = nullptr; KWaylandServer::KeyboardShortcutsInhibitManagerV1Interface *m_keyboardShortcutsInhibitManager = nullptr; QSet m_linuxDmabufBuffers; - struct { - KWaylandServer::ClientConnection *client = nullptr; - QMetaObject::Connection destroyConnection; - } m_xwayland; + QPointer m_xwaylandConnection; KWaylandServer::ClientConnection *m_inputMethodServerConnection = nullptr; KWaylandServer::ClientConnection *m_screenLockerClientConnection = nullptr; struct { diff --git a/workspace.cpp b/workspace.cpp index ab6583ae41..b1592a3db1 100644 --- a/workspace.cpp +++ b/workspace.cpp @@ -125,7 +125,6 @@ Workspace::Workspace() , client_keys_client(nullptr) , global_shortcuts_disabled_for_client(false) , workspaceInit(true) - , startup(nullptr) , set_active_client_recursion(0) , block_stacking_updates(0) , m_sessionManager(new SessionManager(this)) @@ -295,7 +294,11 @@ void Workspace::init() active_client = nullptr; - initWithX11(); + // We want to have some xcb connection while tearing down X11 components. We don't really + // care if the xcb connection is broken or has an error. + connect(kwinApp(), &Application::x11ConnectionChanged, this, &Workspace::initializeX11); + connect(kwinApp(), &Application::x11ConnectionAboutToBeDestroyed, this, &Workspace::cleanupX11); + initializeX11(); Scripting::create(this); @@ -315,24 +318,22 @@ void Workspace::init() // TODO: ungrabXServer() } -void Workspace::initWithX11() +void Workspace::initializeX11() { if (!kwinApp()->x11Connection()) { - connect(kwinApp(), &Application::x11ConnectionChanged, this, &Workspace::initWithX11, Qt::UniqueConnection); return; } - disconnect(kwinApp(), &Application::x11ConnectionChanged, this, &Workspace::initWithX11); atoms->retrieveHelpers(); // first initialize the extensions Xcb::Extensions::self(); - ColorMapper *colormaps = new ColorMapper(this); - connect(this, &Workspace::clientActivated, colormaps, &ColorMapper::update); + m_colorMapper.reset(new ColorMapper(this)); + connect(this, &Workspace::clientActivated, m_colorMapper.data(), &ColorMapper::update); // Call this before XSelectInput() on the root window - startup = new KStartupInfo( - KStartupInfo::DisableKWinModule | KStartupInfo::AnnounceSilenceChanges, this); + m_startup.reset(new KStartupInfo( + KStartupInfo::DisableKWinModule | KStartupInfo::AnnounceSilenceChanges, this)); // Select windowmanager privileges selectWmInputEventMask(); @@ -455,32 +456,54 @@ void Workspace::initWithX11() activateClient(new_active_client); } +void Workspace::cleanupX11() +{ + // We expect that other components will unregister their X11 event filters after the + // connection to the X server has been lost. + + StackingUpdatesBlocker blocker(this); + + // Use stacking_order, so that kwin --replace keeps stacking order. + const QList orderedClients = ensureStackingOrder(clients); + for (X11Client *client : orderedClients) { + client->releaseWindow(true); + unconstrained_stacking_order.removeOne(client); + stacking_order.removeOne(client); + } + + for (Unmanaged *overrideRedirect : unmanaged) { + overrideRedirect->release(ReleaseReason::KWinShutsDown); + unconstrained_stacking_order.removeOne(overrideRedirect); + stacking_order.removeOne(overrideRedirect); + } + + manual_overlays.clear(); + + VirtualDesktopManager *desktopManager = VirtualDesktopManager::self(); + desktopManager->setRootInfo(nullptr); + + X11Client::cleanupX11(); + RootInfo::destroy(); + Xcb::Extensions::destroy(); + + if (xcb_connection_t *connection = kwinApp()->x11Connection()) { + xcb_delete_property(connection, kwinApp()->x11RootWindow(), atoms->kwin_running); + } + + m_colorMapper.reset(); + m_movingClientFilter.reset(); + m_startup.reset(); + m_nullFocus.reset(); + m_syncAlarmFilter.reset(); + m_wasUserInteractionFilter.reset(); + m_xStackingQueryTree.reset(); +} + Workspace::~Workspace() { blockStackingUpdates(true); - // TODO: grabXServer(); - - // Use stacking_order, so that kwin --replace keeps stacking order - const QList stack = stacking_order; - // "mutex" the stackingorder, since anything trying to access it from now on will find - // many dangeling pointers and crash - stacking_order.clear(); - - for (auto it = stack.constBegin(), end = stack.constEnd(); it != end; ++it) { - X11Client *c = qobject_cast(const_cast(*it)); - if (!c) { - continue; - } - // Only release the window - c->releaseWindow(true); - // No removeClient() is called, it does more than just removing. - // However, remove from some lists to e.g. prevent performTransiencyCheck() - // from crashing. - clients.removeAll(c); - m_allClients.removeAll(c); - } - X11Client::cleanupX11(); + cleanupX11(); if (waylandServer()) { const QList shellClients = waylandServer()->clients(); @@ -489,17 +512,10 @@ Workspace::~Workspace() } } - for (auto it = unmanaged.begin(), end = unmanaged.end(); it != end; ++it) - (*it)->release(ReleaseReason::KWinShutsDown); - for (InternalClient *client : m_internalClients) { client->destroyClient(); } - if (auto c = kwinApp()->x11Connection()) { - xcb_delete_property(c, kwinApp()->x11RootWindow(), atoms->kwin_running); - } - for (auto it = deleted.begin(); it != deleted.end();) { emit deletedRemoved(*it); it = deleted.erase(it); @@ -508,15 +524,10 @@ Workspace::~Workspace() delete RuleBook::self(); kwinApp()->config()->sync(); - RootInfo::destroy(); - delete startup; delete Placement::self(); delete client_keys_dialog; qDeleteAll(session); - // TODO: ungrabXServer(); - - Xcb::Extensions::destroy(); _self = nullptr; } @@ -642,7 +653,8 @@ void Workspace::removeClient(X11Client *c) if (c == most_recently_raised) most_recently_raised = nullptr; should_get_focus.removeAll(c); - Q_ASSERT(c != active_client); + if (c == active_client) + active_client = nullptr; if (c == last_active_client) last_active_client = nullptr; if (c == delayfocus_client) @@ -757,6 +769,7 @@ void Workspace::addShellClient(AbstractClient *client) void Workspace::removeShellClient(AbstractClient *client) { + clientHidden(client); m_allClients.removeAll(client); if (client == most_recently_raised) { most_recently_raised = nullptr; @@ -764,6 +777,9 @@ void Workspace::removeShellClient(AbstractClient *client) if (client == delayfocus_client) { cancelDelayFocus(); } + if (client == active_client) { + active_client = nullptr; + } if (client == last_active_client) { last_active_client = nullptr; } @@ -773,7 +789,6 @@ void Workspace::removeShellClient(AbstractClient *client) if (!client->shortcut().isEmpty()) { client->setShortcut(QString()); // Remove from client_keys } - clientHidden(client); emit clientRemoved(client); markXStackingOrderAsDirty(); updateStackingOrder(true); @@ -1258,7 +1273,7 @@ void Workspace::cancelDelayFocus() bool Workspace::checkStartupNotification(xcb_window_t w, KStartupInfoId &id, KStartupInfoData &data) { - return startup->checkStartup(w, id, data) == KStartupInfo::Match; + return m_startup->checkStartup(w, id, data) == KStartupInfo::Match; } /** diff --git a/workspace.h b/workspace.h index 073ef3759b..18206d5cf6 100644 --- a/workspace.h +++ b/workspace.h @@ -52,6 +52,7 @@ class Window; } class AbstractClient; +class ColorMapper; class Compositor; class Deleted; class Group; @@ -530,7 +531,8 @@ Q_SIGNALS: private: void init(); - void initWithX11(); + void initializeX11(); + void cleanupX11(); void initShortcuts(); template void initShortcut(const QString &actionName, const QString &description, const QKeySequence &shortcut, @@ -642,7 +644,8 @@ private: bool workspaceInit; - KStartupInfo* startup; + QScopedPointer m_startup; + QScopedPointer m_colorMapper; QVector workarea; // Array of workareas for virtual desktops // Array of restricted areas that window cannot be moved into diff --git a/x11client.cpp b/x11client.cpp index 8f48a26202..6f8eab45ab 100644 --- a/x11client.cpp +++ b/x11client.cpp @@ -248,8 +248,8 @@ void X11Client::releaseWindow(bool on_shutdown) m_frame.unmap(); // Destroying decoration would cause ugly visual effect destroyDecoration(); cleanGrouping(); + workspace()->removeClient(this); if (!on_shutdown) { - workspace()->removeClient(this); // Only when the window is being unmapped, not when closing down KWin (NETWM sections 5.5,5.7) info->setDesktop(0); info->setState(NET::States(), info->state()); // Reset all state flags diff --git a/xwl/xwayland.cpp b/xwl/xwayland.cpp index 7b23bf821c..0f4d63d904 100644 --- a/xwl/xwayland.cpp +++ b/xwl/xwayland.cpp @@ -4,6 +4,7 @@ Copyright 2014 Martin Gräßlin Copyright 2019 Roman Gilg +Copyright (C) 2020 Vlad Zahorodnii This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -25,6 +26,7 @@ along with this program. If not, see . #include "utils.h" #include "wayland_server.h" #include "xcbutils.h" +#include "xwayland_logging.h" #include #include @@ -32,9 +34,6 @@ along with this program. If not, see . #include #include #include -#include -#include -#include #include // system @@ -88,16 +87,7 @@ Xwayland::Xwayland(ApplicationWaylandAbstract *app, QObject *parent) Xwayland::~Xwayland() { - disconnect(m_xwaylandFailConnection); - destroyX11Connection(); - - if (m_xwaylandProcess) { - m_xwaylandProcess->terminate(); - while (m_xwaylandProcess->state() != QProcess::NotRunning) { - m_app->processEvents(QEventLoop::WaitForMoreEvents); - } - waylandServer()->destroyXWaylandConnection(); - } + stop(); s_self = nullptr; } @@ -106,7 +96,7 @@ QProcess *Xwayland::process() const return m_xwaylandProcess; } -void Xwayland::init() +void Xwayland::start() { int pipeFds[2]; if (pipe(pipeFds) != 0) { @@ -141,6 +131,7 @@ void Xwayland::init() } m_xcbConnectionFd = sx[0]; + m_displayFileDescriptor = pipeFds[0]; m_xwaylandProcess = new Process(this); m_xwaylandProcess->setProcessChannelMode(QProcess::ForwardedErrorChannel); @@ -154,33 +145,126 @@ void Xwayland::init() QStringLiteral("-rootless"), QStringLiteral("-wm"), QString::number(fd)}); - m_xwaylandFailConnection = connect(m_xwaylandProcess, &QProcess::errorOccurred, this, - [this] (QProcess::ProcessError error) { - if (error == QProcess::FailedToStart) { - std::cerr << "FATAL ERROR: failed to start Xwayland" << std::endl; - } else { - std::cerr << "FATAL ERROR: Xwayland failed, going to exit now" << std::endl; - } - Q_EMIT criticalError(1); - } - ); - const int xDisplayPipe = pipeFds[0]; - connect(m_xwaylandProcess, &QProcess::started, this, - [this, xDisplayPipe] { - QFutureWatcher *watcher = new QFutureWatcher(this); - QObject::connect(watcher, &QFutureWatcher::finished, this, &Xwayland::continueStartupWithX, Qt::QueuedConnection); - QObject::connect(watcher, &QFutureWatcher::finished, watcher, &QFutureWatcher::deleteLater, Qt::QueuedConnection); - watcher->setFuture(QtConcurrent::run(readDisplay, xDisplayPipe)); - } - ); + connect(m_xwaylandProcess, &QProcess::errorOccurred, this, &Xwayland::handleXwaylandError); + connect(m_xwaylandProcess, &QProcess::started, this, &Xwayland::handleXwaylandStarted); + connect(m_xwaylandProcess, QOverload::of(&QProcess::finished), + this, &Xwayland::handleXwaylandFinished); m_xwaylandProcess->start(); close(pipeFds[1]); } -void Xwayland::prepareDestroy() +void Xwayland::stop() { + if (!m_xwaylandProcess) { + return; + } + + // If Xwayland has crashed, we must deactivate the socket notifier and ensure that no X11 + // events will be dispatched before blocking; otherwise we will simply hang... + uninstallSocketNotifier(); + delete m_dataBridge; m_dataBridge = nullptr; + + destroyX11Connection(); + + // When the Xwayland process is finally terminated, the finished() signal will be emitted, + // however we don't actually want to process it anymore. Furthermore, we also don't really + // want to handle any errors that may occur during the teardown. + if (m_xwaylandProcess->state() != QProcess::NotRunning) { + disconnect(m_xwaylandProcess, nullptr, this, nullptr); + m_xwaylandProcess->terminate(); + m_xwaylandProcess->waitForFinished(5000); + } + delete m_xwaylandProcess; + m_xwaylandProcess = nullptr; + + waylandServer()->destroyXWaylandConnection(); // This one must be destroyed last! +} + +void Xwayland::dispatchEvents() +{ + xcb_connection_t *connection = kwinApp()->x11Connection(); + if (!connection) { + qCWarning(KWIN_XWL, "Attempting to dispatch X11 events with no connection"); + return; + } + + while (xcb_generic_event_t *event = xcb_poll_for_event(connection)) { + if (m_dataBridge->filterEvent(event)) { + free(event); + continue; + } + long result = 0; + QAbstractEventDispatcher *dispatcher = QCoreApplication::eventDispatcher(); + dispatcher->filterNativeEvent(QByteArrayLiteral("xcb_generic_event_t"), event, &result); + free(event); + } + + xcb_flush(connection); +} + +void Xwayland::installSocketNotifier() +{ + const int fileDescriptor = xcb_get_file_descriptor(kwinApp()->x11Connection()); + + m_socketNotifier = new QSocketNotifier(fileDescriptor, QSocketNotifier::Read, this); + connect(m_socketNotifier, &QSocketNotifier::activated, this, &Xwayland::dispatchEvents); + + QAbstractEventDispatcher *dispatcher = QCoreApplication::eventDispatcher(); + connect(dispatcher, &QAbstractEventDispatcher::aboutToBlock, this, &Xwayland::dispatchEvents); + connect(dispatcher, &QAbstractEventDispatcher::awake, this, &Xwayland::dispatchEvents); +} + +void Xwayland::uninstallSocketNotifier() +{ + QAbstractEventDispatcher *dispatcher = QCoreApplication::eventDispatcher(); + disconnect(dispatcher, &QAbstractEventDispatcher::aboutToBlock, this, &Xwayland::dispatchEvents); + disconnect(dispatcher, &QAbstractEventDispatcher::awake, this, &Xwayland::dispatchEvents); + + delete m_socketNotifier; + m_socketNotifier = nullptr; +} + +void Xwayland::handleXwaylandStarted() +{ + QFutureWatcher *watcher = new QFutureWatcher(this); + connect(watcher, &QFutureWatcher::finished, this, &Xwayland::continueStartupWithX); + connect(watcher, &QFutureWatcher::finished, watcher, &QFutureWatcher::deleteLater); + watcher->setFuture(QtConcurrent::run(readDisplay, m_displayFileDescriptor)); +} + +void Xwayland::handleXwaylandFinished(int exitCode) +{ + qCDebug(KWIN_XWL) << "Xwayland process has quit with exit code" << exitCode; + + // The Xwayland server has crashed... At this moment we have two choices either restart + // Xwayland or shut down all X11 related components. For now, we do the latter, we simply + // tear down everything that has any connection to X11. + stop(); +} + +void Xwayland::handleXwaylandError(QProcess::ProcessError error) +{ + switch (error) { + case QProcess::FailedToStart: + qCWarning(KWIN_XWL) << "Xwayland process failed to start"; + emit criticalError(1); + return; + case QProcess::Crashed: + qCWarning(KWIN_XWL) << "Xwayland process crashed. Shutting down X11 components"; + break; + case QProcess::Timedout: + qCWarning(KWIN_XWL) << "Xwayland operation timed out"; + break; + case QProcess::WriteError: + case QProcess::ReadError: + qCWarning(KWIN_XWL) << "An error occurred while communicating with Xwayland"; + break; + case QProcess::UnknownError: + qCWarning(KWIN_XWL) << "An unknown error has occurred in Xwayland"; + break; + } } void Xwayland::createX11Connection() @@ -199,7 +283,9 @@ void Xwayland::createX11Connection() m_app->setX11RootWindow(defaultScreen()->root); m_app->createAtoms(); - m_app->setupEventFilters(); + m_app->installNativeX11EventFilter(); + + installSocketNotifier(); // Note that it's very important to have valid x11RootWindow(), x11ScreenNumber(), and // atoms when the rest of kwin is notified about the new X11 connection. @@ -212,12 +298,18 @@ void Xwayland::destroyX11Connection() return; } + emit m_app->x11ConnectionAboutToBeDestroyed(); + Xcb::setInputFocus(XCB_INPUT_FOCUS_POINTER_ROOT); m_app->destroyAtoms(); + m_app->removeNativeX11EventFilter(); - emit m_app->x11ConnectionAboutToBeDestroyed(); xcb_disconnect(m_app->x11Connection()); + m_xcbScreen = nullptr; + m_xcbConnectionFd = -1; + m_xfixes = nullptr; + m_app->setX11Connection(nullptr); m_app->setX11ScreenNumber(-1); m_app->setX11RootWindow(XCB_WINDOW_NONE); @@ -234,22 +326,6 @@ void Xwayland::continueStartupWithX() Q_EMIT criticalError(1); return; } - QSocketNotifier *notifier = new QSocketNotifier(xcb_get_file_descriptor(xcbConn), QSocketNotifier::Read, this); - auto processXcbEvents = [this, xcbConn] { - while (auto event = xcb_poll_for_event(xcbConn)) { - if (m_dataBridge->filterEvent(event)) { - free(event); - continue; - } - long result = 0; - QThread::currentThread()->eventDispatcher()->filterNativeEvent(QByteArrayLiteral("xcb_generic_event_t"), event, &result); - free(event); - } - xcb_flush(xcbConn); - }; - connect(notifier, &QSocketNotifier::activated, this, processXcbEvents); - connect(QThread::currentThread()->eventDispatcher(), &QAbstractEventDispatcher::aboutToBlock, this, processXcbEvents); - connect(QThread::currentThread()->eventDispatcher(), &QAbstractEventDispatcher::awake, this, processXcbEvents); xcb_prefetch_extension_data(xcbConn, &xcb_xfixes_id); m_xfixes = xcb_get_extension_data(xcbConn, &xcb_xfixes_id); @@ -264,7 +340,7 @@ void Xwayland::continueStartupWithX() env.insert(QStringLiteral("DISPLAY"), QString::fromUtf8(qgetenv("DISPLAY"))); m_app->setProcessStartupEnvironment(env); - emit initialized(); + emit started(); Xcb::sync(); // Trigger possible errors, there's still a chance to abort } diff --git a/xwl/xwayland.h b/xwl/xwayland.h index df359d6c55..5d2f06ebeb 100644 --- a/xwl/xwayland.h +++ b/xwl/xwayland.h @@ -3,6 +3,7 @@ This file is part of the KDE project. Copyright 2019 Roman Gilg +Copyright (C) 2020 Vlad Zahorodnii This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -22,6 +23,9 @@ along with this program. If not, see . #include "xwayland_interface.h" +#include +#include + #include class xcb_screen_t; @@ -44,9 +48,6 @@ public: Xwayland(ApplicationWaylandAbstract *app, QObject *parent = nullptr); ~Xwayland() override; - void init(); - void prepareDestroy(); - xcb_screen_t *xcbScreen() const { return m_xcbScreen; } @@ -54,27 +55,70 @@ public: return m_xfixes; } + /** + * Returns the associated Xwayland process or @c null if the Xwayland server is inactive. + */ QProcess *process() const override; +public Q_SLOTS: + /** + * Starts the Xwayland server. + * + * This method will spawn an Xwayland process and will establish a new XCB connection to it. + * If a fatal error has occurred during the startup, the criticalError() signal is going to + * be emitted. If the Xwayland server has started successfully, the started() signal will be + * emitted. + * + * @see started(), stop() + */ + void start(); + /** + * Stops the Xwayland server. + * + * This method will destroy the existing XCB connection as well all connected X11 clients. + * + * A SIGTERM signal will be sent to the Xwayland process. If Xwayland doesn't shut down + * within a reasonable amount of time (5 seconds), a SIGKILL signal will be sent and thus + * the process will be killed for good. + * + * If the Xwayland process crashes, the server will be stopped automatically. + * + * @see start() + */ + void stop(); + Q_SIGNALS: - void initialized(); + /** + * This signal is emitted when the Xwayland server has been started successfully and it is + * ready to accept and manage X11 clients. + */ + void started(); void criticalError(int code); +private Q_SLOTS: + void dispatchEvents(); + + void handleXwaylandStarted(); + void handleXwaylandFinished(int exitCode); + void handleXwaylandError(QProcess::ProcessError error); + private: + void installSocketNotifier(); + void uninstallSocketNotifier(); + void createX11Connection(); void destroyX11Connection(); void continueStartupWithX(); DragEventReply dragMoveFilter(Toplevel *target, const QPoint &pos) override; + int m_displayFileDescriptor = -1; int m_xcbConnectionFd = -1; QProcess *m_xwaylandProcess = nullptr; - QMetaObject::Connection m_xwaylandFailConnection; - xcb_screen_t *m_xcbScreen = nullptr; const xcb_query_extension_reply_t *m_xfixes = nullptr; DataBridge *m_dataBridge = nullptr; - + QSocketNotifier *m_socketNotifier = nullptr; ApplicationWaylandAbstract *m_app; Q_DISABLE_COPY(Xwayland)