kill_tree: single-slash taskkill switches, //T was rejected verbatim

A real watchdog defect the new test exposed, not a test-only bug. The
workflow sets MSYS_NO_PATHCONV=1 and MSYS2_ARG_CONV_EXCL='*' (so httrack
sees URL paths unmangled), which makes MSYS pass every argument verbatim.
`taskkill //F //T //PID` therefore reached taskkill with the switches
still doubled, was rejected, and did nothing (masked by `|| true`); only
the direct child died, via kill -9 translating to TerminateProcess on its
winpid. A native grandchild (the wedged httrack.exe in production) would
have survived. Use single-slash switches, which pass through cleanly under
no-conversion, so taskkill /T actually reaps the tree.

Real crawls never hang, so this path was never exercised by the green
runs; 58_watchdog.test is what drove it out.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>

.github/workflows/windows-build.yml

@@ -149,25 +149,28 @@ jobs:
           export HTTPS_SUPPORT=yes BROTLI_ENABLED=yes ZSTD_ENABLED=yes
           export LC_ALL=C.UTF-8
 
-          # A wedged crawl must not eat the job's whole timeout budget.
-          watchdog=""
-          command -v timeout >/dev/null && watchdog="timeout 600"
+          # A wedged crawl must not eat the job's timeout budget. timeout(1)'s
+          # signals can't reap a native httrack.exe (MSYS signals don't reach it),
+          # so a hang orphaned processes that starved the runner; run_with_timeout
+          # TerminateProcess-es the whole tree. 600s clears the slowest multi-pass
+          # crawl (a few passes at --max-time=120 each).
+          . ./testlib.sh
 
           pass=0 fail=0 skip=0 failed="" skipped=""
           for t in 00_runnable.test 01_engine-*.test 01_zlib-*.test \
-                   *_local-*.test 13_crawl_proxy_https.test; do
+                   *_local-*.test 13_crawl_proxy_https.test 58_watchdog.test; do
             rc=0
-            # shellcheck disable=SC2086
-            $watchdog bash "$t" >"$t.log" 2>&1 || rc=$?
+            run_with_timeout 600 bash "$t" >"$t.log" 2>&1 || rc=$?
             case "$rc" in
               0) pass=$((pass + 1)); echo "PASS $t" ;;
               77) skip=$((skip + 1)) skipped="$skipped $t"; echo "SKIP $t" ;;
+              124) fail=$((fail + 1)) failed="$failed $t"; echo "FAIL $t (timed out, tree killed)" ;;
               *)
                 fail=$((fail + 1)) failed="$failed $t"
                 echo "FAIL $t (exit $rc)"
                 # These assert with `test "$(...)" == "..." || exit 1`, which
-                # says nothing at all on failure. Re-run traced.
-                bash -x "$t" >>"$t.log" 2>&1 || true
+                # says nothing at all on failure. Re-run traced, still bounded.
+                run_with_timeout 600 bash -x "$t" >>"$t.log" 2>&1 || true
                 tail -n 25 "$t.log" | sed 's/^/      /'
                 ;;
             esac

tests/58_watchdog.test

