Introduce helper to restart kwin on crash exit

Right now when kwin exits, the user is taken directly back to the login
screen. The login session exits, so all processes then are killed by the
session.

This patchset introduces a mechanism to safely restart kwin. The socket
(typically wayland-0) remains alive and persistent across restarts. This
means if any process reconnects through it's own mechanism or a crash
restart handler the socket appears to work, and blocks until the new
kwin restarts.

This makes it secure and race free.

If the screen was locked at the time kwin went down, this is also
secure. Kwin now checks the status from logind at the time of launch, so
will immediately restore a locked state before any other rendering.

helpers/CMakeLists.txt

@@ -1 +1,2 @@
 add_subdirectory(killer)
+add_subdirectory(wayland_wrapper)

helpers/wayland_wrapper/CMakeLists.txt

@@ -0,0 +1,3 @@
+add_executable(kwin_wayland_wrapper kwin_wrapper.c wl-socket.c)
+set_property(TARGET kwin_wayland_wrapper PROPERTY C_STANDARD 11)
+install(TARGETS kwin_wayland_wrapper ${INSTALL_TARGETS_DEFAULT_ARGS})

helpers/wayland_wrapper/kwin_wrapper.c

@@ -0,0 +1,118 @@
+/*
+    KWin - the KDE window manager
+    This file is part of the KDE project.
+
+    SPDX-FileCopyrightText: 2020 <davidedmundson@kde.org>
+
+    SPDX-License-Identifier: GPL-2.0-or-later
+*/
+
+/**
+ * This tiny executable creates a socket, then starts kwin passing it the FD to the wayland socket.
+ * The WAYLAND_DISPLAY environment variable gets set here and passed to all spawned kwin instances.
+ * On any non-zero kwin exit kwin gets restarted.
+ *
+ * After restart kwin is relaunched but now with the KWIN_RESTART_COUNT env set to an incrementing counter
+ *
+ * It's somewhat  akin to systemd socket activation, but we also need the lock file, finding the next free socket
+ * and so on, hence our own binary.
+ *
+ * Usage kwin_wayland_wrapper [argForKwin] [argForKwin] ...
+ */
+
+#include "wl-socket.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/un.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+char *old_wayland_env = NULL;
+
+#define WAYLAND_ENV_NAME "WAYLAND_DISPLAY"
+
+pid_t launch_kwin(struct wl_socket *socket, int argc, char **argv)
+{
+    printf("Launching kwin\n");
+    pid_t pid = fork();
+    if (pid == 0) { /* child process */
+        char fdString[5]; // long enough string to contain what is probably a 1 digit number.
+        snprintf(fdString, sizeof(fdString) - 1, "%d", wl_socket_get_fd(socket));
+
+        char **args = calloc(argc + 6, sizeof(char *));
+        uint pos = 0;
+        args[pos++] = (char *)"kwin_wayland"; //process name is the first argument by convention
+        args[pos++] = (char *)"--wayland_fd";
+        args[pos++] = fdString;
+
+        // for running in nested wayland, pass the original socket name
+        if (old_wayland_env) {
+            args[pos++] = (char *)"--wayland-display";
+            args[pos++] = old_wayland_env;
+        }
+
+        //copy passed args
+        for (int i = 0; i < argc; i++) {
+            args[pos++] = argv[i];
+        }
+
+        args[pos++] = NULL;
+
+        execvpe("kwin_wayland", args, environ);
+        free(args);
+        exit(127); /* if exec fails */
+    } else {
+        return pid;
+    }
+}
+
+int main(int argc, char **argv)
+{
+    struct wl_socket *socket = wl_socket_create();
+    if (!socket) {
+        return -1;
+    }
+
+    // copy the old WAYLAND_DISPLAY as we are about to overwrite it and kwin may need it
+    if (getenv(WAYLAND_ENV_NAME)) {
+        old_wayland_env = strdup(getenv(WAYLAND_ENV_NAME));
+    }
+
+    setenv(WAYLAND_ENV_NAME, wl_socket_get_display_name(socket), 1);
+
+    int crashCount = 0;
+    while (crashCount < 10) {
+        int status;
+
+        if (crashCount > 0) {
+            char restartCountEnv[3];
+            snprintf(restartCountEnv, sizeof(restartCountEnv) - 1, "%d", crashCount);
+            setenv("KWIN_RESTART_COUNT", restartCountEnv, 1);
+        }
+
+        // forward our own arguments, but drop the first, as that will be our own executable name
+        pid_t pid = launch_kwin(socket, argc - 1, &argv[1]);
+        if (pid < 0) {
+            // failed to launch kwin, we can just quit immediately
+            break;
+        }
+
+        waitpid(pid, &status, 0); /* wait for child */
+
+        if (WIFEXITED(status)) {
+            int exit_status = WEXITSTATUS(status);
+            fprintf(stderr, "Kwin exited with code %d\n", exit_status);
+            break;
+        } else if (WIFSIGNALED(status)) {
+            // we crashed! Let's go again!
+            pid = 0;
+            fprintf(stderr, "Compositor crashed, respawning\n");
+        }
+        crashCount++;
+    }
+
+    wl_socket_destroy(socket);
+    free(old_wayland_env);
+    return 0;
+}

