fix(app): terminate child process on parent crash via reaper

openspec/changes/fix-child-process-cleanup/proposal.md

@@ -0,0 +1,33 @@
+# Change: Fix child process cleanup on parent crash
+
+## Why
+
+When Goggles crashes (e.g., due to Vulkan errors or device mismatch in multi-GPU systems), the spawned child process continues running headlessly. Users are unaware the child is still running, and subsequent launches may fail due to resource conflicts (e.g., capture socket already bound).
+
+The previous approach using `PR_SET_PDEATHSIG` directly from a multi-threaded process is unreliable because the signal is tied to the **thread** that called `fork()`, not the entire process.
+
+## What Changes
+
+Introduce a `goggles-reaper` watchdog process:
+
+```
+Goggles (main process, multi-threaded)
+    ↓ fork() + exec("goggles-reaper") [SAFE: immediate exec]
+goggles-reaper (watchdog, single-threaded)
+    ↓ PR_SET_CHILD_SUBREAPER + PR_SET_PDEATHSIG
+    ↓ fork() + exec(target_app) [SAFE: no threads]
+Target Application
+```
+
+Benefits:
+- Single-threaded reaper ensures `PR_SET_PDEATHSIG` works reliably
+- `PR_SET_CHILD_SUBREAPER` catches orphaned grandchildren
+- Process group kill ensures all descendants are terminated
+
+## Impact
+
+- Affected specs: `app-window`
+- Affected code:
+  - `src/app/main.cpp` - spawn reaper instead of target app directly
+  - `src/app/reaper_main.cpp` - new watchdog process
+  - `src/app/CMakeLists.txt` - build reaper executable

openspec/changes/fix-child-process-cleanup/specs/app-window/spec.md

@@ -0,0 +1,19 @@
+## ADDED Requirements
+
+### Requirement: Child Process Death Signal
+
+The application SHALL configure spawned child processes to receive SIGTERM when the parent process terminates unexpectedly.
+
+#### Scenario: Parent crash terminates child
+
+- **GIVEN** a child process was spawned via `-- <app>` mode
+- **WHEN** the parent goggles process is killed (SIGKILL, crash, or abnormal termination)
+- **THEN** the child process SHALL receive SIGTERM automatically
+- **AND** the child process SHALL terminate
+
+#### Scenario: Parent PID 1 reparenting race
+
+- **GIVEN** a child process is being spawned
+- **WHEN** the parent dies between `fork()` and `prctl()` setup
+- **THEN** the child SHALL detect reparenting to PID 1
+- **AND** SHALL exit immediately with failure code

openspec/changes/fix-child-process-cleanup/tasks.md

@@ -0,0 +1,22 @@
+## 1. goggles-reaper Implementation
+
+- [x] 1.1 Create `src/app/reaper_main.cpp`
+- [x] 1.2 Set `PR_SET_CHILD_SUBREAPER` to adopt orphaned descendants
+- [x] 1.3 Set `PR_SET_PDEATHSIG(SIGTERM)` to detect parent death
+- [x] 1.4 Set up signal handlers for SIGTERM/SIGINT/SIGHUP to trigger child cleanup
+- [x] 1.5 Spawn target app via `fork()` + `execvp()`
+- [x] 1.6 Implement `kill_process_tree()` - recursively kill children via `/proc` scanning
+- [x] 1.7 On signal: kill all children, wait for them, then exit
+- [x] 1.8 Update `src/app/CMakeLists.txt` to build `goggles-reaper`
+
+## 2. main.cpp Changes
+
+- [x] 2.1 Use `posix_spawn()` to exec `goggles-reaper` (safe: immediate exec)
+- [x] 2.2 Pass env vars and target command as arguments to reaper
+- [x] 2.3 Remove direct `PR_SET_PDEATHSIG` usage from main.cpp
+
+## 3. Testing
+
+- [x] 3.1 Update `tests/app/test_child_death_signal.cpp` for reaper architecture
+- [x] 3.2 Manual test: kill goggles, verify target app terminates
+- [x] 3.3 Manual test: target app exits, verify goggles exits cleanly

src/app/CMakeLists.txt

