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