helpers/wayland_wrapper/wl-socket.c

@@ -0,0 +1,170 @@
+/*
+    KWin - the KDE window manager
+    This file is part of the KDE project.
+
+    SPDX-FileCopyrightText: 2020 <davidedmundson@kde.org>
+    SPDX-FileCopyrightText: 2008 Kristian Høgsberg
+
+    SPDX-License-Identifier: GPL-2.0-or-later
+*/
+
+#include <assert.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/file.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/un.h>
+#include <unistd.h>
+
+/* This is the size of the char array in struct sock_addr_un.
+ * No Wayland socket can be created with a path longer than this,
+ * including the null terminator.
+ */
+#ifndef UNIX_PATH_MAX
+#define UNIX_PATH_MAX 108
+#endif
+
+#define LOCK_SUFFIX ".lock"
+#define LOCK_SUFFIXLEN 5
+
+struct wl_socket {
+    int fd;
+    int fd_lock;
+    struct sockaddr_un addr;
+    char lock_addr[UNIX_PATH_MAX + LOCK_SUFFIXLEN];
+    char display_name[16];
+};
+
+static struct wl_socket *wl_socket_alloc(void)
+{
+    struct wl_socket *s;
+
+    s = malloc(sizeof *s);
+    if (!s)
+        return NULL;
+
+    s->fd = -1;
+    s->fd_lock = -1;
+
+    return s;
+}
+
+static int wl_socket_lock(struct wl_socket *socket)
+{
+    struct stat socket_stat;
+
+    snprintf(socket->lock_addr, sizeof socket->lock_addr, "%s%s", socket->addr.sun_path, LOCK_SUFFIX);
+
+    // differing from wl_display, we don't set O_CLOEXEC so that we can pass to the child
+    socket->fd_lock = open(socket->lock_addr, O_CREAT | O_RDWR, (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP));
+
+    if (socket->fd_lock < 0) {
+        printf("unable to open lockfile %s check permissions\n", socket->lock_addr);
+        goto err;
+    }
+
+    if (flock(socket->fd_lock, LOCK_EX | LOCK_NB) < 0) {
+        printf("unable to lock lockfile %s, maybe another compositor is running\n", socket->lock_addr);
+        goto err_fd;
+    }
+
+    if (lstat(socket->addr.sun_path, &socket_stat) < 0) {
+        if (errno != ENOENT) {
+            printf("did not manage to stat file %s\n", socket->addr.sun_path);
+            goto err_fd;
+        }
+    } else if (socket_stat.st_mode & S_IWUSR || socket_stat.st_mode & S_IWGRP) {
+        unlink(socket->addr.sun_path);
+    }
+
+    return 0;
+err_fd:
+    close(socket->fd_lock);
+    socket->fd_lock = -1;
+err:
+    *socket->lock_addr = 0;
+    /* we did not set this value here, but without lock the
+     * socket won't be created anyway. This prevents the
+     * wl_socket_destroy from unlinking already existing socket
+     * created by other compositor */
+    *socket->addr.sun_path = 0;
+
+    return -1;
+}
+
+void wl_socket_destroy(struct wl_socket *s)
+{
+    if (s->addr.sun_path[0])
+        unlink(s->addr.sun_path);
+    if (s->fd >= 0)
+        close(s->fd);
+    if (s->lock_addr[0])
+        unlink(s->lock_addr);
+    if (s->fd_lock >= 0)
+        close(s->fd_lock);
+
+    free(s);
+}
+
+const char *wl_socket_get_display_name(struct wl_socket *s)
+{
+    return s->display_name;
+}
+
+int wl_socket_get_fd(struct wl_socket *s)
+{
+    return s->fd;
+}
+
+struct wl_socket *wl_socket_create()
+{
+    struct wl_socket *s;
+    int displayno = 0;
+    int name_size;
+
+    /* A reasonable number of maximum default sockets. If
+     * you need more than this, use the explicit add_socket API. */
+    const int MAX_DISPLAYNO = 32;
+    const char *runtime_dir = getenv("XDG_RUNTIME_DIR");
+    if (!runtime_dir) {
+        printf("XDG_RUNTIME_DIR not set");
+        return NULL;
+    }
+
+    s = wl_socket_alloc();
+    if (s == NULL)
+        return NULL;
+
+    do {
+        snprintf(s->display_name, sizeof s->display_name, "wayland-%d", displayno);
+        s->addr.sun_family = AF_LOCAL;
+        name_size = snprintf(s->addr.sun_path, sizeof s->addr.sun_path, "%s/%s", runtime_dir, s->display_name) + 1;
+        assert(name_size > 0);
+
+        if (name_size > (int)sizeof s->addr.sun_path) {
+            goto fail;
+        }
+
+        if (wl_socket_lock(s) < 0)
+            continue;
+
+        s->fd = socket(PF_LOCAL, SOCK_STREAM, 0);
+
+        int size = SUN_LEN(&s->addr);
+        int ret = bind(s->fd, &s->addr, size);
+        if (ret < 0) {
+            goto fail;
+        }
+        ret = listen(s->fd, 128);
+        if (ret < 0) {
+            goto fail;
+        }
+        return s;
+    } while (displayno++ < MAX_DISPLAYNO);
+
+fail:
+    wl_socket_destroy(s);
+    return NULL;
+}