@@ -28,3 +28,10 @@ set_target_properties(goggles PROPERTIES
 goggles_enable_clang_tidy(goggles)
 goggles_enable_sanitizers(goggles)
 goggles_enable_profiling(goggles)
+
+# Reaper watchdog process
+add_executable(goggles-reaper reaper_main.cpp)
+set_target_properties(goggles-reaper PROPERTIES
+    RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin
+)
+goggles_enable_clang_tidy(goggles-reaper)

src/app/main.cpp

@@ -2,13 +2,15 @@
 #include "cli.hpp"
 
 #include <SDL3/SDL.h>
+#include <array>
 #include <cerrno>
 #include <chrono>
 #include <csignal>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <exception>
+#include <filesystem>
 #include <spawn.h>
 #include <string>
 #include <string_view>
@@ -20,6 +22,17 @@
 #include <util/logging.hpp>
 #include <vector>
 
+static auto get_reaper_path() -> std::string {
+    std::array<char, 4096> exe_path{};
+    const ssize_t len = readlink("/proc/self/exe", exe_path.data(), exe_path.size() - 1);
+    if (len <= 0) {
+        return "goggles-reaper";
+    }
+    exe_path[static_cast<size_t>(len)] = '\0';
+    const std::filesystem::path exe_dir = std::filesystem::path(exe_path.data()).parent_path();
+    return (exe_dir / "goggles-reaper").string();
+}
+
 static auto spawn_target_app(const std::vector<std::string>& command,
                              const std::string& x11_display, const std::string& wayland_display,
                              uint32_t app_width, uint32_t app_height, const std::string& gpu_uuid)
@@ -71,18 +84,22 @@ static auto spawn_target_app(const std::vector<std::string>& command,
     }
     envp.push_back(nullptr);
 
+    // Build argv: goggles-reaper <target_command...>
+    const std::string reaper_path = get_reaper_path();
     std::vector<char*> argv;
-    argv.reserve(command.size() + 1);
+    argv.reserve(command.size() + 2);
+    argv.push_back(const_cast<char*>(reaper_path.c_str()));
     for (const auto& arg : command) {
         argv.push_back(const_cast<char*>(arg.c_str()));
     }
     argv.push_back(nullptr);
 
     pid_t pid = -1;
-    const int rc = posix_spawnp(&pid, argv[0], nullptr, nullptr, argv.data(), envp.data());
+    const int rc =
+        posix_spawn(&pid, reaper_path.c_str(), nullptr, nullptr, argv.data(), envp.data());
     if (rc != 0) {
         return goggles::make_error<pid_t>(goggles::ErrorCode::unknown_error,
-                                          std::string("posix_spawnp() failed: ") +
+                                          std::string("posix_spawn() failed: ") +
                                               std::strerror(rc));
     }
 

src/app/reaper_main.cpp

@@ -0,0 +1,155 @@
+// goggles-reaper: single-threaded watchdog process
+// Ensures target app and descendants are terminated when goggles dies
+
+#include <array>
+#include <cerrno>
+#include <csignal>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <dirent.h>
+#include <sys/prctl.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <vector>
+
+namespace {
+
+volatile sig_atomic_t g_should_run = 1;
+
+auto get_child_pids(pid_t parent_pid) -> std::vector<pid_t> {
+    std::vector<pid_t> pids;
+    DIR* proc_dir = opendir("/proc");
+    if (proc_dir == nullptr) {
+        return pids;
+    }
+
+    struct dirent* entry = nullptr;
+    while ((entry = readdir(proc_dir)) != nullptr) {
+        if (entry->d_type != DT_DIR || entry->d_name[0] < '0' || entry->d_name[0] > '9') {
+            continue;
+        }
+
+        std::array<char, 64> path{};
+        std::snprintf(path.data(), path.size(), "/proc/%s/stat", entry->d_name);
+
+        FILE* stat_file = std::fopen(path.data(), "r");
+        if (stat_file == nullptr) {
+            continue;
+        }
+
+        std::array<char, 512> buf{};
+        if (std::fgets(buf.data(), buf.size(), stat_file) == nullptr) {
+            std::fclose(stat_file);
+            continue;
+        }
+        std::fclose(stat_file);
+
+        const char* comm_end = std::strrchr(buf.data(), ')');
+        if (comm_end == nullptr || comm_end[1] == '\0') {
+            continue;
+        }
+
+        char state = 0;
+        pid_t ppid = -1;
+        if (std::sscanf(comm_end + 1, " %c %d", &state, &ppid) != 2) {
+            continue;
+        }
+
+        if (ppid == parent_pid) {
+            pids.push_back(static_cast<pid_t>(std::atoi(entry->d_name)));
+        }
+    }
+    closedir(proc_dir);
+    return pids;
+}
+
+auto kill_process_tree(pid_t pid, int sig) -> void {
+    const auto children = get_child_pids(pid);
+    kill(pid, sig);
+    for (const pid_t child : children) {
+        kill_process_tree(child, sig);
+    }
+}
+
+auto kill_all_children(int sig) -> void {
+    const auto children = get_child_pids(getpid());
+    for (const pid_t child : children) {
+        kill_process_tree(child, sig);
+    }
+}
+
+auto wait_all_children() -> void {
+    while (waitpid(-1, nullptr, 0) > 0 || errno == EINTR) {
+    }
+}
+
+auto signal_handler(int /*sig*/) -> void {
+    g_should_run = 0;
+}
+
+auto setup_signal_handlers() -> void {
+    struct sigaction sa{};
+    sa.sa_handler = signal_handler;
+    sigemptyset(&sa.sa_mask);
+    sigaction(SIGTERM, &sa, nullptr);
+    sigaction(SIGINT, &sa, nullptr);
+    sigaction(SIGHUP, &sa, nullptr);
+}
+
+} // namespace
+
+auto main(int argc, char** argv) -> int {
+    if (argc < 2) {
+        std::fprintf(stderr, "goggles-reaper: missing command\n");
+        return EXIT_FAILURE;
+    }
+
+    const pid_t parent_pid = getppid();
+
+    // Become subreaper - orphaned descendants reparent to us
+    prctl(PR_SET_CHILD_SUBREAPER, 1, 0, 0, 0);
+
+    // Die when parent (goggles) dies
+    prctl(PR_SET_PDEATHSIG, SIGTERM, 0, 0, 0);
+    if (getppid() != parent_pid) {
+        return EXIT_FAILURE;
+    }
+
+    setup_signal_handlers();
+
+    // Fork and exec target app
+    const pid_t child = fork();
+    if (child < 0) {
+        std::fprintf(stderr, "goggles-reaper: fork failed: %s\n", std::strerror(errno));
+        return EXIT_FAILURE;
+    }
+
+    if (child == 0) {
+        execvp(argv[1], &argv[1]);
+        std::fprintf(stderr, "goggles-reaper: exec failed: %s\n", std::strerror(errno));
+        _exit(EXIT_FAILURE);
+    }
+
+    // Wait for primary child or signal
+    int status = 0;
+    while (g_should_run != 0) {
+        const pid_t pid = waitpid(child, &status, 0);
+        if (pid == child) {
+            break;
+        }
+        if (pid == -1 && errno != EINTR) {
+            break;
+        }
+    }
+
+    // Cleanup all remaining children
+    g_should_run = 0;
+    kill_all_children(SIGKILL);
+    wait_all_children();
+
+    if (WIFEXITED(status)) {
+        return WEXITSTATUS(status);
+    }
+    return EXIT_FAILURE;
+}

tests/CMakeLists.txt

@@ -173,3 +173,16 @@ if(DEFINED ENV{CI} OR DEFINED ENV{GITHUB_ACTIONS})
     set_property(TEST goggles_auto_input_forwarding_x11 PROPERTY DISABLED TRUE)
     set_property(TEST goggles_auto_input_forwarding_wayland PROPERTY DISABLED TRUE)
 endif()
+
+# Integration test: validates child process receives SIGTERM when parent dies
+add_executable(goggles_test_child_death_signal
+    app/test_child_death_signal.cpp
+)
+
+goggles_enable_sanitizers(goggles_test_child_death_signal)
+
+add_test(NAME goggles_test_child_death_signal
+         COMMAND goggles_test_child_death_signal)
+
+set_property(TEST goggles_test_child_death_signal
+    PROPERTY WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})