Skip to content

fix(main): exit Windows stdio servers when the spawning client dies (#914) #1874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
LazyXuan wants to merge 2 commits into
base: main
Choose a base branch
from
+238 −23
Open

fix(main): exit Windows stdio servers when the spawning client dies (#914) #1874

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3900b00
fix(main): exit Windows stdio servers when the spawning client dies (…
LazyXuan Aug 28, 2026
41f52a0
test(watchdog): guard the ps -W pipeline for POSIX cleanup
LazyXuan Aug 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 122 additions & 3 deletions src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ enum {
#ifdef _WIN32
#include <shellapi.h> /* CommandLineToArgvW — not pulled in by windows.h under WIN32_LEAN_AND_MEAN */
#include <io.h>
#include <tlhelp32.h> /* CreateToolhelp32Snapshot — parent PID discovery for the parent-death watchdog */
#endif
#include "ui/http_server.h"
#include "ui/embedded_assets.h"
Expand Down Expand Up @@ -390,7 +391,15 @@ static void signal_handler(int sig) {
* otherwise linger forever blocked on stdin. POSIX has no portable "notify on
* parent death" primitive (PR_SET_PDEATHSIG is Linux-only), so we poll getppid:
* once the parent dies the process is reparented (ppid changes, typically to 1)
* and we shut down. Windows is unaffected (job objects handle this) — #ifndef. */
* and we shut down.
*
* Windows used to be excluded here on the assumption that job objects cover it
* (#914 proved they do not): the KILL_ON_JOB_CLOSE job in subprocess.c only
* wraps processes CBM itself spawns. An MCP stdio server is spawned BY the
* client as its child, and Windows does not propagate parent termination to
* children, so the orphan lingers holding SQLite WAL read locks. The Windows
* branch below waits on a handle to the parent process instead — a signaled
* handle is exact (no PID-reuse window), so no polling of the PID is needed. */

#ifndef _WIN32
typedef struct {
Expand Down Expand Up @@ -537,6 +546,110 @@ static bool client_start_parent_watchdog(pid_t initial_ppid) {
}
return true;
}
#else /* _WIN32 */
/* Windows parent-death watchdog — the #914 half of the story.
*
* The KILL_ON_JOB_CLOSE job in subprocess.c only contains processes CBM
* spawns itself; a stdio MCP server is the CLIENT's child, and Windows never
* propagates a parent's termination to its children, so a force-killed client
* leaves the server lingering on stdin while pinning SQLite WAL read locks.
* There is no reparenting to poll for either — instead we open a handle to
* the parent at startup and wait on it: the kernel signals a process handle
* exactly once, when the process terminates, and the held handle pins the
* process object, so PID reuse cannot fool the wait the way re-reading a ppid
* could. The 500 ms timeout exists only to re-check g_shutdown, mirroring the
* POSIX poll cadence.
*
* The worker path keeps its POSIX-only guard: workers are spawned by CBM's own
* subprocess layer inside a kill-on-close job, so containment there is already
* the job object's job. */
typedef struct {
HANDLE parent_process;
bool exit_on_parent_death;
} parent_watchdog_config_t;

static void *parent_watchdog_thread(void *arg) {
parent_watchdog_config_t config = *(parent_watchdog_config_t *)arg;

while (!atomic_load(&g_shutdown)) {
DWORD wait_status = WaitForSingleObject(config.parent_process, 500);
if (wait_status == WAIT_OBJECT_0) {
static const char msg[] = "level=warn msg=parent.exited reason=handle_signaled\n";
(void)_write(_fileno(stderr), msg, sizeof(msg) - 1);
if (config.exit_on_parent_death) {
/* Same deliberate hard stop as the POSIX branch: a lingering
* orphan must release its daemon connection, file locks and
* WAL read lock through kernel handle reclamation, and no
* atexit cleanup is trustworthy after the owning client is
* gone. */
_exit(0);
}
request_shutdown();
break;
}
if (wait_status != WAIT_TIMEOUT) {
break; /* handle became unwaitable — stop watching, never spin */
}
}
return NULL;
}

/* Toolhelp is the documented way to learn one's own parent PID on Windows;
* the PEB value is not exposed through any public API. Returns 0 when the
* lookup itself fails; callers treat that as "no parent signal available". */
static DWORD win_parent_pid_from_snapshot(void) {
DWORD parent_pid = 0;
HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
if (snapshot == INVALID_HANDLE_VALUE) {
return 0;
}
PROCESSENTRY32W entry;
ZeroMemory(&entry, sizeof(entry));
entry.dwSize = sizeof(entry);
DWORD self_pid = GetCurrentProcessId();
if (Process32FirstW(snapshot, &entry)) {
do {
if (entry.th32ProcessID == self_pid) {
parent_pid = entry.th32ParentProcessID;
break;
}
} while (Process32NextW(snapshot, &entry));
}
CloseHandle(snapshot);
return parent_pid;
}

static bool client_start_parent_watchdog(DWORD initial_ppid) {
/* Mirrors the POSIX initial_ppid <= 1 bail-out: when no trustworthy parent
* signal exists at startup — snapshot failed, reserved PID, or the parent
* already exited so the handle cannot be opened — keep running and rely on
* the stdin EOF path instead of watching nothing or dying on a false
* alarm. A watchdog thread creation failure, by contrast, is fatal: the
* same fail-closed choice as POSIX. */
if (initial_ppid == 0) {
return true;
}
HANDLE parent_process = OpenProcess(SYNCHRONIZE, FALSE, initial_ppid);
if (!parent_process) {
return true;
}
static parent_watchdog_config_t client_config;
client_config.parent_process = parent_process;
client_config.exit_on_parent_death = true;
cbm_thread_t watchdog;
if (cbm_thread_create(&watchdog, PARENT_WATCHDOG_STACK_SIZE, parent_watchdog_thread,
&client_config) != 0) {
CloseHandle(parent_process);
return false;
}
if (cbm_thread_detach(&watchdog) != 0) {
atomic_store(&g_shutdown, 1);
(void)cbm_thread_join(&watchdog);
CloseHandle(parent_process);
return false;
}
return true;
}
#endif

/* ── CLI mode ───────────────────────────────────────────────────── */
Expand Down Expand Up @@ -2527,6 +2640,11 @@ int main(int argc, char **argv) {
cbm_alloc_init();
#ifndef _WIN32
pid_t process_initial_ppid = getppid();
#else
/* Captured at the same instant as POSIX: the later OpenProcess in
* client_start_parent_watchdog must target the process that spawned us,
* not whatever may have recycled the PID meanwhile. */
DWORD process_initial_ppid = win_parent_pid_from_snapshot();
#endif
#ifdef _WIN32
{
Expand Down Expand Up @@ -3114,15 +3232,16 @@ int main(int argc, char **argv) {
"cbm-with-ui`.\n");
}
}
#ifndef _WIN32
/* The Windows branch of this call is the #914 fix: identical placement and
* failure handling as POSIX (fail-closed — a client that cannot arm its
* parent watchdog would linger as an orphan after the editor dies). */
if (!client_start_parent_watchdog(process_initial_ppid)) {
(void)fprintf(stderr, "codebase-memory-mcp: parent-death watchdog could not start\n");
(void)cbm_daemon_runtime_client_close(g_daemon_client, MAIN_CLOSE_TIMEOUT_MS);
g_daemon_client = NULL;
(void)main_version_cohort_close(&client_cohort_lease, &client_cohort_manager);
return EXIT_FAILURE;
}
#endif

setup_signal_handlers();
int result = cbm_daemon_frontend_mcp_run(g_daemon_client, client_cohort_manager, stdin, stdout);
Expand Down
136 changes: 116 additions & 20 deletions tests/test_parent_watchdog.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,18 @@
# Strategy: launch the binary under a wrapper "parent" process (stdin kept open
# via a FIFO so the server doesn't see EOF), record the child's PID, then kill
# the wrapper. The watchdog should notice the changed ppid and exit within a
# few seconds. Skipped on Windows-like shells (the watchdog is POSIX-only).
# few seconds. On Windows (MSYS2 shells) the same tree applies: the wrapper is
# an MSYS bash process, the child a native binary whose parent handle the
# watchdog waits on; killing the wrapper terminates that parent process (#914).
set -euo pipefail

ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
BINARY="${CBM_TEST_BINARY:-${ROOT}/build/c/codebase-memory-mcp}"

windows_mode=0
case "$(uname -s)" in
MINGW*|MSYS*|CYGWIN*)
echo "skipping parent watchdog test on Windows"
exit 0
windows_mode=1
;;
esac

Expand All @@ -30,33 +32,91 @@ source "${ROOT}/scripts/test-runtime.sh"
cbm_test_runtime_init
tmpdir="${CBM_TEST_RUNTIME_ROOT}"
wrapper_pid=""
writer_pid=""

kill_hard() {
# MSYS signal delivery does not reliably terminate the Windows process
# behind an MSYS pid (observed: a wrapper bash kept running after kill -9,
# so a child's real parent never actually died). Force-kill through
# TerminateProcess on the Windows pid as well. The winpid must be resolved
# BEFORE the msys kill: kill -9 removes the pid from the MSYS process table
# immediately, and the mapping would be lost. `ps -W` exists only on MSYS —
# under `set -o pipefail` a failing ps would abort this helper on POSIX —
# so the pipeline is guarded and simply yields an empty winpid elsewhere.
local pid="$1" winpid
winpid="$(ps -W 2>/dev/null | awk -v m="${pid}" '$1==m {print $4; exit}' || true)"
kill -9 "${pid}" 2>/dev/null || true
[[ -n "${winpid}" ]] && taskkill //F //PID "${winpid}" >/dev/null 2>&1 || true
}

cleanup() {
if [[ -s "${tmpdir}/child.pid" ]]; then
local child_pid
child_pid="$(cat "${tmpdir}/child.pid" 2>/dev/null || true)"
[[ -n "${child_pid}" ]] && kill "${child_pid}" 2>/dev/null || true
[[ -n "${child_pid}" ]] && kill_hard "${child_pid}" || true
fi
# On Windows the pipe writer is an orphaned helper that outlives the wrapper
# by design (it holds stdin open); only the test knows its PID.
if [[ -n "${writer_pid}" ]]; then
kill_hard "${writer_pid}" || true
fi
[[ -n "${wrapper_pid}" ]] && kill "${wrapper_pid}" 2>/dev/null || true
[[ -n "${wrapper_pid}" ]] && kill_hard "${wrapper_pid}" || true
cbm_test_runtime_cleanup "${BINARY}"
}
trap cleanup EXIT

# Wrapper "parent": opens the FIFO read-write so it stays open, launches the
# MCP server with that FIFO as stdin, records the child PID, then waits.
cat >"${tmpdir}/wrapper.sh" <<'SH'
if (( windows_mode )); then
# MSYS FIFOs are not readable by native Windows binaries (the server blocks
# forever), so the stdin-holder is an anonymous PIPE instead: a writer helper
# forwards the initialize request once the test drops it into a file, then
# holds the pipe's write end open forever. Killing the wrapper orphans the
# writer too, so stdin never sees EOF — the child's exit can only come from
# the parent-death watchdog.
cat >"${tmpdir}/wrapper.sh" <<'SH'
#!/usr/bin/env bash
set -euo pipefail
echo $$ >"${TMPDIR_PATH}/writer.pid"
(
for _ in {1..300}; do
if [[ -s "${REQ_FILE}" ]]; then
cat "${REQ_FILE}"
exec sleep 3600
fi
sleep 0.1
done
) | "${CBM_BINARY}" >"${TMPDIR_PATH}/child.out" 2>"${TMPDIR_PATH}/child.err" &
echo "$!" >"${TMPDIR_PATH}/child.pid"
wait
SH
chmod +x "${tmpdir}/wrapper.sh"
: >"${tmpdir}/request.json"

CBM_BINARY="${BINARY}" REQ_FILE="${tmpdir}/request.json" TMPDIR_PATH="${tmpdir}" \
"${tmpdir}/wrapper.sh" &
wrapper_pid=$!
for _ in {1..50}; do
[[ -s "${tmpdir}/writer.pid" ]] && break
sleep 0.1
done
[[ -s "${tmpdir}/writer.pid" ]] && writer_pid="$(cat "${tmpdir}/writer.pid")"
else
# Wrapper "parent": opens the FIFO read-write so it stays open, launches the
# MCP server with that FIFO as stdin, records the child PID, then waits.
cat >"${tmpdir}/wrapper.sh" <<'SH'
#!/usr/bin/env bash
set -euo pipefail
exec 3<>"${FIFO}"
"${CBM_BINARY}" <&3 >"${TMPDIR_PATH}/child.out" 2>"${TMPDIR_PATH}/child.err" &
echo "$!" >"${TMPDIR_PATH}/child.pid"
wait
SH
chmod +x "${tmpdir}/wrapper.sh"
mkfifo "${tmpdir}/stdin"
chmod +x "${tmpdir}/wrapper.sh"
mkfifo "${tmpdir}/stdin"

CBM_BINARY="${BINARY}" FIFO="${tmpdir}/stdin" TMPDIR_PATH="${tmpdir}" \
"${tmpdir}/wrapper.sh" &
wrapper_pid=$!
CBM_BINARY="${BINARY}" FIFO="${tmpdir}/stdin" TMPDIR_PATH="${tmpdir}" \
"${tmpdir}/wrapper.sh" &
wrapper_pid=$!
fi

# Wait for the child PID file to appear.
for _ in {1..50}; do
Expand All @@ -80,9 +140,14 @@ fi
# the frontend reached its stdio loop after installing the parent watchdog.
# The old mem.init log sync point belonged to the pre-daemon architecture: the
# shared daemon now owns memory initialization, so a frontend need not emit it.
if (( windows_mode )); then
request_target="${tmpdir}/request.json"
else
request_target="${tmpdir}/stdin"
fi
printf '%s\n' \
'{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"parent-watchdog-test","version":"1.0"}}}' \
>"${tmpdir}/stdin"
>"${request_target}"
for _ in {1..150}; do
if [[ -s "${tmpdir}/child.out" ]] &&
grep -Eq '"id"[[:space:]]*:[[:space:]]*1' "${tmpdir}/child.out"; then
Expand All @@ -98,8 +163,35 @@ if ! grep -Eq '"id"[[:space:]]*:[[:space:]]*1' "${tmpdir}/child.out" 2>/dev/null
fi

# Kill the wrapper parent: the orphaned child must now self-exit.
kill -9 "${wrapper_pid}"
wait "${wrapper_pid}" 2>/dev/null || true
if (( windows_mode )); then
# Two Windows-specific traps make "kill -9 $wrapper_pid" wrong here:
# 1. MSYS kill -9 does not reliably terminate the Windows process behind
# an MSYS pid (a wrapper bash keeps running), and
# 2. the wrapper's background PIPELINE puts an intermediate subshell bash
# between wrapper and server, and the watchdog watches the
# Windows-physical parent (Toolhelp ParentProcessId) — that subshell.
# Resolve the child's actual Windows parent and TerminateProcess exactly
# that one, mirroring a force-killed MCP client. The wrapper is NOT waited
# on yet: its wait covers the whole pipeline job, and the stdin-holding
# writer must survive until the child's exit is observed, or stdin would
# EOF and the test could pass without the watchdog doing anything.
child_winpid="$(ps -W 2>/dev/null | awk -v m="${child_pid}" '$1==m {print $4; exit}')"
if [[ -z "${child_winpid}" ]]; then
echo "child windows pid not found for msys pid ${child_pid}" >&2
exit 3
fi
parent_winpid="$(powershell.exe -NoProfile -Command \
"(Get-CimInstance Win32_Process -Filter \"ProcessId=${child_winpid}\").ParentProcessId" \
2>/dev/null | tr -d '[:space:]')"
if [[ -z "${parent_winpid}" || ! "${parent_winpid}" =~ ^[0-9]+$ ]]; then
echo "could not resolve the child's windows parent pid" >&2
exit 3
fi
taskkill //F //PID "${parent_winpid}" >/dev/null 2>&1 || true
else
kill -9 "${wrapper_pid}"
wait "${wrapper_pid}" 2>/dev/null || true
fi

deadline=$((SECONDS + 15))
while (( SECONDS < deadline )); do
Expand All @@ -109,10 +201,14 @@ while (( SECONDS < deadline )); do
fi
# A zombie no longer holds stdin or runs the MCP loop; kill -0 still reports
# it until launchd/test parent reaps it, so treat that as a successful exit.
child_state="$(ps -p "${child_pid}" -o stat= 2>/dev/null | tr -d '[:space:]' || true)"
if [[ "${child_state}" == Z* ]]; then
echo "ok: child ${child_pid} exited after parent death (zombie awaiting reap)"
exit 0
# Windows has no zombie state — an exited process simply disappears from
# kill -0 — so this probe is POSIX-only.
if [[ "${windows_mode}" -eq 0 ]]; then
child_state="$(ps -p "${child_pid}" -o stat= 2>/dev/null | tr -d '[:space:]' || true)"
if [[ "${child_state}" == Z* ]]; then
echo "ok: child ${child_pid} exited after parent death (zombie awaiting reap)"
exit 0
fi
fi
sleep 0.2
done
Expand Down