@@ -0,0 +1,73 @@
+#!/bin/bash
+#
+# Unit-tests the suite watchdog (run_with_timeout/kill_tree in testlib.sh): a
+# hung child must be killed tree-and-all and reported 124, never left to orphan
+# a spinning httrack.exe. Regression guard for the Windows x64 hangs that read as
+# "the hosted runner lost communication with the server".
+
+set -euo pipefail
+
+testdir=$(cd "$(dirname "$0")" && pwd)
+# shellcheck source=tests/testlib.sh
+. "${testdir}/testlib.sh"
+
+tmp=$(mktemp -d "${TMPDIR:-/tmp}/httrack_wd.XXXXXX")
+trap 'rm -rf "$tmp"' EXIT
+fail() {
+    echo "FAIL: $*" >&2
+    exit 1
+}
+
+# Exit status passes through when the command finishes in time.
+rc=0 && run_with_timeout 30 sh -c 'exit 0' || rc=$?
+test "$rc" -eq 0 || fail "clean exit reported $rc"
+rc=0 && run_with_timeout 30 sh -c 'exit 7' || rc=$?
+test "$rc" -eq 7 || fail "exit 7 reported $rc"
+
+# An overrunning command is reported 124, near the deadline not the child's end.
+start=$SECONDS
+rc=0
+if is_windows; then
+    run_with_timeout 2 ping -n 30 127.0.0.1 || rc=$?
+else
+    run_with_timeout 2 sleep 30 || rc=$?
+fi
+test "$rc" -eq 124 || fail "hang reported $rc, want 124"
+test "$((SECONDS - start))" -lt 15 || fail "watchdog fired late"
+
+# The native descendant tree is really dead, not orphaned. On Windows an outer
+# MSYS bash with a native ping grandchild is the exact case signals can't reap;
+# on POSIX a surviving grandchild would touch the marker after we stop waiting.
+rc=0
+if is_windows; then
+    # Existence by exact Windows PID, not a global ping.exe count: the timing
+    # sub-test above leaves a still-dying ping that a count would race. Plain
+    # tasklist, no switches (the workflow's MSYS2_ARG_CONV_EXCL='*' mangles a
+    # //FI filter arg into a silent no-match); $2 is the PID column.
+    alive() { tasklist 2>/dev/null | awk -v p="$1" '$2 == p {f = 1} END {exit !f}'; }
+    # The grandchild ping records its own Windows PID: non-empty proves it ran
+    # (positive control, so the reap check can't pass vacuously), then it must go.
+    gw="$tmp/gcwinpid"
+    # `wait` keeps the bash alive on the ping past the deadline; without it the
+    # bash exits at once and run_with_timeout returns 0 instead of timing out.
+    # shellcheck disable=SC2016 # $1 expands in the bash -c child, by design
+    run_with_timeout 2 bash -c 'ping -n 30 127.0.0.1 >/dev/null 2>&1 & cat /proc/$!/winpid >"$1"; wait' _ "$gw" || rc=$?
+    test "$rc" -eq 124 || fail "grandchild hang reported $rc"
+    w=$(cat "$gw" 2>/dev/null)
+    test -n "$w" || fail "positive control: grandchild ping never started"
+    sleep 1
+    ! alive "$w" || fail "native ping grandchild (pid $w) survived the watchdog"
+else
+    started="$tmp/started" marker="$tmp/alive"
+    # The grandchild (backgrounded subshell) marks that it ran, then would touch
+    # the marker after the deadline; killing only the direct child orphans it, so
+    # the marker still appears. started present + marker absent = ran and reaped.
+    # shellcheck disable=SC2016 # $1/$2 expand in the sh -c child, by design
+    run_with_timeout 2 sh -c '{ : >"$1"; sleep 10; : >"$2"; } & wait' _ "$started" "$marker" || rc=$?
+    test "$rc" -eq 124 || fail "grandchild hang reported $rc"
+    test -e "$started" || fail "positive control: grandchild never ran"
+    sleep 12
+    test ! -e "$marker" || fail "watchdog left a grandchild running"
+fi
+
+echo "watchdog OK"

tests/Makefile.am

@@ -134,6 +134,7 @@ TESTS = \
 	54_local-update-truncate-purge.test \
 	55_local-chunked.test \
 	56_local-proxy-noleak.test \
-	57_local-proxy-connect.test
+	57_local-proxy-connect.test \
+	58_watchdog.test
 
 CLEANFILES = check-network_sh.cache

tests/testlib.sh

@@ -43,3 +43,50 @@ stop_server() {
     wait "$1" 2>/dev/null || true
     return 0
 }
+
+# Kill a backgrounded job and its whole descendant tree. POSIX: the caller must
+# have put the job in its own process group (run_with_timeout does) so we signal
+# the group; a bare kill would orphan the grandchildren. Windows: the tree is
+# native processes MSYS can't signal, so taskkill /T ends it by Windows PID.
+# Single-slash switches: the workflow sets MSYS_NO_PATHCONV/MSYS2_ARG_CONV_EXCL,
+# so args pass verbatim and a //T would reach taskkill unfolded and be rejected.
+kill_tree() {
+    local pid=$1
+    if is_windows; then
+        local winpid=
+        test -r "/proc/$pid/winpid" && winpid=$(cat "/proc/$pid/winpid" 2>/dev/null)
+        if test -n "$winpid"; then
+            taskkill /F /T /PID "$winpid" >/dev/null 2>&1 || true
+        else
+            # The offline suite runs serially, so no wanted process races this.
+            taskkill /F /IM httrack.exe >/dev/null 2>&1 || true
+            taskkill /F /IM python.exe >/dev/null 2>&1 || true
+        fi
+    fi
+    kill -9 -"$pid" 2>/dev/null || kill -9 "$pid" 2>/dev/null || true
+}
+
+# Run "$@" under a wall-clock deadline of $1 seconds; return its exit status, or
+# 124 if it overran and was killed. timeout(1) is unusable here: it's absent on
+# macOS and its signals can't reap httrack.exe on Windows. We poll and kill_tree.
+run_with_timeout() {
+    local secs=$1
+    shift
+    local had_m=
+    case "$-" in *m*) had_m=1 ;; esac
+    is_windows || set -m # own process group, so kill_tree can signal the group
+    "$@" &
+    local pid=$!
+    test -n "$had_m" || is_windows || set +m
+    local waited=0
+    while kill -0 "$pid" 2>/dev/null; do
+        if test "$waited" -ge "$secs"; then
+            kill_tree "$pid"
+            wait "$pid" 2>/dev/null || true
+            return 124
+        fi
+        sleep 1
+        waited=$((waited + 1))
+    done
+    wait "$pid"
+}