From 05a3e2bad9a3baf27293f0b5ed4bef4f38952804 Mon Sep 17 00:00:00 2001 From: Aleix Pol Gonzalez Date: Tue, 2 Jan 2024 18:34:26 +0100 Subject: [PATCH] systemd: Set up a watchdog Allows to notify systemd whether kwin is still running and possibly restart the service if it stops responding. Use Type=notify-reload to watch the kwin service. This will make it so we receive SIGHUP rather than SIGTERM on the wrapper which we can handle gracefully and stop the kwin process and restart as expected. https://www.freedesktop.org/software/systemd/man/latest/systemd.service.html Signed-off-by: Victoria Fischer --- CMakeLists.txt | 3 + plasma-kwin_wayland.service.in | 4 ++ src/CMakeLists.txt | 12 ++++ src/helpers/wayland_wrapper/kwin_wrapper.cpp | 50 +++++++++---- src/watchdog.cpp | 74 ++++++++++++++++++++ 5 files changed, 130 insertions(+), 13 deletions(-) create mode 100644 src/watchdog.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index c4e7066a43..f2d1c6b03c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -356,6 +356,9 @@ set_package_properties(QAccessibilityClient6 PROPERTIES ) set(HAVE_ACCESSIBILITY ${QAccessibilityClient6_FOUND}) +pkg_check_modules(libsystemd IMPORTED_TARGET libsystemd) +add_feature_info(libsystemd libsystemd_FOUND "Required for setting up the service watchdog") + option(KWIN_BUILD_GLOBALSHORTCUTS "Enable building of KWin with global shortcuts support" ON) if(KWIN_BUILD_GLOBALSHORTCUTS) find_package(KGlobalAccelD REQUIRED) diff --git a/plasma-kwin_wayland.service.in b/plasma-kwin_wayland.service.in index 6db09b26ba..d5333d901b 100644 --- a/plasma-kwin_wayland.service.in +++ b/plasma-kwin_wayland.service.in @@ -3,6 +3,10 @@ Description=KDE Window Manager PartOf=graphical-session.target [Service] +Type=notify-reload ExecStart=@CMAKE_INSTALL_FULL_BINDIR@/kwin_wayland_wrapper --xwayland BusName=org.kde.KWinWrapper Slice=session.slice +WatchdogSec=15s +NotifyAccess=all +WatchdogSignal=SIGHUP diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 1f8bee962a..cb3d70105c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -377,6 +377,18 @@ install(TARGETS kwin_x11 ${KDE_INSTALL_TARGETS_DEFAULT_ARGS}) add_executable(kwin_wayland main_wayland.cpp) +if(TARGET PkgConfig::libsystemd) + ecm_qt_declare_logging_category(kwin_wayland + HEADER watchdoglogging.h + IDENTIFIER KWIN_WATCHDOG + CATEGORY_NAME kwin_watchdog + DEFAULT_SEVERITY Info + ) + + target_sources(kwin_wayland PRIVATE watchdog.cpp) + target_link_libraries(kwin_wayland PkgConfig::libsystemd) +endif() + target_link_libraries(kwin_wayland kwin KWinXwaylandServerModule diff --git a/src/helpers/wayland_wrapper/kwin_wrapper.cpp b/src/helpers/wayland_wrapper/kwin_wrapper.cpp index 9f08261faf..1a88ad38af 100644 --- a/src/helpers/wayland_wrapper/kwin_wrapper.cpp +++ b/src/helpers/wayland_wrapper/kwin_wrapper.cpp @@ -36,13 +36,18 @@ #include "xauthority.h" #include "xwaylandsocket.h" +using namespace std::chrono_literals; + class KWinWrapper : public QObject { Q_OBJECT public: KWinWrapper(QObject *parent); ~KWinWrapper(); + void run(); + void restart(); + void terminate(std::chrono::milliseconds timeout); private: wl_socket *m_socket; @@ -52,11 +57,14 @@ private: std::unique_ptr m_xwlSocket; QTemporaryFile m_xauthorityFile; + const std::chrono::microseconds m_watchdogInterval; + bool m_watchdogIntervalOk; }; KWinWrapper::KWinWrapper(QObject *parent) : QObject(parent) , m_kwinProcess(new QProcess(this)) + , m_watchdogInterval(std::chrono::microseconds(qEnvironmentVariableIntValue("WATCHDOG_USEC", &m_watchdogIntervalOk) / 2)) { m_socket = wl_socket_create(); if (!m_socket) { @@ -82,13 +90,7 @@ KWinWrapper::KWinWrapper(QObject *parent) KWinWrapper::~KWinWrapper() { wl_socket_destroy(m_socket); - if (m_kwinProcess) { - disconnect(m_kwinProcess, nullptr, this, nullptr); - m_kwinProcess->terminate(); - m_kwinProcess->waitForFinished(); - m_kwinProcess->kill(); - m_kwinProcess->waitForFinished(); - } + terminate(30s); } void KWinWrapper::run() @@ -147,7 +149,6 @@ void KWinWrapper::run() env.insert("XAUTHORITY", m_xauthorityFile.fileName()); } } - auto envSyncJob = new KUpdateLaunchEnvironmentJob(env); connect(envSyncJob, &KUpdateLaunchEnvironmentJob::finished, this, []() { // The service name is merely there to indicate to the world that we're up and ready with all envs exported @@ -155,21 +156,44 @@ void KWinWrapper::run() }); } +void KWinWrapper::terminate(std::chrono::milliseconds timeout) +{ + if (m_kwinProcess) { + disconnect(m_kwinProcess, nullptr, this, nullptr); + m_kwinProcess->terminate(); + m_kwinProcess->waitForFinished(timeout.count() / 2); + if (m_kwinProcess->state() != QProcess::NotRunning) { + m_kwinProcess->kill(); + m_kwinProcess->waitForFinished(timeout.count() / 2); + } + } +} + +void KWinWrapper::restart() +{ + terminate(m_watchdogIntervalOk ? std::chrono::duration_cast(m_watchdogInterval) : 30000ms); + m_kwinProcess->start(); +} + int main(int argc, char **argv) { QCoreApplication app(argc, argv); app.setQuitLockEnabled(false); // don't exit when the first KJob finishes KSignalHandler::self()->watchSignal(SIGTERM); - QObject::connect(KSignalHandler::self(), &KSignalHandler::signalReceived, &app, [&app](int signal) { - if (signal == SIGTERM) { - app.quit(); - } - }); + KSignalHandler::self()->watchSignal(SIGHUP); KWinWrapper wrapper(&app); wrapper.run(); + QObject::connect(KSignalHandler::self(), &KSignalHandler::signalReceived, &app, [&app, &wrapper](int signal) { + if (signal == SIGTERM) { + app.quit(); + } else if (signal == SIGHUP) { // The systemd service will issue SIGHUP when it's locked up so that we can restarted + wrapper.restart(); + } + }); + return app.exec(); } diff --git a/src/watchdog.cpp b/src/watchdog.cpp new file mode 100644 index 0000000000..f33b548496 --- /dev/null +++ b/src/watchdog.cpp @@ -0,0 +1,74 @@ +/* + KWin - the KDE window manager + This file is part of the KDE project. + + SPDX-FileCopyrightText: 2024 Aleix Pol i Gonzalez + + SPDX-License-Identifier: GPL-2.0-or-later +*/ + +#include "watchdoglogging.h" +#include +#include +#include + +class Watchdog : public QObject +{ + Q_OBJECT +public: + Watchdog(QObject *parent) + : QObject(parent) + { + bool ok; + // by 1/2 to have some margin to hit the watchdog interval timely, as recommended by the docs + const std::chrono::microseconds watchdogIntervalInUs(qEnvironmentVariableIntValue("WATCHDOG_USEC", &ok) / 2); + if (!ok) { + qCDebug(KWIN_WATCHDOG) << "Watchdog: disabled, not running on a systemd environment or watchdog is not set up. No WATCHDOG_USEC."; + deleteLater(); + return; + } + m_onBehalf = qEnvironmentVariableIntValue("WATCHDOG_PID", &ok); + if (!ok) { + qCDebug(KWIN_WATCHDOG) << "Watchdog: disabled, not running on a systemd environment or watchdog is not set up. No WATCHDOG_PID."; + deleteLater(); + return; + } + qunsetenv("WATCHDOG_USEC"); + qunsetenv("WATCHDOG_PID"); + auto t = new QTimer(this); + t->setInterval(std::chrono::duration_cast(watchdogIntervalInUs)); + t->setSingleShot(false); + + // If service Type=notify the service is only considered ready once we send this + const int notified = sd_pid_notify(m_onBehalf, 0, "READY=1"); + if (notified < 1) { + qCCritical(KWIN_WATCHDOG) << "Watchdog: failed to set the process as ready, systemd will probably kill the process soon. :'(" << notified << strerror(-notified); + } else { + qCInfo(KWIN_WATCHDOG) << "Watchdog: enabled. Interval:" << watchdogIntervalInUs << t->interval() << qgetenv("NOTIFY_SOCKET") << geteuid(); + } + + const auto bark = [this]() { + const int ret = sd_pid_notify(m_onBehalf, 0, "WATCHDOG=1"); + if (ret < 1) { + qCCritical(KWIN_WATCHDOG) << "Watchdog: failed to bark, systemd will probably kill the process soon. :'(" << ret << strerror(-ret); + } else { + qCDebug(KWIN_WATCHDOG) << "Watchdog: bark!"; + } + }; + bark(); + connect(t, &QTimer::timeout, this, bark); + t->start(); + } + +private: + pid_t m_onBehalf = 0; +}; + +static void setupWatchdog() +{ + new Watchdog(QCoreApplication::instance()); +} + +Q_COREAPP_STARTUP_FUNCTION(setupWatchdog) + +#include "watchdog.moc"