helpers/wayland_wrapper/wl-socket.h

@@ -0,0 +1,31 @@
+/*
+    KWin - the KDE window manager
+    This file is part of the KDE project.
+
+    SPDX-FileCopyrightText: 2020 <davidedmundson@kde.org>
+
+    SPDX-License-Identifier: GPL-2.0-or-later
+*/
+
+#pragma once
+
+/**
+ * Allocate and create a socket
+ * It is bound and accepted
+ */
+struct wl_socket *wl_socket_create();
+
+/**
+ * Returns the file descriptor for the socket
+ */
+int wl_socket_get_fd(struct wl_socket *);
+
+/**
+ * Returns the name of the socket, i.e "wayland-0"
+ */
+char *wl_socket_get_display_name(struct wl_socket *);
+
+/**
+ * Cleanup resources and close the FD
+ */
+void wl_socket_destroy(struct wl_socket *socket);

main_wayland.cpp

@@ -317,9 +317,19 @@ static const QString s_hwcomposerPlugin = QStringLiteral("KWinWaylandHwcomposerB
 #endif
 static const QString s_virtualPlugin = QStringLiteral("KWinWaylandVirtualBackend");
 
-static QString automaticBackendSelection()
+
+enum SpawnMode {
+    Standalone,
+    ReusedSocket
+};
+
+static QString automaticBackendSelection(SpawnMode spawnMode)
 {
-    if (qEnvironmentVariableIsSet("WAYLAND_DISPLAY")) {
+    /* WAYLAND_DISPLAY is set by the kwin_wayland_wrapper, so we can't use it for automatic detection.
+    * If kwin_wayland_wrapper is used nested on wayland, we won't be in this path as
+    * it explicitly sets '--socket' which means a backend is set and we won't be in this path anyway
+    */
+    if (qEnvironmentVariableIsSet("WAYLAND_DISPLAY") && spawnMode == Standalone) {
         return s_waylandPlugin;
     }
     if (qEnvironmentVariableIsSet("DISPLAY")) {
@@ -497,10 +507,16 @@ int main(int argc, char * argv[])
                                     QStringLiteral("count"));
     outputCountOption.setDefaultValue(QString::number(1));
 
+    QCommandLineOption waylandSocketFdOption(QStringLiteral("wayland_fd"),
+                                    i18n("Wayland socket to use for incoming connections."),
+                                    QStringLiteral("wayland_fd"));
+
     QCommandLineParser parser;
     a.setupCommandLine(&parser);
     parser.addOption(xwaylandOption);
     parser.addOption(waylandSocketOption);
+    parser.addOption(waylandSocketFdOption);
+
     if (hasX11Option) {
         parser.addOption(x11DisplayOption);
     }
@@ -653,7 +669,7 @@ int main(int argc, char * argv[])
 
     if (pluginName.isEmpty()) {
         std::cerr << "No backend specified through command line argument, trying auto resolution" << std::endl;
-        pluginName = KWin::automaticBackendSelection();
+        pluginName = KWin::automaticBackendSelection(parser.isSet(waylandSocketFdOption) ? KWin::ReusedSocket : KWin::Standalone);
     }
 
     auto pluginIt = std::find_if(availablePlugins.begin(), availablePlugins.end(),
@@ -678,7 +694,29 @@ int main(int argc, char * argv[])
     if (parser.isSet(noGlobalShortcutsOption)) {
         flags |= KWin::WaylandServer::InitializationFlag::NoGlobalShortcuts;
     }
-    if (!server->init(parser.value(waylandSocketOption), flags)) {
+
+
+    if (parser.isSet(waylandSocketFdOption)) {
+        bool ok;
+        int fd = parser.value(waylandSocketFdOption).toInt(&ok);
+        if (ok ) {
+            // make sure we don't leak this FD to children
+            fcntl(fd, F_SETFD, O_CLOEXEC);
+            server->display()->addSocketFileDescriptor(fd);
+        } else {
+            std::cerr << "FATAL ERROR: could not parse socket FD" << std::endl;
+            return 1;
+        }
+    } else {
+        const QString socketName = parser.value(waylandSocketOption);
+        // being empty is fine here, addSocketName will automatically pick one
+        if (!server->display()->addSocketName(socketName)) {
+            std::cerr << "FATAL ERROR: could not add wayland socket " << qPrintable(socketName) << std::endl;
+            return 1;
+        }
+    }
+
+    if (!server->init(flags)) {
         std::cerr << "FATAL ERROR: could not create Wayland server" << std::endl;
         return 1;
     }
@@ -698,7 +736,9 @@ int main(int argc, char * argv[])
     a.platform()->setInitialOutputCount(outputCount);
 
     QObject::connect(&a, &KWin::Application::workspaceCreated, server, &KWin::WaylandServer::initWorkspace);
-    environment.insert(QStringLiteral("WAYLAND_DISPLAY"), server->socketName());
+    if (!server->socketName().isEmpty()) {
+        environment.insert(QStringLiteral("WAYLAND_DISPLAY"), server->socketName());
+    }
     a.setProcessStartupEnvironment(environment);
     a.setStartXwayland(parser.isSet(xwaylandOption));
     a.setApplicationsToStart(parser.positionalArguments());

wayland_server.cpp

@@ -332,11 +332,15 @@ bool WaylandServer::start()
 
 bool WaylandServer::init(const QString &socketName, InitializationFlags flags)
 {
-    m_initFlags = flags;
-    m_display = new KWinDisplay(this);
     if (!m_display->addSocketName(socketName)) {
         return false;
     }
+    return init(flags);
+}
+
+bool WaylandServer::init(InitializationFlags flags)
+{
+    m_initFlags = flags;
     m_compositor = new CompositorInterface(m_display, m_display);
     connect(m_compositor, &CompositorInterface::surfaceCreated, this,
         [this] (SurfaceInterface *surface) {
@@ -347,7 +351,7 @@ bool WaylandServer::init(const QString &socketName, InitializationFlags flags)
                 return;
             }
             if (surface->client() != xWaylandConnection()) {
-                // setting surface is only relevat for Xwayland clients
+                // setting surface is only relevant for Xwayland clients
                 return;
             }
 

wayland_server.h

@@ -87,7 +87,9 @@ public:
     Q_DECLARE_FLAGS(InitializationFlags, InitializationFlag)
 
     ~WaylandServer() override;
-    bool init(const QString &socketName = QString(), InitializationFlags flags = InitializationFlag::NoOptions);
+    bool init(const QString &socketName, InitializationFlags flags = InitializationFlag::NoOptions);
+    bool init(InitializationFlags flags = InitializationFlag::NoOptions);
+
     bool start();
     void terminateClientConnections();