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

@@ -20,6 +20,7 @@
 #include "wayland_server.h"
 #include "workspace.h"
 #include "xwayland/xwayland.h"
+#include "xwayland/xwaylandlauncher.h"
 
 #include <KPluginMetaData>
 
@@ -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<Xwl::Xwayland>(this);
+        m_xwayland->init();
     }
 
-    m_xwayland = std::make_unique<Xwl::Xwayland>(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

autotests/integration/test_helpers.cpp

@@ -9,7 +9,6 @@
 #include <config-kwin.h>
 
 #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<xcb_connection_t *> watcher;
+    QEventLoop e;
+    e.connect(&watcher, &QFutureWatcher<xcb_connection_t *>::finished, &e, &QEventLoop::quit);
+    QFuture<xcb_connection_t *> 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)

autotests/integration/xwaylandserver_restart_test.cpp

@@ -60,15 +60,22 @@ void XwaylandServerRestartTest::testRestart()
 
     Xwl::Xwayland *xwayland = static_cast<Xwl::Xwayland *>(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(),

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<Xwl::Xwayland>(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<Xwl::Xwayland>(this);
+        m_xwayland->xwaylandLauncher()->setListenFDs(m_xwaylandListenFds);
+        m_xwayland->xwaylandLauncher()->setDisplayName(m_xwaylandDisplay);
+        m_xwayland->xwaylandLauncher()->setXauthority(m_xwaylandXauthority);
+        m_xwayland->init();
     }
     startSession();
     notifyStarted();

src/main_wayland.h

@@ -61,7 +61,6 @@ protected:
 
 private:
     void continueStartupWithScene();
-    void finalizeStartup();
     void startSession();
     void refreshSettings(const KConfigGroup &group, const QByteArrayList &names);
 

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<DataBridge>();
 
-    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();
 

src/xwayland/xwayland.h

@@ -38,7 +38,7 @@ public:
     Xwayland(Application *app);
     ~Xwayland() override;
 
-    void start();
+    void init();
 
     XwaylandLauncher *xwaylandLauncher() const;
 

src/xwayland/xwaylandlauncher.cpp

@@ -29,6 +29,7 @@
 #include <QHostInfo>
 #include <QRandomGenerator>
 #include <QScopeGuard>
+#include <QSocketNotifier>
 #include <QTimer>
 
 // 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<int> 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;
     }
 }

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;
 };