diff --git a/tests/helpers/mock_remote_server.py b/tests/helpers/mock_remote_server.py index 20a0c7ae0..ed1056ecb 100644 --- a/tests/helpers/mock_remote_server.py +++ b/tests/helpers/mock_remote_server.py @@ -742,7 +742,57 @@ def do_POST(self): self._send_json(404, {"error": "not found"}) +def _close_inherited_fds(): + # #1107: this mock outlives the command that starts it -- every test backgrounds + # it and it runs until killed -- so every descriptor it inherits it holds for + # as long as it runs. Under a parallel bats run that inheritance included the + # harness's own high-numbered pipes (measured: fd 143 and 146), and bats then + # waited forever for an EOF this process was keeping from arriving, hanging the + # whole shard even though every test had already reported ok. The start sites + # close fd 3 by name (`3>&-`), which never reaches 143/146. + # + # The fix mirrors scripts/lib/close-fds.sh for the sync engine: close every + # descriptor at or above 3 that we inherited, BEFORE the listen socket is + # opened so it gets a fresh, un-closed fd. 0/1/2 are kept -- stdin is + # /dev/null, the port is printed on stdout, and stderr carries the log. + # Enumerate /dev/fd when present (exact, and reaches the high fds a fixed + # range would still cover but is cheaper than sweeping to the rlimit); fall + # back to a bounded range otherwise. Closing an already-closed fd is ignored. + keep = (0, 1, 2) + try: + fds = [int(e) for e in os.listdir("/dev/fd") if e.isdigit()] + except OSError: + fds = list(range(3, 256)) + for fd in fds: + if fd in keep: + continue + try: + os.close(fd) + except OSError: + pass + + +def _report_open_fds(path): + # Test-only (#1107). After the close above, write this process's open + # descriptors so a test can prove nothing the harness opened survived. It is + # read against a BASELINE (this same report with nothing extra inherited), + # the way test_engine_inherited_fds.bats does for the engine: the descriptor + # the listing itself opens appears in both and cancels, so only a leaked + # inherited fd shows as a difference. The list is taken before this file is + # opened, so the report's own fd is not counted. + try: + entries = sorted(int(e) for e in os.listdir("/dev/fd") if e.isdigit()) + except OSError: + entries = [] + with open(path, "w") as fh: + fh.write("".join("%d\n" % fd for fd in entries)) + + def main(): + _close_inherited_fds() + report = os.environ.get("MOCK_FD_REPORT", "") + if report: + _report_open_fds(report) port = int(sys.argv[1]) if len(sys.argv) > 1 else 0 server = LoopbackHTTPServer(("127.0.0.1", port), Handler) print(server.server_port, flush=True) diff --git a/tests/test_engine_inherited_fds.bats b/tests/test_engine_inherited_fds.bats index bf39999e1..26fb78f2d 100644 --- a/tests/test_engine_inherited_fds.bats +++ b/tests/test_engine_inherited_fds.bats @@ -116,3 +116,50 @@ _write_self_reporter() { grep -qx MARKER-ERR "$WORK/err.txt" grep -qx ping "$FD_REPORT.stdin" } + +# The mock remote server has the same shape as the engine above: a test +# backgrounds it and it runs until killed, so any harness descriptor it inherits +# and never closes it holds for the life of the shard. Under a parallel bats run +# that was the harness's own high pipes -- measured, fd 143 and 146, held by six +# survivors -- and bats then waited forever for an EOF the mock kept from +# arriving, hanging the shard with every test already ok (#1107). The start sites +# close fd 3 by name (`3>&-`), which never reaches 143/146; the mock closes the +# range itself, mirroring the engine. Measured against a baseline, the descriptor +# the listing opens appears in both and cancels; only a surviving inherited fd +# shows as a difference. +@test "the mock remote server does not inherit descriptors the harness opened (#1107)" { + local mock="$BATS_TEST_DIRNAME/helpers/mock_remote_server.py" + local py="${MOCK_PYTHON3:-$(command -v python3 || true)}" + [ -n "$py" ] || skip "python3 not on PATH" + + # Baseline: nothing beyond 0/1/2 inherited. + MOCK_FD_REPORT="$WORK/fd-base" "$py" "$mock" 0 \ + "$WORK/base.port" 2>/dev/null 3>&- & + local bpid=$! i + for i in $(seq 1 100); do [ -s "$WORK/fd-base" ] && break; sleep 0.1; done + kill "$bpid" 2>/dev/null || true; wait "$bpid" 2>/dev/null || true + local baseline; baseline="$(cat "$WORK/fd-base" 2>/dev/null)" + + # Measured: high pipes inherited, exactly what the `3>&-` at each start site + # leaves open. The numbers are the ones the captured hang held; the assertion is + # only that a high inherited pipe does not survive into the server. + ( + exec 143> >(cat >/dev/null) 146> >(cat >/dev/null) + MOCK_FD_REPORT="$WORK/fd-meas" "$py" "$mock" 0 \ + "$WORK/meas.port" 2>/dev/null 3>&- & + mpid=$! + for i in $(seq 1 100); do [ -s "$WORK/fd-meas" ] && break; sleep 0.1; done + kill "$mpid" 2>/dev/null || true; wait "$mpid" 2>/dev/null || true + ) + local measured; measured="$(cat "$WORK/fd-meas" 2>/dev/null)" + + [ -n "$baseline$measured" ] || { echo "the report never appeared"; false; } + [ "$baseline" = "$measured" ] || { + echo "baseline [$baseline] measured [$measured]" + false + } + if grep -qxE '143|146' <<<"$measured"; then + echo "an inherited harness pipe survived into the server: [$measured]" + false + fi +} diff --git a/tests/test_team_list.bats b/tests/test_team_list.bats index 9b1f52393..8dd6ef6af 100644 --- a/tests/test_team_list.bats +++ b/tests/test_team_list.bats @@ -10,6 +10,12 @@ setup() { } teardown() { + # (#1107) A test that fails between starting the mock and its inline kill would + # otherwise leak the process. It is harmless now that the mock closes the + # descriptors it inherits, but a leaked server still holds a port and memory, so + # reap it here too. bats runs each test in its own subshell, so MOCK_PID is + # unset for a case that never started one. + [ -n "${MOCK_PID:-}" ] && kill "$MOCK_PID" 2>/dev/null || true teardown_test_env } @@ -53,7 +59,8 @@ json_field() { "$MOCK_PYTHON3" "$BATS_TEST_DIRNAME/helpers/mock_remote_server.py" 0 \ "$TEST_SKILL_DIR/server.port" 2>"$TEST_SKILL_DIR/server.log" 3>&- & - local mock_pid=$! + MOCK_PID=$! + local mock_pid=$MOCK_PID wait_for_file_contains "$TEST_SKILL_DIR/server.port" '^[0-9][0-9]*$' local mock_port; mock_port="$(cat "$TEST_SKILL_DIR/server.port")" local endpoint="http://127.0.0.1:$mock_port" @@ -76,7 +83,8 @@ json_field() { bash "$SCRIPTS/join.sh" myteam alice claude-code /tmp/project-a "$MOCK_PYTHON3" "$BATS_TEST_DIRNAME/helpers/mock_remote_server.py" 0 \ "$TEST_SKILL_DIR/server.port" 2>"$TEST_SKILL_DIR/server.log" 3>&- & - local mock_pid=$! + MOCK_PID=$! + local mock_pid=$MOCK_PID wait_for_file_contains "$TEST_SKILL_DIR/server.port" '^[0-9][0-9]*$' local mock_port; mock_port="$(cat "$TEST_SKILL_DIR/server.port")" local endpoint="http://127.0.0.1:$mock_port"