From 5de348f86076fa6c64db6859752c6ceafe52fe6b Mon Sep 17 00:00:00 2001 From: mweidle73 Date: Sat, 17 Oct 2026 15:51:14 +0200 Subject: [PATCH 1/5] fix(signals): Reap requests before manager exit Wait for the sole executor to acknowledge complete request reaping before catchable shutdown exits the manager. Retain that ownership after the leader group identity is retired, with a fixed fail-stop bound if the executor cannot finish. Keep executor-side fail-stop nonwaiting to avoid awaiting itself. Do not alter the wire format, request results or ordinary spawn path. Add delayed/stalled C reap barriers and a built-manager subreaper oracle for both request kinds and termination signals. The original manager and missing-wait/deadline mutations fail the new assertions. Supply Python through the CI image, not runtime package installation. --- .github/ci/Dockerfile.trixie | 1 + CHANGELOG.md | 7 ++ Makefile | 1 + README.md | 8 +- WORK_QUEUE.md | 16 +++ doc/lifecycle-v1.md | 17 ++- src/spawn-posix.c | 62 ++++++++- src/spawn-posix.h | 5 +- tests/check_signal_reaping.py | 224 +++++++++++++++++++++++++++++++++ tests/check_signal_shutdown.sh | 6 +- tests/spawn-posix-tests.c | 125 +++++++++++++++++- tools/spawn-signals.adb | 3 +- 12 files changed, 456 insertions(+), 19 deletions(-) create mode 100644 tests/check_signal_reaping.py diff --git a/.github/ci/Dockerfile.trixie b/.github/ci/Dockerfile.trixie index 046dbcf..2cf3ccc 100644 --- a/.github/ci/Dockerfile.trixie +++ b/.github/ci/Dockerfile.trixie @@ -14,6 +14,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ libc6-dev \ make \ pandoc \ + python3 \ && rm -rf /var/lib/apt/lists/* WORKDIR /work diff --git a/CHANGELOG.md b/CHANGELOG.md index 90428c5..3a6aa91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,13 @@ Gitlink even when the two Spawn Manager commits are not direct ancestors. ## [Unreleased] +### Fixed + +- Catchable manager shutdown now waits for its executor to reap request + descendants before exiting, including after the leader identity has been + retired. A stalled reap acknowledgement fail-stops within a fixed bound. + Shell and structured requests retain the existing wire format and results. + ### Added - Added public contribution guidance and a private vulnerability-reporting diff --git a/Makefile b/Makefile index 43022fb..48493e2 100644 --- a/Makefile +++ b/Makefile @@ -59,6 +59,7 @@ tests: $(POSIX_TEST) $(PROTOCOL_FAILURE_MANAGER) spawn_tests spawn_manager @$(OBJDIR)/spawn_manager 47 $(OBJDIR)/spawn_manager_min_spawn & @$(OBJDIR)/test_runner @tests/check_signal_shutdown.sh "$(CURDIR)" + @python3 tests/check_signal_reaping.py "$(CURDIR)/obj/spawn_manager" 1 @tests/check_adaflags.sh "$(CURDIR)" @tests/check_install.sh "$(CURDIR)" diff --git a/README.md b/README.md index 388d6f1..abecd7b 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ Building Spawn Manager requires: - GNU Make. The tests additionally require -[Ahven](https://www.stronglytyped.org/ahven/), Bash +[Ahven](https://www.stronglytyped.org/ahven/), Bash, Python 3 and standard Linux process utilities. Coverage needs `lcov` and `genhtml`. Building the optional HTML documentation requires [Pandoc](https://pandoc.org/). @@ -119,6 +119,12 @@ its process and log ordering remains deterministic: make tests ``` +The signal integration check uses its own fallback subreaper. It detects +children abandoned by a terminated manager even when a container init would +otherwise collect them. Shell and structured requests exercise both SIGINT +and SIGTERM; direct C fault tests also cover delayed and stalled reaping. +A fixture self-test checks recovery across newly adopted generations. + Run the comparative execution benchmarks: ```sh diff --git a/WORK_QUEUE.md b/WORK_QUEUE.md index 1386880..047975d 100644 --- a/WORK_QUEUE.md +++ b/WORK_QUEUE.md @@ -17,6 +17,22 @@ define acceptance evidence before starting an item. ## Accepted follow-ups +### Catchable shutdown reaping + +Implemented on the current topic; upstream acceptance remains pending. +SIGINT/SIGTERM used to kill the request group and immediately exit the manager, +which could abandon killed descendants before the executor reaped them. The +shutdown hook now awaits a bounded acknowledgement from the sole reaper, +including the interval after leader retirement. It does not change the wire +protocol, pool API or ordinary request execution path. + +Direct C barriers pin delayed and stalled descendant reaping. A built-manager +check becomes the nearest fallback subreaper and rejects adopted children for +both request kinds and termination signals, independent of container init. +Fixture recovery also pins adoption across killed generations. +Carry this correction into the shared-output topic before caller acceptance; +the downstream Make migration does not silently change its dependency. + ### Neutral GitHub organization The canonical repository initially remains under its current personal GitHub diff --git a/doc/lifecycle-v1.md b/doc/lifecycle-v1.md index b432b1e..537a3bb 100644 --- a/doc/lifecycle-v1.md +++ b/doc/lifecycle-v1.md @@ -109,9 +109,20 @@ pre-exec `Close_Descriptors` stage instead of proceeding to `execve`. The Work Queue retains the missing unlimited-limit fault injection and boundedness evidence. -The attached manager signal handler invokes only the synchronized C -containment hook and immediate OS exit. The pool owns socket-path cleanup after -the manager has terminated. +The attached manager signal handler invokes the synchronized C shutdown hook, +then exits. The hook closes the fork gate and kills the still-pinned group. +It then waits for the normal executor to acknowledge complete leader and +descendant reaping; it does not introduce a competing `waitpid` owner. A +mutex-protected in-progress flag spans the interval after the numeric group +identity is retired but before descendant collection finishes. + +The reap acknowledgement has a fixed one-second wait, below the pool's +three-second manager wait. If the executor cannot finish, the manager +fail-stops with status 125 instead of waiting indefinitely or reporting normal +shutdown. Executor-side containment failure does not wait for itself. A +fail-stop cannot promise that uncollectable children have been reaped; their +eventual collection belongs to the next subreaper or host init. The pool owns +socket-path cleanup after the manager has terminated. ## Failure classification diff --git a/src/spawn-posix.c b/src/spawn-posix.c index 2024b96..7d683d1 100644 --- a/src/spawn-posix.c +++ b/src/spawn-posix.c @@ -48,6 +48,7 @@ #define DESCRIPTOR_CEILING_UNINITIALIZED -2 #define POLL_SLICE_MS 10 #define TERMINATION_GRACE_MS 100 +#define SHUTDOWN_REAP_WAIT_MS 1000 /* * ERROR_FD is the sole descriptor intentionally kept across child setup. The @@ -68,6 +69,12 @@ extern char **environ; static pthread_mutex_t group_state_lock = PTHREAD_MUTEX_INITIALIZER; static pid_t active_group; static int shutdown_requested; +/* + * Protected by group_state_lock. Unlike the process-group identity this stays + * set after leader retirement until descendant reaping has also completed. + * The interrupt task waits on this acknowledgement, never on a reusable PGID. + */ +static int request_in_progress; /* * The manager opens every persistent descriptor before its request loop. @@ -680,13 +687,11 @@ static int terminate_group( } /* - * GNAT interrupt-task hook: prevent a later fork, kill the currently published - * request group and preserve errno. The Ada handler exits the manager after - * this returns; normal execution remains the sole reaper meanwhile. + * Prevent a later fork and kill the still-pinned request group. Do not wait: + * executor-side fail-stop also uses this hook and must not await itself. */ -void spawn_posix_terminate_current(void) +static void request_shutdown(void) { - int saved_errno = errno; pid_t group; lock_group_state(); @@ -695,6 +700,48 @@ void spawn_posix_terminate_current(void) if (group > 0) (void)kill(-group, SIGKILL); unlock_group_state(); +} + +/* + * Let the sole executor finish reaping without holding its ownership mutex. + * A fixed one-second deadline stays below the pool's three-second wait; a + * stalled executor fail-stops rather than deadlocking an interrupt-server task. + */ +static void wait_for_request_reaping(void) +{ + int64_t started = monotonic_milliseconds(); + int64_t deadline; + + if (started < 0) + _exit(CONTAINMENT_FAILURE_EXIT); + deadline = started > INT64_MAX - SHUTDOWN_REAP_WAIT_MS + ? INT64_MAX : started + SHUTDOWN_REAP_WAIT_MS; + for (;;) { + int pending; + struct timespec pause = { 0, POLL_SLICE_MS * 1000 * 1000 }; + + lock_group_state(); + pending = request_in_progress; + unlock_group_state(); + if (!pending) + return; + if (remaining_milliseconds(deadline) == 0) + _exit(CONTAINMENT_FAILURE_EXIT); + (void)nanosleep(&pause, NULL); + } +} + +/* + * GNAT interrupt-task hook: prevent another fork, kill the published group and + * await the executor's final reap before the Ada handler exits the manager. + * Waiting uses no Ada runtime or competing waitpid owner and preserves errno. + */ +void spawn_posix_terminate_current(void) +{ + int saved_errno = errno; + + request_shutdown(); + wait_for_request_reaping(); errno = saved_errno; } @@ -820,6 +867,7 @@ int spawn_posix_execute( if (setpgid(pid, pid) < 0 && errno != EACCES && errno != ESRCH) group_setup_error = errno; active_group = pid; + request_in_progress = 1; unlock_group_state(); if (set_manager_signal_mask( SIG_SETMASK, &original_signal_mask, NULL) < 0) { @@ -978,7 +1026,7 @@ int spawn_posix_execute( } if (containment_failed) { /* Never resume the manager after losing request-group ownership. */ - spawn_posix_terminate_current(); + request_shutdown(); _exit(CONTAINMENT_FAILURE_EXIT); } if (manager_signals_blocked @@ -990,6 +1038,8 @@ int spawn_posix_execute( unlock_group_state(); _exit(CONTAINMENT_FAILURE_EXIT); } + /* All leader and descendant waits precede this shutdown acknowledgement. */ + request_in_progress = 0; unlock_group_state(); if (pidfd >= 0) (void)close(pidfd); diff --git a/src/spawn-posix.h b/src/spawn-posix.h index b2469ef..397d81d 100644 --- a/src/spawn-posix.h +++ b/src/spawn-posix.h @@ -95,12 +95,13 @@ int spawn_posix_execute( struct spawn_posix_result *result); /* - * Kill the active request group, if any. + * Kill the active request group, if any, and await its executor's final reap. * * This function is called by GNAT's SIGINT/SIGTERM interrupt-server task. It * synchronizes through a process-local mutex with fork publication and exact * leader reaping; it is not a raw POSIX signal handler. The normal execute path - * remains the sole reaper. + * remains the sole reaper, including descendants after leader retirement. + * A stalled executor fail-stops the manager after a one-second reap wait. */ void spawn_posix_terminate_current(void); diff --git a/tests/check_signal_reaping.py b/tests/check_signal_reaping.py new file mode 100644 index 0000000..bc77fc6 --- /dev/null +++ b/tests/check_signal_reaping.py @@ -0,0 +1,224 @@ +#!/usr/bin/env python3 +# +# Copyright (C) 2026 secunet Security Networks AG +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the Free +# Software Foundation; either version 2 of the License, or (at your option) +# any later version. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. + +"""Require the actual manager to reap its group before catchable shutdown.""" + +import ctypes +import json +import os +from pathlib import Path +import shlex +import signal +import socket +import struct +import subprocess +import sys +import tempfile +import time + + +PR_SET_CHILD_SUBREAPER = 36 +WORKER = """import json, os, signal +from pathlib import Path +child = os.fork() +if child == 0: + while True: + signal.pause() +ready = Path('ready.json.new') +ready.write_text(json.dumps([os.getpid(), child])) +ready.replace('ready.json') +while True: + signal.pause() +""" + + +def require(condition, message): + """Make failed containment observable rather than relying on an init.""" + if not condition: + raise AssertionError(message) + + +def string(value): + """Encode one documented protocol string, not shell source.""" + data = os.fsencode(value) + return struct.pack(">I", len(data)) + data + + +def request(kind, version, worker, directory): + """Exercise both public request styles with the same descendant fixture.""" + if kind == "shell": + # The trailing command keeps Bash as leader, with Python beneath it. + payload = string(shlex.join([sys.executable, str(worker)]) + "; :") + payload += string(directory) + struct.pack(">q", -1) + message = 1 + else: + payload = string(sys.executable) + struct.pack(">I", 1) + payload += string(worker) + payload += struct.pack(">I", 0) + string(directory) + payload += b"\x00\x00" + struct.pack(">q", -1) + message = 2 + header = struct.pack(">4sHHI", b"SPWN", version, message, len(payload)) + return header + payload + + +def wait_until(predicate, manager, message): + """Bound fixture startup and fail if the manager dies before readiness.""" + deadline = time.monotonic() + 5 + while not predicate(): + require(manager.poll() is None, message + ": manager exited") + require(time.monotonic() < deadline, message + ": timed out") + time.sleep(0.01) + + +def collect_children(): + """Collect every adopted descendant, never the already waited manager.""" + adopted = [] + deadline = time.monotonic() + 5 + while True: + try: + pid, _ = os.waitpid(-1, os.WNOHANG) + except ChildProcessError: + return adopted + if pid: + adopted.append(pid) + else: + require(time.monotonic() < deadline, "live adopted descendant") + time.sleep(0.01) + + +def connect_when_ready(connection, address): + """A bound socket can exist before the manager has started listening.""" + try: + connection.connect(str(address)) + return True + except (FileNotFoundError, ConnectionRefusedError): + return False + + +def recover(manager): + """Kill and reap private test subjects even when the assertion fails.""" + if manager.poll() is None: + manager.kill() + manager.wait(timeout=5) + children = Path(f"/proc/{os.getpid()}/task/{os.getpid()}/children") + # No other subprocess is active. Unreaped children cannot reuse a PID. + # Repeat: killing an adopted parent can expose another live generation. + deadline = time.monotonic() + 5 + while True: + for value in children.read_text().split(): + try: + os.kill(int(value), signal.SIGKILL) + except ProcessLookupError: + pass + try: + os.waitpid(-1, os.WNOHANG) + except ChildProcessError: + return + require(time.monotonic() < deadline, "private recovery timed out") + time.sleep(0.01) + + +def check_recovery(): + """Reap three generations, including children adopted during cleanup.""" + with tempfile.TemporaryDirectory(prefix="spawn-recovery-") as temporary: + directory = Path(temporary) + worker = directory / "worker.py" + worker.write_text(WORKER) + manager = subprocess.Popen( + ["/bin/bash", "-c", + shlex.join([sys.executable, str(worker)]) + "; :"], + cwd=directory, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + children = Path(f"/proc/{os.getpid()}/task/{os.getpid()}/children") + try: + ready = directory / "ready.json" + wait_until(ready.is_file, manager, "recovery fixture not ready") + members = json.loads(ready.read_text()) + require(len(members) == 2, "recovery fixture lacks generations") + for pid in members: + os.kill(pid, 0) + recover(manager) + require(not children.read_text().strip(), + "recovery returned with unreaped private children") + finally: + # Independent rescue must not depend on the helper under test. + if manager.poll() is None: + manager.kill() + manager.wait(timeout=5) + deadline = time.monotonic() + 5 + while True: + for value in children.read_text().split(): + try: + os.kill(int(value), signal.SIGKILL) + except ProcessLookupError: + pass + try: + os.waitpid(-1, os.WNOHANG) + except ChildProcessError: + break + require(time.monotonic() < deadline, "recovery rescue timed out") + time.sleep(0.01) + print("PASS signal fixture: newly adopted recovery children", flush=True) + + +def exercise(executable, version, kind, termination): + """Detect orphan zombies independently of the surrounding PID 1 behavior.""" + with tempfile.TemporaryDirectory(prefix="spawn-reaping-") as temporary: + directory = Path(temporary) + worker = directory / "worker.py" + worker.write_text(WORKER) + with (directory / "manager.log").open("wb") as output: + manager = subprocess.Popen( + [str(executable), "8192", "manager.sock"], cwd=directory, + stdout=output, stderr=subprocess.STDOUT) + try: + with socket.socket(socket.AF_UNIX) as connection: + connection.settimeout(5) + wait_until(lambda: connect_when_ready( + connection, directory / "manager.sock"), + manager, "manager socket not ready") + connection.sendall( + request(kind, version, worker, directory)) + ready = directory / "ready.json" + wait_until(ready.is_file, manager, "request not ready") + members = json.loads(ready.read_text()) + require(len(members) == 2, "missing descendant fixture") + for pid in members: + os.kill(pid, 0) + os.kill(manager.pid, termination) + require(manager.wait(timeout=3) == 0, + "catchable shutdown did not complete normally") + adopted = collect_children() + require(not adopted, + f"{kind}/{termination.name} left adopted children: " + f"{adopted}") + finally: + recover(manager) + print(f"PASS signal reaping: {kind}/{termination.name}", flush=True) + + +def main(): + """Become the nearest fallback reaper and assert it is never needed.""" + executable = Path(sys.argv[1]).resolve(strict=True) + version = int(sys.argv[2]) + libc = ctypes.CDLL(None, use_errno=True) + require(libc.prctl(PR_SET_CHILD_SUBREAPER, 1, 0, 0, 0) == 0, + "cannot enable test subreaper") + check_recovery() + for kind in ("shell", "exec"): + for termination in (signal.SIGINT, signal.SIGTERM): + exercise(executable, version, kind, termination) + + +if __name__ == "__main__": + main() diff --git a/tests/check_signal_shutdown.sh b/tests/check_signal_shutdown.sh index bdc508e..bc73e21 100755 --- a/tests/check_signal_shutdown.sh +++ b/tests/check_signal_shutdown.sh @@ -23,8 +23,8 @@ manager_body=$source_root/tools/spawn_manager.adb posix_core=$source_root/src/spawn-posix.c # The attached handler runs in a GNAT interrupt-server task. Keep it to the -# synchronized C containment hook and immediate process exit; the pool owns -# socket-path cleanup after the manager has terminated. +# synchronized C shutdown hook followed by process exit. The hook waits for +# the executor's bounded reap; the pool owns socket-path cleanup afterwards. actual=$( awk ' /^[[:space:]]*procedure Handle_Signal$/ { handler = 1 } @@ -69,4 +69,4 @@ if test "$manager_mask_calls" -ne 1 || test "$child_mask_calls" -ne 1; then exit 1 fi -echo "Signal shutdown contains the active group before immediate exit" +echo "Signal shutdown delegates bounded containment and reaping to C" diff --git a/tests/spawn-posix-tests.c b/tests/spawn-posix-tests.c index 9142c53..e941945 100644 --- a/tests/spawn-posix-tests.c +++ b/tests/spawn-posix-tests.c @@ -65,6 +65,10 @@ static atomic_int pause_parent_after_fork; static atomic_int parent_fork_paused; static atomic_int release_parent_fork; static atomic_int termination_call_started; +static atomic_int termination_call_finished; +static atomic_int pause_group_reap_once; +static atomic_int group_reap_paused; +static atomic_int release_group_reap; static char setuid_fixture_directory[PATH_MAX]; static char setuid_fixture_path[PATH_MAX]; @@ -207,6 +211,17 @@ pid_t __wrap_waitpid(pid_t pid, int *status, int options) { pid_t waited; + if (getpid() == wrapper_owner && pid < 0 && options == WNOHANG + && atomic_exchange(&pause_group_reap_once, 0)) { + /* The leader is retired, but its adopted child is not reaped yet. */ + atomic_store(&group_reap_paused, 1); + while (!atomic_load(&release_group_reap)) { + struct timespec pause = { 0, 1000 * 1000 }; + + (void)nanosleep(&pause, NULL); + } + } + if (getpid() == wrapper_owner && pid < 0 && options == WNOHANG && fail_group_reap_once) { fail_group_reap_once = 0; @@ -1256,6 +1271,8 @@ static void test_group_reap_failure_fail_stop( struct threaded_execution { const char *self; + const char *fixture; + const char *output_path; int return_code; struct spawn_posix_result result; }; @@ -1266,11 +1283,15 @@ static void *run_threaded_execution(void *argument) struct threaded_execution *execution = argument; char *empty_environment[] = { NULL }; char *arguments[] = { - (char *)execution->self, "fixture", "tree", NULL + (char *)execution->self, "fixture", + (char *)(execution->fixture != NULL ? execution->fixture : "tree"), + NULL }; execution->return_code = spawn_posix_execute( - execution->self, arguments, 0, empty_environment, "/", 0, NULL, + execution->self, arguments, 0, empty_environment, "/", + execution->output_path != NULL ? SPAWN_POSIX_TRUNCATE_FILE : 0, + execution->output_path, 0, NULL, 1000, &execution->result); return NULL; } @@ -1281,13 +1302,14 @@ static void *run_threaded_termination(void *argument) (void)argument; atomic_store(&termination_call_started, 1); spawn_posix_terminate_current(); + atomic_store(&termination_call_finished, 1); return NULL; } /* Exercise the publication race inside one disposable manager process. */ static void run_threaded_group_publication(const char *self) { - struct threaded_execution execution = { self, 0, { 0, 0, 0, 0, 0 } }; + struct threaded_execution execution = { .self = self }; pthread_t execution_thread; pthread_t termination_thread; struct timespec pause = { 0, 10 * 1000 * 1000 }; @@ -1350,6 +1372,102 @@ static void test_threaded_group_publication(const char *self) pass("threaded shutdown waits for request-group publication"); } +/* Bound every test barrier so a broken acknowledgement cannot hang tests. */ +static void wait_for_test_flag(atomic_int *flag, const char *message) +{ + struct timespec pause = { 0, 10 * 1000 * 1000 }; + + for (int attempt = 0; attempt < 500 && !atomic_load(flag); ++attempt) + (void)nanosleep(&pause, NULL); + require(atomic_load(flag), message); +} + +/* Hold the executor precisely between leader retirement and descendant reap. */ +static void run_threaded_reaping( + const char *self, + const char *pid_path, + int stall) +{ + struct threaded_execution execution = { + .self = self, .fixture = "orphan", .output_path = pid_path + }; + pthread_t executor; + pthread_t terminator; + struct timespec pause = { 0, 10 * 1000 * 1000 }; + char *contents; + pid_t descendant; + int returned_early; + + /* Alarm is recovery only; an exhausted production deadline exits 125. */ + alarm(5); + wrapper_owner = getpid(); + atomic_store(&pause_group_reap_once, 1); + atomic_store(&group_reap_paused, 0); + atomic_store(&release_group_reap, 0); + atomic_store(&termination_call_started, 0); + atomic_store(&termination_call_finished, 0); + require(pthread_create( + &executor, NULL, run_threaded_execution, &execution) == 0, + "start paused reaping executor"); + wait_for_test_flag(&group_reap_paused, "executor missed reap barrier"); + contents = read_file(pid_path); + descendant = (pid_t)strtol(contents, NULL, 10); + free(contents); + require(descendant > 0 && kill(descendant, 0) == 0, + "reaping fixture has no unreaped descendant"); + if (stall) { + spawn_posix_terminate_current(); + _exit(96); + } + + require(pthread_create( + &terminator, NULL, run_threaded_termination, NULL) == 0, + "start reaping termination task"); + wait_for_test_flag(&termination_call_started, "termination task missing"); + for (int attempt = 0; + attempt < 20 && !atomic_load(&termination_call_finished); ++attempt) + (void)nanosleep(&pause, NULL); + returned_early = atomic_load(&termination_call_finished); + atomic_store(&release_group_reap, 1); + require(pthread_join(terminator, NULL) == 0, "join reaping terminator"); + require(pthread_join(executor, NULL) == 0, "join reaping executor"); + require(!returned_early, "shutdown returned before descendant reaping"); + require(execution.return_code == 0, + "paused reaping executor did not complete"); + require_exited(&execution.result, 0, "paused reaping result"); + (void)alarm(0); +} + +/* Assert the post-retirement wait and its fail-stop bound in fresh managers. */ +static void test_threaded_reaping(const char *self, const char *pid_path) +{ + for (int stall = 0; stall < 2; ++stall) { + pid_t supervisor = fork(); + pid_t adopted; + int status; + int child_status; + + require(supervisor >= 0, "fork reaping supervisor"); + if (supervisor == 0) { + run_threaded_reaping(self, pid_path, stall); + _exit(0); + } + require(__real_waitpid(supervisor, &status, 0) == supervisor, + "wait for reaping supervisor"); + /* A fail-stop can leave killed children; own and reap those here. */ + do { + adopted = __real_waitpid(-1, &child_status, 0); + } while (adopted > 0 || (adopted < 0 && errno == EINTR)); + require(adopted < 0 && errno == ECHILD, + "reaping supervisor left an adopted child"); + require(WIFEXITED(status) && WEXITSTATUS(status) + == (stall ? CONTAINMENT_EXIT_STATUS : 0), + "shutdown reap acknowledgement or bound failed"); + pass(stall ? "stalled shutdown reaping fail-stops within its bound" + : "shutdown waits beyond process-group identity retirement"); + } +} + static void test_timeout_group( const char *self, const char *pid_path) @@ -1485,6 +1603,7 @@ int main(int argc, char *argv[], char *envp[]) test_success_group_cleanup(self, pid_path); test_child_signal_dispositions(fifo_path); test_threaded_group_publication(self); + test_threaded_reaping(self, pid_path); require(close(inherited_fd) == 0 && close(source_fd) == 0, "close descriptor fixture"); diff --git a/tools/spawn-signals.adb b/tools/spawn-signals.adb index 7325ec6..3c0c04f 100644 --- a/tools/spawn-signals.adb +++ b/tools/spawn-signals.adb @@ -37,7 +37,8 @@ package body Spawn.Signals is with Import, Convention => C, External_Name => "spawn_posix_terminate_current"; - -- Kill the C-owned active request group, or do nothing when none exists. + -- Kill the C-owned group and await its executor's bounded reap before + -- allowing the manager to exit. ------------------------------------------------------------------------- From dc3b3bb635e3ff7960a8b28a0f839cbff00309b3 Mon Sep 17 00:00:00 2001 From: mweidle73 Date: Sat, 17 Oct 2026 18:12:13 +0200 Subject: [PATCH 2/5] test(signals): Bound reaping test recovery Bound adopted-child collection in the delayed-reaping fixture and respect OBJDIR when locating the built manager. Keep completed work in the changelog instead of the public follow-up queue. --- CHANGELOG.md | 14 +++++++------- Makefile | 2 +- WORK_QUEUE.md | 16 ---------------- tests/spawn-posix-tests.c | 12 +++++++++--- 4 files changed, 17 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a6aa91..cb42049 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,13 +16,6 @@ Gitlink even when the two Spawn Manager commits are not direct ancestors. ## [Unreleased] -### Fixed - -- Catchable manager shutdown now waits for its executor to reap request - descendants before exiting, including after the leader identity has been - retired. A stalled reap acknowledgement fail-stops within a fixed bound. - Shell and structured requests retain the existing wire format and results. - ### Added - Added public contribution guidance and a private vulnerability-reporting @@ -39,6 +32,13 @@ Gitlink even when the two Spawn Manager commits are not direct ancestors. Abuild integration and GitHub overlay branches as frozen history. - Replaced Codelabs synchronization with local release-tag integrity checks. +### Fixed + +- Catchable manager shutdown now waits for its executor to reap request + descendants before exiting, including after the leader identity has been + retired. A stalled reap acknowledgement fail-stops within a fixed bound. + Shell and structured requests retain the existing wire format and results. + ## [0.2.0] - 2026-09-27 ### Added diff --git a/Makefile b/Makefile index 48493e2..c10bff9 100644 --- a/Makefile +++ b/Makefile @@ -59,7 +59,7 @@ tests: $(POSIX_TEST) $(PROTOCOL_FAILURE_MANAGER) spawn_tests spawn_manager @$(OBJDIR)/spawn_manager 47 $(OBJDIR)/spawn_manager_min_spawn & @$(OBJDIR)/test_runner @tests/check_signal_shutdown.sh "$(CURDIR)" - @python3 tests/check_signal_reaping.py "$(CURDIR)/obj/spawn_manager" 1 + @python3 tests/check_signal_reaping.py "$(abspath $(OBJDIR))/spawn_manager" 1 @tests/check_adaflags.sh "$(CURDIR)" @tests/check_install.sh "$(CURDIR)" diff --git a/WORK_QUEUE.md b/WORK_QUEUE.md index 047975d..1386880 100644 --- a/WORK_QUEUE.md +++ b/WORK_QUEUE.md @@ -17,22 +17,6 @@ define acceptance evidence before starting an item. ## Accepted follow-ups -### Catchable shutdown reaping - -Implemented on the current topic; upstream acceptance remains pending. -SIGINT/SIGTERM used to kill the request group and immediately exit the manager, -which could abandon killed descendants before the executor reaped them. The -shutdown hook now awaits a bounded acknowledgement from the sole reaper, -including the interval after leader retirement. It does not change the wire -protocol, pool API or ordinary request execution path. - -Direct C barriers pin delayed and stalled descendant reaping. A built-manager -check becomes the nearest fallback subreaper and rejects adopted children for -both request kinds and termination signals, independent of container init. -Fixture recovery also pins adoption across killed generations. -Carry this correction into the shared-output topic before caller acceptance; -the downstream Make migration does not silently change its dependency. - ### Neutral GitHub organization The canonical repository initially remains under its current personal GitHub diff --git a/tests/spawn-posix-tests.c b/tests/spawn-posix-tests.c index e941945..0f4e5e2 100644 --- a/tests/spawn-posix-tests.c +++ b/tests/spawn-posix-tests.c @@ -1446,6 +1446,7 @@ static void test_threaded_reaping(const char *self, const char *pid_path) pid_t adopted; int status; int child_status; + struct timespec pause = { 0, 10 * 1000 * 1000 }; require(supervisor >= 0, "fork reaping supervisor"); if (supervisor == 0) { @@ -1455,9 +1456,14 @@ static void test_threaded_reaping(const char *self, const char *pid_path) require(__real_waitpid(supervisor, &status, 0) == supervisor, "wait for reaping supervisor"); /* A fail-stop can leave killed children; own and reap those here. */ - do { - adopted = __real_waitpid(-1, &child_status, 0); - } while (adopted > 0 || (adopted < 0 && errno == EINTR)); + for (int attempt = 0; attempt < 500; ++attempt) { + adopted = __real_waitpid(-1, &child_status, WNOHANG); + if (adopted < 0 && errno == ECHILD) + break; + require(adopted >= 0 || errno == EINTR, + "reaping supervisor recovery failed"); + (void)nanosleep(&pause, NULL); + } require(adopted < 0 && errno == ECHILD, "reaping supervisor left an adopted child"); require(WIFEXITED(status) && WEXITSTATUS(status) From 0c53d8a54bc5f28843aabfff79ce40e414e1d291 Mon Sep 17 00:00:00 2001 From: mweidle73 Date: Sun, 18 Oct 2026 08:48:55 +0200 Subject: [PATCH 3/5] test(signals): Verify descendant reap ownership Enable subreaping in each forked supervisor because Linux does not inherit that setting, even though the initialization cache is copied. Require no adopted child after normal completion and outer recovery only for the deliberately stalled fail-stop. The rootless Trixie suite passes. Missing supervisor initialization and false ECHILD after the reap barrier both fail the new assertion. Removing that assertion makes the skipped-reap mutation pass again. --- tests/spawn-posix-tests.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/spawn-posix-tests.c b/tests/spawn-posix-tests.c index 0f4e5e2..0a71a23 100644 --- a/tests/spawn-posix-tests.c +++ b/tests/spawn-posix-tests.c @@ -1400,6 +1400,9 @@ static void run_threaded_reaping( /* Alarm is recovery only; an exhausted production deadline exits 125. */ alarm(5); + /* fork copies the core's cache, but not Linux's subreaper setting. */ + require(prctl(PR_SET_CHILD_SUBREAPER, 1) == 0, + "enable reaping supervisor subreaper"); wrapper_owner = getpid(); atomic_store(&pause_group_reap_once, 1); atomic_store(&group_reap_paused, 0); @@ -1441,11 +1444,15 @@ static void run_threaded_reaping( /* Assert the post-retirement wait and its fail-stop bound in fresh managers. */ static void test_threaded_reaping(const char *self, const char *pid_path) { + /* Own failed supervisors' descendants without hiding successful leaks. */ + require(prctl(PR_SET_CHILD_SUBREAPER, 1) == 0, + "enable reaping recovery subreaper"); for (int stall = 0; stall < 2; ++stall) { pid_t supervisor = fork(); pid_t adopted; int status; int child_status; + int recovered_child = 0; struct timespec pause = { 0, 10 * 1000 * 1000 }; require(supervisor >= 0, "fork reaping supervisor"); @@ -1462,6 +1469,8 @@ static void test_threaded_reaping(const char *self, const char *pid_path) break; require(adopted >= 0 || errno == EINTR, "reaping supervisor recovery failed"); + if (adopted > 0) + recovered_child = 1; (void)nanosleep(&pause, NULL); } require(adopted < 0 && errno == ECHILD, @@ -1469,6 +1478,10 @@ static void test_threaded_reaping(const char *self, const char *pid_path) require(WIFEXITED(status) && WEXITSTATUS(status) == (stall ? CONTAINMENT_EXIT_STATUS : 0), "shutdown reap acknowledgement or bound failed"); + /* Only the deliberately stalled fail-stop may need outer recovery. */ + require(recovered_child == stall, + stall ? "stalled supervisor left no recovery child" + : "successful supervisor left an adopted child"); pass(stall ? "stalled shutdown reaping fail-stops within its bound" : "shutdown waits beyond process-group identity retirement"); } From f343b61df6677d515e5072ed80ea81dae898c006 Mon Sep 17 00:00:00 2001 From: mweidle73 Date: Sun, 18 Oct 2026 10:16:22 +0200 Subject: [PATCH 4/5] test(signals): Isolate publication descendants Enable subreaping in the publication supervisor and wait for actual descendant metadata before releasing its fork barrier. Reuse bounded recovery and reject adoption at each test boundary, so one fixture cannot contaminate the next. The old test fails natively on ARM64. Both ARM64 and emulated AMD64 reject missing subreaper setup and skipped collection mutations; corrected direct-core runs pass repeatedly on both. Production code and the wire contract are unchanged. --- tests/spawn-posix-tests.c | 88 +++++++++++++++++++++++++++++---------- 1 file changed, 66 insertions(+), 22 deletions(-) diff --git a/tests/spawn-posix-tests.c b/tests/spawn-posix-tests.c index 0a71a23..63474b2 100644 --- a/tests/spawn-posix-tests.c +++ b/tests/spawn-posix-tests.c @@ -1306,14 +1306,61 @@ static void *run_threaded_termination(void *argument) return NULL; } -/* Exercise the publication race inside one disposable manager process. */ -static void run_threaded_group_publication(const char *self) +/* Wait for flushed descendant metadata at a freshly removed fixture path. */ +static void wait_for_fixture_output(const char *path) { - struct threaded_execution execution = { .self = self }; + struct timespec pause = { 0, 10 * 1000 * 1000 }; + struct stat status; + int ready = 0; + + for (int attempt = 0; attempt < 500; ++attempt) { + if (stat(path, &status) == 0 && status.st_size > 0) { + ready = 1; + break; + } + (void)nanosleep(&pause, NULL); + } + require(ready, "publication fixture did not create a descendant"); +} + +/* Bound recovery, then report adoption so cleanup cannot hide a test leak. */ +static int recover_adopted_children(void) +{ + struct timespec pause = { 0, 10 * 1000 * 1000 }; + pid_t adopted; + int child_status; + int recovered_child = 0; + + for (int attempt = 0; attempt < 500; ++attempt) { + adopted = __real_waitpid(-1, &child_status, WNOHANG); + if (adopted < 0 && errno == ECHILD) + break; + require(adopted >= 0 || errno == EINTR, + "supervisor recovery failed"); + if (adopted > 0) + recovered_child = 1; + (void)nanosleep(&pause, NULL); + } + require(adopted < 0 && errno == ECHILD, + "supervisor left an adopted child"); + return recovered_child; +} + +/* Exercise publication with a real descendant, not a host-speed assumption. */ +static void run_threaded_group_publication( + const char *self, + const char *pid_path) +{ + struct threaded_execution execution = { + .self = self, .output_path = pid_path + }; pthread_t execution_thread; pthread_t termination_thread; struct timespec pause = { 0, 10 * 1000 * 1000 }; + /* fork inherits the core cache, but not the kernel subreaper setting. */ + require(prctl(PR_SET_CHILD_SUBREAPER, 1) == 0, + "enable publication supervisor subreaper"); wrapper_owner = getpid(); atomic_store(&pause_parent_after_fork, 1); atomic_store(&parent_fork_paused, 0); @@ -1328,6 +1375,7 @@ static void run_threaded_group_publication(const char *self) (void)nanosleep(&pause, NULL); require(atomic_load(&parent_fork_paused), "threaded execution did not reach the fork barrier"); + wait_for_fixture_output(pid_path); require(pthread_create( &termination_thread, NULL, run_threaded_termination, NULL) == 0, "create threaded termination"); @@ -1352,23 +1400,33 @@ static void run_threaded_group_publication(const char *self) } /* Prove shutdown cannot pass the fork-to-group-publication window. */ -static void test_threaded_group_publication(const char *self) +static void test_threaded_group_publication( + const char *self, + const char *pid_path) { pid_t supervisor; int supervisor_status; + int recovered_child; + require(prctl(PR_SET_CHILD_SUBREAPER, 1) == 0, + "enable publication recovery subreaper"); + require(unlink(pid_path) == 0 || errno == ENOENT, + "reset publication pid file"); supervisor = fork(); require(supervisor >= 0, "fork threaded publication supervisor"); if (supervisor == 0) { - run_threaded_group_publication(self); + run_threaded_group_publication(self, pid_path); _exit(0); } require(__real_waitpid( supervisor, &supervisor_status, 0) == supervisor, "wait for threaded publication supervisor"); + recovered_child = recover_adopted_children(); require(WIFEXITED(supervisor_status) && WEXITSTATUS(supervisor_status) == 0, "threaded publication supervisor failed"); + require(!recovered_child, + "publication supervisor left an adopted child"); pass("threaded shutdown waits for request-group publication"); } @@ -1449,11 +1507,8 @@ static void test_threaded_reaping(const char *self, const char *pid_path) "enable reaping recovery subreaper"); for (int stall = 0; stall < 2; ++stall) { pid_t supervisor = fork(); - pid_t adopted; int status; - int child_status; - int recovered_child = 0; - struct timespec pause = { 0, 10 * 1000 * 1000 }; + int recovered_child; require(supervisor >= 0, "fork reaping supervisor"); if (supervisor == 0) { @@ -1463,18 +1518,7 @@ static void test_threaded_reaping(const char *self, const char *pid_path) require(__real_waitpid(supervisor, &status, 0) == supervisor, "wait for reaping supervisor"); /* A fail-stop can leave killed children; own and reap those here. */ - for (int attempt = 0; attempt < 500; ++attempt) { - adopted = __real_waitpid(-1, &child_status, WNOHANG); - if (adopted < 0 && errno == ECHILD) - break; - require(adopted >= 0 || errno == EINTR, - "reaping supervisor recovery failed"); - if (adopted > 0) - recovered_child = 1; - (void)nanosleep(&pause, NULL); - } - require(adopted < 0 && errno == ECHILD, - "reaping supervisor left an adopted child"); + recovered_child = recover_adopted_children(); require(WIFEXITED(status) && WEXITSTATUS(status) == (stall ? CONTAINMENT_EXIT_STATUS : 0), "shutdown reap acknowledgement or bound failed"); @@ -1621,7 +1665,7 @@ int main(int argc, char *argv[], char *envp[]) require(unlink(pid_path) == 0, "reset descendant pid file"); test_success_group_cleanup(self, pid_path); test_child_signal_dispositions(fifo_path); - test_threaded_group_publication(self); + test_threaded_group_publication(self, pid_path); test_threaded_reaping(self, pid_path); require(close(inherited_fd) == 0 && close(source_fd) == 0, From 2ceed04e16da949e27ea916dec2de4cdbaf98052 Mon Sep 17 00:00:00 2001 From: mweidle73 Date: Sun, 18 Oct 2026 10:34:30 +0200 Subject: [PATCH 5/5] test(signals): Keep fixture socket paths short Connect relative to the fixture directory and use contextlib.chdir to restore the caller directory on every return and exception. The harness is sequential, so no other cwd consumer can race this scope. Add real-socket checks for missing, refused, successful and unexpected error outcomes under an overlong parent path. Absolute-connect and cwd-leak mutations fail. The Trixie suite passes, including an actual manager run with a long TMPDIR. Production behavior is unchanged. --- tests/check_signal_reaping.py | 78 +++++++++++++++++++++++++++++++---- 1 file changed, 71 insertions(+), 7 deletions(-) diff --git a/tests/check_signal_reaping.py b/tests/check_signal_reaping.py index bc77fc6..5ec870d 100644 --- a/tests/check_signal_reaping.py +++ b/tests/check_signal_reaping.py @@ -14,7 +14,9 @@ """Require the actual manager to reap its group before catchable shutdown.""" +from contextlib import chdir import ctypes +import errno import json import os from pathlib import Path @@ -98,12 +100,72 @@ def collect_children(): def connect_when_ready(connection, address): - """A bound socket can exist before the manager has started listening.""" - try: - connection.connect(str(address)) - return True - except (FileNotFoundError, ConnectionRefusedError): - return False + """Use a short socket address and restore cwd on every return or error.""" + # This sequential harness has no competing process-wide cwd consumer. + with chdir(address.parent): + try: + connection.connect(address.name) + return True + except (FileNotFoundError, ConnectionRefusedError): + return False + + +def check_socket_paths(): + """Pin long-parent connections and cwd restoration on every outcome.""" + original = Path.cwd() + with tempfile.TemporaryDirectory(prefix="spawn-socket-") as temporary: + directory = Path(temporary) / ("long-directory-" * 12) + directory.mkdir(mode=0o700) + address = directory / "manager.sock" + try: + # Independent control: this absolute address really is too long. + with socket.socket(socket.AF_UNIX) as connection: + try: + connection.connect(str(address)) + except OSError as error: + require(str(error) == "AF_UNIX path too long", + "socket fixture does not exceed the path limit") + else: + raise AssertionError("absolute socket path was accepted") + + with socket.socket(socket.AF_UNIX) as server: + server.settimeout(5) + with socket.socket(socket.AF_UNIX) as connection: + require(not connect_when_ready(connection, address), + "missing socket unexpectedly connected") + require(Path.cwd() == original, "missing socket changed cwd") + + with chdir(directory): + server.bind(address.name) + with socket.socket(socket.AF_UNIX) as connection: + require(not connect_when_ready(connection, address), + "unlistening socket unexpectedly connected") + require(Path.cwd() == original, "refused socket changed cwd") + + server.listen(1) + with socket.socket(socket.AF_UNIX) as connection: + connection.settimeout(5) + require(connect_when_ready(connection, address), + "relative socket connection failed") + accepted, _ = server.accept() + accepted.close() + require(Path.cwd() == original, "successful socket changed cwd") + + with socket.socket(socket.AF_UNIX) as connection: + connection.close() + try: + connect_when_ready(connection, address) + except OSError as error: + require(error.errno == errno.EBADF, + "unexpected closed-socket error") + else: + raise AssertionError("closed socket was accepted") + require(Path.cwd() == original, "socket exception changed cwd") + finally: + # Rescue comes after the assertions, so it cannot mask a leak. + os.chdir(original) + print("PASS signal fixture: relative sockets and cwd restoration", + flush=True) def recover(manager): @@ -166,7 +228,8 @@ def check_recovery(): os.waitpid(-1, os.WNOHANG) except ChildProcessError: break - require(time.monotonic() < deadline, "recovery rescue timed out") + require(time.monotonic() < deadline, + "recovery rescue timed out") time.sleep(0.01) print("PASS signal fixture: newly adopted recovery children", flush=True) @@ -214,6 +277,7 @@ def main(): libc = ctypes.CDLL(None, use_errno=True) require(libc.prctl(PR_SET_CHILD_SUBREAPER, 1, 0, 0, 0) == 0, "cannot enable test subreaper") + check_socket_paths() check_recovery() for kind in ("shell", "exec"): for termination in (signal.SIGINT, signal.SIGTERM):