From 466f2fe8ba211c039c6bdad3f52d66638aa8f658 Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Fri, 27 Jan 2023 12:29:32 +0000 Subject: [PATCH] Launch xwayland on demand This installs a socket notifier onto our xwayland socket, when a user connects we launch xwayland. The client then connections once kwin has established itself as the compositor. For a full desktop plasma session this patch effectively does nothing too useful as we still start kcminit and make xrdb calls on startup which in turn will launch X, but for the same reason this patch should be harmless now as we're still processing the xrdb calls before any clients will connect. --- autotests/integration/kwin_wayland_test.cpp | 21 ++----- autotests/integration/test_helpers.cpp | 11 +++- .../xwaylandserver_restart_test.cpp | 13 +++- src/main_wayland.cpp | 25 ++------ src/main_wayland.h | 1 - src/xwayland/xwayland.cpp | 18 +++--- src/xwayland/xwayland.h | 2 +- src/xwayland/xwaylandlauncher.cpp | 62 +++++++++++-------- src/xwayland/xwaylandlauncher.h | 8 +-- 9 files changed, 80 insertions(+), 81 deletions(-) diff --git a/autotests/integration/kwin_wayland_test.cpp b/autotests/integration/kwin_wayland_test.cpp index 0695070f5c..0eba087e46 100644 --- a/autotests/integration/kwin_wayland_test.cpp +++ b/autotests/integration/kwin_wayland_test.cpp @@ -20,6 +20,7 @@ #include "wayland_server.h" #include "workspace.h" #include "xwayland/xwayland.h" +#include "xwayland/xwaylandlauncher.h" #include @@ -143,15 +144,6 @@ void WaylandTestApplication::performStartup() connect(Compositor::self(), &Compositor::sceneCreated, this, &WaylandTestApplication::continueStartupWithScene); } -void WaylandTestApplication::finalizeStartup() -{ - if (m_xwayland) { - disconnect(m_xwayland.get(), &Xwl::Xwayland::errorOccurred, this, &WaylandTestApplication::finalizeStartup); - disconnect(m_xwayland.get(), &Xwl::Xwayland::started, this, &WaylandTestApplication::finalizeStartup); - } - notifyStarted(); -} - void WaylandTestApplication::continueStartupWithScene() { disconnect(Compositor::self(), &Compositor::sceneCreated, this, &WaylandTestApplication::continueStartupWithScene); @@ -166,15 +158,12 @@ void WaylandTestApplication::continueStartupWithScene() qFatal("Failed to initialize the Wayland server, exiting now"); } - if (operationMode() == OperationModeWaylandOnly) { - finalizeStartup(); - return; + if (operationMode() == OperationModeXwayland) { + m_xwayland = std::make_unique(this); + m_xwayland->init(); } - m_xwayland = std::make_unique(this); - connect(m_xwayland.get(), &Xwl::Xwayland::errorOccurred, this, &WaylandTestApplication::finalizeStartup); - connect(m_xwayland.get(), &Xwl::Xwayland::started, this, &WaylandTestApplication::finalizeStartup); - m_xwayland->start(); + notifyStarted(); } Test::VirtualInputDevice *WaylandTestApplication::virtualPointer() const diff --git a/autotests/integration/test_helpers.cpp b/autotests/integration/test_helpers.cpp index 8f23490288..fb0eb12144 100644 --- a/autotests/integration/test_helpers.cpp +++ b/autotests/integration/test_helpers.cpp @@ -9,7 +9,6 @@ #include #include "kwin_wayland_test.h" -#include "qtconcurrentrun.h" #if KWIN_BUILD_SCREENLOCKER #include "screenlockerwatcher.h" @@ -995,7 +994,15 @@ void XcbConnectionDeleter::operator()(xcb_connection_t *pointer) Test::XcbConnectionPtr createX11Connection() { - return Test::XcbConnectionPtr(xcb_connect(null, null)); + QFutureWatcher watcher; + QEventLoop e; + e.connect(&watcher, &QFutureWatcher::finished, &e, &QEventLoop::quit); + QFuture future = QtConcurrent::run([]() { + return xcb_connect(nullptr, nullptr); + }); + watcher.setFuture(future); + e.exec(); + return Test::XcbConnectionPtr(future.result()); } WaylandOutputManagementV2::WaylandOutputManagementV2(struct ::wl_registry *registry, int id, int version) diff --git a/autotests/integration/xwaylandserver_restart_test.cpp b/autotests/integration/xwaylandserver_restart_test.cpp index 91d257e821..42af0a798c 100644 --- a/autotests/integration/xwaylandserver_restart_test.cpp +++ b/autotests/integration/xwaylandserver_restart_test.cpp @@ -60,15 +60,22 @@ void XwaylandServerRestartTest::testRestart() Xwl::Xwayland *xwayland = static_cast(kwinApp()->xwayland()); - // Pretend that the Xwayland process has crashed by sending a SIGKILL to it. QSignalSpy startedSpy(xwayland, &Xwl::Xwayland::started); + QSignalSpy stoppedSpy(xwayland, &Xwl::Xwayland::errorOccurred); + + Test::createX11Connection(); // trigger an X11 start + QTRY_COMPARE(startedSpy.count(), 1); + + // Pretend that the Xwayland process has crashed by sending a SIGKILL to it. kwin_safe_kill(xwayland->xwaylandLauncher()->process()); - QVERIFY(startedSpy.wait()); - QCOMPARE(startedSpy.count(), 1); + + QTRY_COMPARE(stoppedSpy.count(), 1); // Check that the compositor still accepts new X11 clients. Test::XcbConnectionPtr c = Test::createX11Connection(); QVERIFY(!xcb_connection_has_error(c.get())); + + QTRY_COMPARE(startedSpy.count(), 2); const QRect rect(0, 0, 100, 200); xcb_window_t windowId = xcb_generate_id(c.get()); xcb_create_window(c.get(), XCB_COPY_FROM_PARENT, windowId, rootWindow(), diff --git a/src/main_wayland.cpp b/src/main_wayland.cpp index 763afd849e..d33ccbd3e2 100644 --- a/src/main_wayland.cpp +++ b/src/main_wayland.cpp @@ -167,25 +167,12 @@ void ApplicationWayland::continueStartupWithScene() qFatal("Failed to initialze the Wayland server, exiting now"); } - if (operationMode() == OperationModeWaylandOnly) { - finalizeStartup(); - return; - } - - m_xwayland = std::make_unique(this); - m_xwayland->xwaylandLauncher()->setListenFDs(m_xwaylandListenFds); - m_xwayland->xwaylandLauncher()->setDisplayName(m_xwaylandDisplay); - m_xwayland->xwaylandLauncher()->setXauthority(m_xwaylandXauthority); - connect(m_xwayland.get(), &Xwl::Xwayland::errorOccurred, this, &ApplicationWayland::finalizeStartup); - connect(m_xwayland.get(), &Xwl::Xwayland::started, this, &ApplicationWayland::finalizeStartup); - m_xwayland->start(); -} - -void ApplicationWayland::finalizeStartup() -{ - if (m_xwayland) { - disconnect(m_xwayland.get(), &Xwl::Xwayland::errorOccurred, this, &ApplicationWayland::finalizeStartup); - disconnect(m_xwayland.get(), &Xwl::Xwayland::started, this, &ApplicationWayland::finalizeStartup); + if (operationMode() == OperationModeXwayland) { + m_xwayland = std::make_unique(this); + m_xwayland->xwaylandLauncher()->setListenFDs(m_xwaylandListenFds); + m_xwayland->xwaylandLauncher()->setDisplayName(m_xwaylandDisplay); + m_xwayland->xwaylandLauncher()->setXauthority(m_xwaylandXauthority); + m_xwayland->init(); } startSession(); notifyStarted(); diff --git a/src/main_wayland.h b/src/main_wayland.h index 7bf5bdfc85..9ce60ab04f 100644 --- a/src/main_wayland.h +++ b/src/main_wayland.h @@ -61,7 +61,6 @@ protected: private: void continueStartupWithScene(); - void finalizeStartup(); void startSession(); void refreshSettings(const KConfigGroup &group, const QByteArrayList &names); diff --git a/src/xwayland/xwayland.cpp b/src/xwayland/xwayland.cpp index 7b987ffeba..0213794c6e 100644 --- a/src/xwayland/xwayland.cpp +++ b/src/xwayland/xwayland.cpp @@ -207,9 +207,16 @@ Xwayland::~Xwayland() m_launcher->stop(); } -void Xwayland::start() +void Xwayland::init() { - m_launcher->start(); + m_launcher->enable(); + + auto env = m_app->processStartupEnvironment(); + env.insert(QStringLiteral("DISPLAY"), m_launcher->displayName()); + env.insert(QStringLiteral("XAUTHORITY"), m_launcher->xauthority()); + qputenv("DISPLAY", m_launcher->displayName().toLatin1()); + qputenv("XAUTHORITY", m_launcher->xauthority().toLatin1()); + m_app->setProcessStartupEnvironment(env); } XwaylandLauncher *Xwayland::xwaylandLauncher() const @@ -314,13 +321,6 @@ void Xwayland::handleXwaylandReady() m_dataBridge = std::make_unique(); - auto env = m_app->processStartupEnvironment(); - env.insert(QStringLiteral("DISPLAY"), m_launcher->displayName()); - env.insert(QStringLiteral("XAUTHORITY"), m_launcher->xauthority()); - qputenv("DISPLAY", m_launcher->displayName().toLatin1()); - qputenv("XAUTHORITY", m_launcher->xauthority().toLatin1()); - m_app->setProcessStartupEnvironment(env); - connect(workspace(), &Workspace::outputOrderChanged, this, &Xwayland::updatePrimary); updatePrimary(); diff --git a/src/xwayland/xwayland.h b/src/xwayland/xwayland.h index cd07a23c8f..b12cdf7d3b 100644 --- a/src/xwayland/xwayland.h +++ b/src/xwayland/xwayland.h @@ -38,7 +38,7 @@ public: Xwayland(Application *app); ~Xwayland() override; - void start(); + void init(); XwaylandLauncher *xwaylandLauncher() const; diff --git a/src/xwayland/xwaylandlauncher.cpp b/src/xwayland/xwaylandlauncher.cpp index f9330e7383..5d8016bfd1 100644 --- a/src/xwayland/xwaylandlauncher.cpp +++ b/src/xwayland/xwaylandlauncher.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include // system @@ -69,11 +70,12 @@ void XwaylandLauncher::setXauthority(const QString &xauthority) m_xAuthority = xauthority; } -void XwaylandLauncher::start() +void XwaylandLauncher::enable() { - if (m_xwaylandProcess) { + if (m_enabled) { return; } + m_enabled = true; if (!m_listenFds.isEmpty()) { Q_ASSERT(!m_displayName.isEmpty()); @@ -86,13 +88,35 @@ void XwaylandLauncher::start() m_listenFds = m_socket->fileDescriptors(); } - startInternal(); + for (int socket : qAsConst(m_listenFds)) { + QSocketNotifier *notifier = new QSocketNotifier(socket, QSocketNotifier::Read, this); + connect(notifier, &QSocketNotifier::activated, this, [this]() { + if (!m_xwaylandProcess) { + start(); + } + }); + connect(this, &XwaylandLauncher::started, notifier, [notifier]() { + notifier->setEnabled(false); + }); + connect(this, &XwaylandLauncher::finished, notifier, [this, notifier]() { + // only reactivate if we've not shut down due to the crash count + notifier->setEnabled(m_enabled); + }); + } } -bool XwaylandLauncher::startInternal() +void XwaylandLauncher::disable() { - Q_ASSERT(!m_xwaylandProcess); + m_enabled = false; + stop(); +} +bool XwaylandLauncher::start() +{ + Q_ASSERT(m_enabled); + if (m_xwaylandProcess) { + return false; + } QVector fdsToClose; auto cleanup = qScopeGuard([&fdsToClose] { for (const int fd : std::as_const(fdsToClose)) { @@ -188,15 +212,6 @@ bool XwaylandLauncher::startInternal() return true; } -void XwaylandLauncher::stop() -{ - if (!m_xwaylandProcess) { - return; - } - - stopInternal(); -} - QString XwaylandLauncher::displayName() const { return m_displayName; @@ -217,8 +232,11 @@ QProcess *XwaylandLauncher::process() const return m_xwaylandProcess; } -void XwaylandLauncher::stopInternal() +void XwaylandLauncher::stop() { + if (!m_xwaylandProcess) { + return; + } Q_EMIT finished(); maybeDestroyReadyNotifier(); @@ -236,14 +254,6 @@ void XwaylandLauncher::stopInternal() m_xwaylandProcess = nullptr; } -void XwaylandLauncher::restartInternal() -{ - if (m_xwaylandProcess) { - stopInternal(); - } - startInternal(); -} - void XwaylandLauncher::maybeDestroyReadyNotifier() { if (m_readyNotifier) { @@ -266,17 +276,17 @@ void XwaylandLauncher::handleXwaylandFinished(int exitCode, QProcess::ExitStatus switch (options->xwaylandCrashPolicy()) { case XwaylandCrashPolicy::Restart: if (++m_crashCount <= options->xwaylandMaxCrashCount()) { - restartInternal(); + stop(); m_resetCrashCountTimer->start(std::chrono::minutes(10)); } else { qCWarning(KWIN_XWL, "Stopping Xwayland server because it has crashed %d times " "over the past 10 minutes", m_crashCount); - stop(); + disable(); } break; case XwaylandCrashPolicy::Stop: - stop(); + disable(); break; } } diff --git a/src/xwayland/xwaylandlauncher.h b/src/xwayland/xwaylandlauncher.h index eb1d785bb1..f9917ecc59 100644 --- a/src/xwayland/xwaylandlauncher.h +++ b/src/xwayland/xwaylandlauncher.h @@ -56,7 +56,9 @@ public: */ void setXauthority(const QString &xauthority); - void start(); + void enable(); + void disable(); + bool start(); void stop(); QString displayName() const; @@ -91,9 +93,6 @@ private Q_SLOTS: private: void maybeDestroyReadyNotifier(); - bool startInternal(); - void stopInternal(); - void restartInternal(); QProcess *m_xwaylandProcess = nullptr; QSocketNotifier *m_readyNotifier = nullptr; @@ -104,6 +103,7 @@ private: QString m_displayName; QString m_xAuthority; + bool m_enabled = false; int m_crashCount = 0; int m_xcbConnectionFd = -1; };