Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 50 additions & 0 deletions tests/helpers/mock_remote_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
47 changes: 47 additions & 0 deletions tests/test_engine_inherited_fds.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
</dev/null >"$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 \
</dev/null >"$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
}
12 changes: 10 additions & 2 deletions tests/test_team_list.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -53,7 +59,8 @@ json_field() {

"$MOCK_PYTHON3" "$BATS_TEST_DIRNAME/helpers/mock_remote_server.py" 0 \
</dev/null > "$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"
Expand All @@ -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 \
</dev/null > "$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"
Expand Down
Loading