From ebc7633f7ea8b443f07ef6dab3dcb18bab339cc3 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Thu, 1 Oct 2026 01:35:47 -0500 Subject: [PATCH 01/15] tools: queue heavy QA jobs behind the shared local lock Adds tools/qa-resource/qa_resource.py, a stdlib-only runner that takes package, render or simulator jobs one at a time under the existing ~/.codex/local-ios-qa.lock. It checks memory, disk and other heavy processes at admission, waits for foreign builds to exit with kqueue without signaling them, and ignores the agent-device testCommand service for package and render jobs. On cancel or timeout it stops only its own child process group, and shuts down only a simulator it booted. Parents block on `wait`, which takes the job's liveness flock, or read job.json. Co-Authored-By: Claude Opus 5.5 (1M context) --- tools/qa-resource/README.md | 39 ++ tools/qa-resource/qa_resource.py | 757 +++++++++++++++++++++++ tools/qa-resource/test_qa_resource.py | 833 ++++++++++++++++++++++++++ 3 files changed, 1629 insertions(+) create mode 100644 tools/qa-resource/README.md create mode 100644 tools/qa-resource/qa_resource.py create mode 100644 tools/qa-resource/test_qa_resource.py diff --git a/tools/qa-resource/README.md b/tools/qa-resource/README.md new file mode 100644 index 000000000..6185920db --- /dev/null +++ b/tools/qa-resource/README.md @@ -0,0 +1,39 @@ +# Run heavy QA jobs + +Use this queue for package builds and tests, Blender renders, and simulator runs. Run quick unit checks directly; they never need to wait for this queue. Python 3 on macOS is the only dependency. + +From the worktree, submit a package job and wait for its result: + +```sh +python3 tools/qa-resource/qa_resource.py submit --job-id package-001 --kind package --cwd "$PWD" --admission-deadline 1200 --timeout 600 -- swift test --package-path ios/HouseScanKit --jobs 1 +python3 tools/qa-resource/qa_resource.py wait --job-id package-001 --max-wait 1800 +python3 tools/qa-resource/qa_resource.py status --job-id package-001 +python3 tools/qa-resource/qa_resource.py status +python3 tools/qa-resource/qa_resource.py cancel --job-id package-001 +``` + +Use `run` instead of `submit` to stay in the foreground. Use `--kind render` for renders. For simulator work, use `--kind simulator --device UDID`; other kinds reject `--device`. Pick a new job id for every attempt. Existing job directories are never overwritten. + +All subcommands accept `--state-dir DIR` and `--lock FILE`, before or after the subcommand. Defaults are `~/.codex/qa-resource` and the existing shared `~/.codex/local-ios-qa.lock`. Do not delete or replace the shared lock file. + +Read `~/.codex/qa-resource/jobs/ID/job.json` for status, reasons, timestamps, ownership, and cleanup errors. `output.log` contains child output. `runner.log` contains timestamped runner events, also echoed by `run`. `submit` prints one JSON receipt with `jobId`, `jobDir`, `resultFile`, and runner `pid`. `wait` uses the job's kernel liveness lock, not polling. Its default maximum wait is 86400 seconds. + +Nothing pushes a completion event to T3 or a manager session. A parent learns the result in one of two ways. It can block in `wait`, running it again with the same id if its shell tool's time limit is shorter than the job. Or it can start `run` as a background command and let its host report the exit. Either way, `job.json` holds the result. + +Read `job.json`, not just the exit code. `run` and `wait` return the child's code for success or failure, 124 for child timeout, 130 for cancellation, 75 for admission timeout, and 125 for sensing or cleanup errors. `wait` returns 3 when its own deadline expires without affecting the job, 4 for a runner that died without a terminal result, and 2 for an unknown job. Child exit codes can overlap these values. If a child dies from signal N, `run` and `wait` return 128+N. `job.json` keeps the negative child return code in `exitCode` and N in `signal`. + +Admission requires memory pressure level 1 or 2, at least 35% memory free, and at least 5 GiB free on the HOME volume. These are operational limits copied from `qa-slot.py`, not calibrated measurements. Unreadable resources fail admission. Heavy build and render processes block admission, except a verified agent-device `testCommand` service. That service blocks a simulator job only on the same device. The named simulator must exist and be Shutdown; other booted devices do not block. Package and render jobs never call `simctl`. + +Heavy-process exit waits use kqueue. Memory recovery, disk space, and device state have no stdlib kernel notification here, so they are resampled every 30 seconds. The global lock stays held during resource waits. The admission deadline covers queueing and resource checks. Simulator boot and bootstatus each have a fixed 300-second limit, an operational limit rather than a measured startup time. Cancellation waits for the boot result and ownership record. If boot times out, the runner reports the device state but never shuts it down without proof of ownership. The child timeout starts separately. + +## What the tool cannot guarantee + +The queue cannot identify a foreign shared-lock holder or attribute a heavy process to a project. FIFO ordering applies only to this tool's tickets; `qa-slot.py` and `package-run.py` still race for the shared flock. Cancellation signals only a verified live runner, which stops its own child group and shuts down only the device it booted. The runner handles SIGTERM, SIGINT, and SIGHUP; later signals cannot interrupt cleanup. A stale job's recorded group and booted-device ownership are reported, never cleaned. Inspect those manually before reusing a device. + +Cancel can signal a reused PID if reuse happens between the runner start-time check and SIGTERM; eliminating that window on macOS needs per-job IPC. + +Boot ownership does not identify a simulator session; if another lane shuts down and re-boots the same UDID mid-job, final shutdown affects their session. + +A stuck helper is killed at its subprocess timeout, but `subprocess.run` then reaps it without a deadline, so that failure can exceed the stated limits. + +If group cleanup fails, the runner holds the shared lock for up to 300 more seconds, an operational limit rather than a measured recovery time. It releases the lock when no live members remain or the hold expires, records the outcome in `cleanupError`, and keeps status `cleanup_failed`. diff --git a/tools/qa-resource/qa_resource.py b/tools/qa-resource/qa_resource.py new file mode 100644 index 000000000..40c5234d3 --- /dev/null +++ b/tools/qa-resource/qa_resource.py @@ -0,0 +1,757 @@ +#!/usr/bin/env python3 +"""Serialize heavyweight local QA work without taking ownership of other jobs.""" +import argparse +from contextlib import closing, contextmanager +from datetime import datetime, timezone +import errno +import fcntl +import json +import math +import os +from pathlib import Path +import re +import select +import shlex +import signal +import subprocess +import sys +import time + +# Operational limits copied from qa-slot.py, not calibrated measurements. +MIN_MEMORY_PERCENT = 35 +MIN_DISK_BYTES = 5 * 1024**3 +ALLOWED_PRESSURES = (1, 2) +RESAMPLE_SECONDS = 30 +CLEANUP_GRACE_SECONDS = 10 +CLEANUP_CONFIRM_SECONDS = 3 +# Operational hold limit, not a measured time for an unkillable process to exit. +CLEANUP_HOLD_SECONDS = 300 +SENSING_TIMEOUT = 10 +# Operational boot limit, not a measured simulator startup time. +BOOT_TIMEOUT_SECONDS = 300 +RUNNER_SIGNALS = (signal.SIGTERM, signal.SIGINT, signal.SIGHUP) +HEAVY_NAMES = {"xcodebuild", "swift-build", "swift-test", "swift-frontend", + "swift-driver", "blender"} +SERVICE_TEST = "AgentDeviceRunnerUITests/RunnerTests/testCommand" +TERMINAL = {"succeeded", "failed", "timed_out", "cancelled", "admission_timeout", + "error", "cleanup_failed"} + + +class Cancelled(Exception): + def __init__(self, signum): + self.signum = signum + + +class DeadlineExpired(Exception): + pass + + +def timestamp(): + return datetime.now(timezone.utc).isoformat() + + +@contextmanager +def bounded(seconds): + """Interrupt even a blocking flock; Python retries EINTR unless handlers raise.""" + if seconds <= 0: + raise DeadlineExpired() + old = signal.getsignal(signal.SIGALRM) + def alarm(signum, frame): + raise DeadlineExpired() + signal.signal(signal.SIGALRM, alarm) + previous = signal.setitimer(signal.ITIMER_REAL, seconds) + try: + yield + finally: + signal.setitimer(signal.ITIMER_REAL, 0) + signal.signal(signal.SIGALRM, old) + if previous[0]: + signal.setitimer(signal.ITIMER_REAL, *previous) + + +def command_output(argv): + # subprocess.run kills a timed-out helper, but its subsequent reap is unbounded. + result = subprocess.run(argv, capture_output=True, text=True, timeout=SENSING_TIMEOUT) + if result.returncode: + raise RuntimeError(f"{' '.join(argv)} exited {result.returncode}: {result.stderr.strip()}") + return result.stdout.strip() + + +def runner_start(pid): + return command_output(["ps", "-o", "lstart=", "-p", str(pid)]) + + +def sense_memory(): + try: + pressure = int(command_output(["sysctl", "-n", "kern.memorystatus_vm_pressure_level"])) + text = command_output(["memory_pressure"]) + match = re.search(r"System-wide memory free percentage: (\d+)%", text) + if not match or not 0 <= int(match[1]) <= 100: + raise ValueError("missing or invalid free percentage") + return pressure, int(match[1]) + except (Cancelled, DeadlineExpired): + raise + except Exception as error: + raise RuntimeError(f"cannot read memory: {error}") from error + + +def sense_disk(): + # df uses the HOME volume and lets subprocess tests substitute the executable. + text = command_output(["df", "-Pk", str(Path.home())]) + match = re.search(r"^.+?\s+\d+\s+\d+\s+(\d+)\s+\d+%\s+", text.splitlines()[-1]) if text else None + if match is None: + raise RuntimeError(f"cannot read free disk bytes: {text!r}") + return int(match[1]) * 1024 + + +def list_processes(): + rows = command_output(["ps", "-axo", "pid=,pgid=,stat=,comm="]) + processes = [] + for row in rows.splitlines(): + pid, pgid, state, comm = row.strip().split(maxsplit=3) + if int(pid) == os.getpid() or state.startswith("Z") or Path(comm).name not in HEAVY_NAMES: + continue + # comm may contain spaces. Only the first three columns are split. + # A timed-out ps is killed, but subprocess.run's subsequent reap is unbounded. + args = subprocess.run(["ps", "-o", "args=", "-p", pid], capture_output=True, + text=True, timeout=SENSING_TIMEOUT) + if args.returncode: # The process may have exited between the two ps calls. + if args.returncode == 1 and not args.stdout.strip(): + continue + raise RuntimeError(f"cannot inspect arguments for process {pid}") + processes.append(dict(pid=int(pid), pgid=int(pgid), stat=state, + comm=comm, args=args.stdout.strip())) + return processes + + +def option_values(tokens, option): + values = [] + for index, token in enumerate(tokens): + if token == option and index + 1 < len(tokens): + values.append(tokens[index + 1]) + elif token.startswith(option + ":"): + values.append(token[len(option) + 1:]) + return values + + +def classify_process(process, kind, device=None): + """Return blocker, ignored, or irrelevant without inspecting the machine.""" + if process.get("stat", "").startswith("Z") or Path(process["comm"]).name not in HEAVY_NAMES: + return "irrelevant" + try: + tokens = shlex.split(process["args"]) + except ValueError: + return "blocker" + service = (Path(process["comm"]).name == "xcodebuild" + and "test-without-building" in tokens + and SERVICE_TEST in option_values(tokens, "-only-testing")) + if not service: + return "blocker" + destinations = [] + for index, token in enumerate(tokens): + if token == "-destination": + # ps args does not preserve argv quoting around 'iOS Simulator'. + # Join the value through the next option to retain its comma-delimited id. + end = index + 1 + while end < len(tokens) and not tokens[end].startswith("-"): + end += 1 + destinations.append(" ".join(tokens[index + 1:end])) + matches = any(re.search(r"(?:^|,)\s*id=" + re.escape(device or "") + r"(?:,|$)", d) + for d in destinations) + return "blocker" if kind == "simulator" and matches else "ignored" + + +def resource_blockers(pressure, memory_percent, free_bytes): + reasons = [] + if pressure not in ALLOWED_PRESSURES or memory_percent < MIN_MEMORY_PERCENT: + reasons.append(f"memory pressure {pressure}, {memory_percent}% free") + if free_bytes < MIN_DISK_BYTES: + reasons.append("less than 5 GiB free on HOME volume") + return reasons + + +def simctl(*args, timeout=SENSING_TIMEOUT): + # A timeout kills xcrun, but subprocess.run does not bound the subsequent reap. + return subprocess.run(["xcrun", "simctl", *args], capture_output=True, + text=True, timeout=timeout) + + +def sense_devices(): + result = simctl("list", "devices", "-j") + if result.returncode: + raise RuntimeError(f"xcrun simctl list devices -j exited {result.returncode}") + try: + return [device for group in json.loads(result.stdout)["devices"].values() for device in group] + except (ValueError, KeyError, TypeError) as error: + raise RuntimeError("cannot parse xcrun simctl list devices -j") from error + + +def wait_process_exit(pids, seconds): + with closing(select.kqueue()) as queue: + registered = False + for pid in pids: + try: + queue.control([select.kevent(pid, filter=select.KQ_FILTER_PROC, + flags=select.KQ_EV_ADD | select.KQ_EV_ONESHOT, + fflags=select.KQ_NOTE_EXIT)], 0, 0) + registered = True + except OSError as error: + if error.errno != errno.ESRCH: + raise + return # Resample immediately if a blocker has already exited. + if registered: + queue.control(None, len(pids), seconds) + + +def group_has_members(pgid): + rows = command_output(["ps", "-axo", "pgid=,stat="]) + return any(int(group) == pgid and not state.startswith("Z") + for group, state in (row.split() for row in rows.splitlines())) + + +def signal_owned_group(pgid, signum): + try: + os.killpg(pgid, signum) + return True + except ProcessLookupError: + return False + except PermissionError as error: + # Darwin can return EPERM for zombies. Inspect before declaring release. + if not group_has_members(pgid): + return False + raise RuntimeError(f"cannot signal owned group {pgid}; live members remain") from error + + +def wait_child_exit(child, seconds): + """Observe exit without reaping; the zombie leader pins its process-group id.""" + with closing(select.kqueue()) as queue: + event = select.kevent(child.pid, filter=select.KQ_FILTER_PROC, + flags=select.KQ_EV_ADD | select.KQ_EV_ONESHOT, + fflags=select.KQ_NOTE_EXIT) + try: + return bool(queue.control([event], 1, max(0, seconds))) + except OSError as error: + if error.errno != errno.ESRCH: + raise + # This is our direct, unreaped child, so ESRCH means it already exited. + return True + + +def stop_owned_command(child): + if child is None: + return None + # Never poll or reap before the last group operation: the leader pins the pgid. + if signal_owned_group(child.pid, signal.SIGTERM): + deadline = time.monotonic() + CLEANUP_GRACE_SECONDS + while time.monotonic() < deadline: + if not group_has_members(child.pid): + break + time.sleep(0.05) + else: + signal_owned_group(child.pid, signal.SIGKILL) + deadline = time.monotonic() + CLEANUP_CONFIRM_SECONDS + while group_has_members(child.pid): + if time.monotonic() >= deadline: + raise RuntimeError(f"could not confirm owned group {child.pid} gone") + time.sleep(0.05) + return child.wait(timeout=CLEANUP_CONFIRM_SECONDS) + + +def hold_owned_group(child): + deadline = time.monotonic() + CLEANUP_HOLD_SECONDS + while True: + try: + if not group_has_members(child.pid): + return "no live members remained when the shared QA lock was released" + remaining = "live members remained" + except Exception as error: + remaining = f"live membership was unknown ({error})" + seconds = deadline - time.monotonic() + if seconds <= 0: + return f"{remaining} when the shared QA lock was released after the cleanup hold expired" + time.sleep(min(1, seconds)) + + +def cleanup_step(errors, label, action): + try: + action() + except Exception as error: + errors.append(f"{label}: {error}") + + +@contextmanager +def deferred_signals(signals): + pending = [] + def record(signum, frame): + if not pending: + pending.append(signum) + previous = {sig: signal.signal(sig, record) for sig in signals} + try: + yield pending + finally: + errors = [] + for sig, handler in previous.items(): + try: + signal.signal(sig, handler) + except Cancelled as error: + if not pending: + pending.append(error.signum) + except Exception as error: + errors.append(f"restore signal {sig}: {error}") + if errors: + raise RuntimeError("; ".join(errors)) + + +def write_job(directory, job): + temporary = directory / "job.json.tmp" + temporary.write_text(json.dumps(job, sort_keys=True) + "\n") + os.replace(temporary, directory / "job.json") + + +def read_job(directory): + return json.loads((directory / "job.json").read_text()) + + +def exit_code(job): + child_code = job["exitCode"] or 0 + if child_code < 0: + child_code = 128 - child_code + return {"timed_out": 124, "cancelled": 130, "admission_timeout": 75, + "error": 125, "cleanup_failed": 125}.get(job["status"], child_code) + + +def create_job(args): + directory = args.state_dir / "jobs" / args.job_id + directory.parent.mkdir(parents=True, exist_ok=True) + try: + directory.mkdir() + except FileExistsError as error: + raise ValueError(f"job {args.job_id} already exists; choose a new id") from error + live = (directory / "live.lock").open("a+") + fcntl.flock(live, fcntl.LOCK_EX | fcntl.LOCK_NB) + job = dict(jobId=args.job_id, kind=args.kind, argv=args.argv, cwd=str(args.cwd), + device=args.device, status="queued", reasons=[], runnerPid=None, + runnerStart=None, childPgid=None, bootedByJob=False, exitCode=None, + signal=None, error=None, shutdownExitCode=None, cleanupError=None, + queuedAt=timestamp(), admittedAt=None, finishedAt=None, + admissionDeadline=args.admission_deadline, timeout=args.timeout, + ignoredProcesses=[], logs={name: str(directory / (name + ".log")) + for name in ("output", "runner")}) + (directory / "output.log").touch() + (directory / "runner.log").touch() + write_job(directory, job) + return directory, live, job + + +def acquire_ticket(state_dir, job_id, deadline): + queue = state_dir / "queue" + queue.mkdir(parents=True, exist_ok=True) + with (queue / ".guard").open("a+") as guard: + with bounded(deadline - time.monotonic()): + fcntl.flock(guard, fcntl.LOCK_EX) + counter = queue / "counter" + seq = int(counter.read_text()) + 1 if counter.exists() else 1 + counter.write_text(str(seq)) + path = queue / f"{seq:012d}.{job_id}" + ticket = path.open("a+") + fcntl.flock(ticket, fcntl.LOCK_EX | fcntl.LOCK_NB) + return path, ticket + + +def release_ticket(path, ticket): + errors = [] + # Unlink while locked so the next head cannot mistake us for a live ticket. + if path is not None: + cleanup_step(errors, "unlink queue ticket", lambda: path.unlink(missing_ok=True)) + if ticket is not None and not ticket.closed: + cleanup_step(errors, "unlock queue ticket", lambda: fcntl.flock(ticket, fcntl.LOCK_UN)) + cleanup_step(errors, "close queue ticket", ticket.close) + return errors + + +def run_job(directory, live, job, lock_path, echo=True, owned_ticket=None, deadline=None): + global_lock = child = None + child_finished = False + cleanup_errors = [] + ticket_path, ticket = owned_ticket if owned_ticket else (None, None) + if deadline is None: + deadline = time.monotonic() + job["admissionDeadline"] + log = (directory / "runner.log").open("a", buffering=1) + def update(**fields): + job.update(fields) + write_job(directory, job) + line = f"{timestamp()} {job['status']} " + json.dumps(fields, sort_keys=True) + log.write(line + "\n") + if echo: + print(line, flush=True) + def interrupted(signum, frame): + # A second signal must not escape while the first exception is handled. + for sig in RUNNER_SIGNALS: + signal.signal(sig, signal.SIG_IGN) + raise Cancelled(signum) + old_signals = {sig: signal.signal(sig, interrupted) for sig in RUNNER_SIGNALS} + try: + update(runnerPid=os.getpid(), runnerStart=runner_start(os.getpid())) + if ticket is None: + ticket_path, ticket = acquire_ticket(directory.parent.parent, job["jobId"], deadline) + while True: + lower = sorted(p for p in ticket_path.parent.iterdir() + if re.match(r"\d{12}\.", p.name) and p.name < ticket_path.name) + if not lower: + break + update(status="waiting", reasons=[f"queued behind {lower[-1].name}"]) + try: + prior = lower[-1].open("r") + except FileNotFoundError: + continue + with prior, bounded(deadline - time.monotonic()): + fcntl.flock(prior, fcntl.LOCK_SH) + lower[-1].unlink(missing_ok=True) + lock_path.parent.mkdir(parents=True, exist_ok=True) + # This inode is shared with existing tools. Never unlink or replace it. + global_lock = lock_path.open("a+") + try: + fcntl.flock(global_lock, fcntl.LOCK_EX | fcntl.LOCK_NB) + except BlockingIOError: + update(status="waiting", reasons=["shared QA lock held by another holder"]) + with bounded(deadline - time.monotonic()): + fcntl.flock(global_lock, fcntl.LOCK_EX) + cleanup_errors.extend(release_ticket(ticket_path, ticket)) + while True: + with bounded(deadline - time.monotonic()): + pressure, memory = sense_memory() + reasons = resource_blockers(pressure, memory, sense_disk()) + heavy, ignored = [], [] + for process in list_processes(): + classification = classify_process(process, job["kind"], job["device"]) + if classification == "blocker": + heavy.append(process["pid"]) + reasons.append(f"heavy process {process['pid']} {process['comm']}") + elif classification == "ignored": + ignored.append(dict(pid=process["pid"], reason="verified agent-device service on another or unneeded device")) + if job["kind"] == "simulator": + devices = sense_devices() + target = next((d for d in devices if d["udid"] == job["device"]), None) + if target is None: + raise RuntimeError(f"device {job['device']} does not exist; no device is created") + # Other booted devices do not block; memory limits cover their load. + if target["state"] != "Shutdown": + reasons.append(f"device {job['device']} is {target['state']}") + update(status="waiting", reasons=reasons, ignoredProcesses=ignored) + if not reasons: + break + remaining = deadline - time.monotonic() + if remaining <= 0: + raise DeadlineExpired() + if heavy: + wait_process_exit(heavy, min(remaining, RESAMPLE_SECONDS)) + else: + time.sleep(min(remaining, RESAMPLE_SECONDS)) + if job["kind"] == "simulator": + # CoreSimulator can finish a boot after xcrun is interrupted. Defer + # cancellation until success and its ownership record are both known. + previous_mask = signal.pthread_sigmask(signal.SIG_BLOCK, RUNNER_SIGNALS) + try: + try: + result = simctl("boot", job["device"], timeout=BOOT_TIMEOUT_SECONDS) + except subprocess.TimeoutExpired as error: + try: + devices = sense_devices() + target = next((d for d in devices if d["udid"] == job["device"]), None) + state = f"device is {target['state']}" if target else "device is missing" + except Exception as inspection_error: + state = f"device state unreadable: {inspection_error}" + raise RuntimeError( + f"boot of {job['device']} did not finish in {BOOT_TIMEOUT_SECONDS:g} s; " + f"{state}; not shut down because this job cannot prove it booted it" + ) from error + if result.returncode: + raise RuntimeError(f"xcrun simctl boot {job['device']} exited {result.returncode}") + update(bootedByJob=True) + finally: + signal.pthread_sigmask(signal.SIG_SETMASK, previous_mask) + try: + result = simctl("bootstatus", job["device"], timeout=BOOT_TIMEOUT_SECONDS) + except subprocess.TimeoutExpired as error: + raise RuntimeError( + f"xcrun simctl bootstatus {job['device']} did not finish in {BOOT_TIMEOUT_SECONDS:g} s" + ) from error + if result.returncode: + raise RuntimeError(f"xcrun simctl bootstatus {job['device']} exited {result.returncode}") + elif time.monotonic() >= deadline: + raise DeadlineExpired() + with (directory / "output.log").open("a") as output: + # Record cancellation without blocking signals inherited by the payload. + with deferred_signals(RUNNER_SIGNALS) as pending: + child = subprocess.Popen(job["argv"], cwd=job["cwd"], start_new_session=True, + stdin=subprocess.DEVNULL, stdout=output, stderr=subprocess.STDOUT) + child_started = time.monotonic() + job["childPgid"] = child.pid + write_job(directory, job) + if pending: + interrupted(pending[0], None) + update(status="running", reasons=[], admittedAt=timestamp(), childPgid=child.pid) + child_finished = wait_child_exit(child, job["timeout"] - (time.monotonic() - child_started)) + if not child_finished: + job.update(status="timed_out", exitCode=124) + except Cancelled as error: + job.update(status="cancelled", exitCode=130, signal=error.signum) + except DeadlineExpired: + job.update(status="admission_timeout", exitCode=75) + except Exception as error: + if isinstance(error, subprocess.TimeoutExpired) and time.monotonic() >= deadline: + job.update(status="admission_timeout", exitCode=75) + else: + job.update(status="error", exitCode=125, error=str(error)) + finally: + def mark_cleanup_errors(): + if cleanup_errors: + job.update(status="cleanup_failed", exitCode=125, cleanupError="; ".join(cleanup_errors)) + for sig in old_signals: + cleanup_step(cleanup_errors, f"ignore signal {sig}", lambda sig=sig: signal.signal(sig, signal.SIG_IGN)) + try: + code = stop_owned_command(child) + if child_finished and job["status"] == "running": + job.update(status="succeeded" if code == 0 else "failed", exitCode=code, + signal=-code if code < 0 else None) + except Exception as error: + cleanup_errors.append(f"stop owned group: {error}") + mark_cleanup_errors() + cleanup_step(cleanup_errors, "write cleanup hold status", lambda: write_job(directory, job)) + if child is not None: + try: + cleanup_errors.append(hold_owned_group(child)) + except Exception as hold_error: + cleanup_errors.append(f"cleanup hold: {hold_error}; live membership unknown at lock release") + # All group signaling and inspection are over before this reap. + cleanup_step(cleanup_errors, "reap owned child after hold", + lambda: child.wait(timeout=CLEANUP_CONFIRM_SECONDS)) + if job["bootedByJob"]: + try: + # Ownership proves boot success, not session identity after another lane reboots this UDID. + result = simctl("shutdown", job["device"]) + job["shutdownExitCode"] = result.returncode + if result.returncode: + raise RuntimeError(f"xcrun simctl shutdown {job['device']} exited {result.returncode}") + except Exception as error: + cleanup_errors.append(str(error)) + mark_cleanup_errors() + cleanup_step(cleanup_errors, "write terminal result", lambda: update(finishedAt=timestamp())) + if global_lock is not None: + cleanup_step(cleanup_errors, "unlock shared QA lock", lambda: fcntl.flock(global_lock, fcntl.LOCK_UN)) + cleanup_step(cleanup_errors, "close shared QA lock", global_lock.close) + cleanup_step(cleanup_errors, "release queue ticket", + lambda: cleanup_errors.extend(release_ticket(ticket_path, ticket))) + cleanup_step(cleanup_errors, "close runner log", log.close) + mark_cleanup_errors() + cleanup_step(cleanup_errors, "write release result", lambda: write_job(directory, job)) + recorded_errors = len(cleanup_errors) + cleanup_step(cleanup_errors, "close liveness lock", live.close) + for sig, previous in old_signals.items(): + cleanup_step(cleanup_errors, f"restore signal {sig}", + lambda sig=sig, previous=previous: signal.signal(sig, previous)) + if len(cleanup_errors) != recorded_errors: + mark_cleanup_errors() + cleanup_step(cleanup_errors, "write finalizer errors", lambda: write_job(directory, job)) + return exit_code(job) + + +def resume(directory, fd, lock_path, ticket_fd, ticket_path, deadline): + directory = Path(directory) + with os.fdopen(fd, "a+") as live, os.fdopen(ticket_fd, "a+") as ticket: + return run_job(directory, live, read_job(directory), Path(lock_path), echo=False, + owned_ticket=(Path(ticket_path), ticket), deadline=deadline) + + +def submit(directory, live, job, lock_path): + # Allocate the ticket before returning the receipt. Runner scheduling must not + # reverse two completed submissions. Both locked descriptions survive exec. + script = ("import sys; sys.path.insert(0, sys.argv[1]); import qa_resource as q; " + "sys.exit(q.resume(sys.argv[2], int(sys.argv[3]), sys.argv[4], " + "int(sys.argv[5]), sys.argv[6], float(sys.argv[7])))") + ticket_path = ticket = runner = None + close_errors = [] + deadline = time.monotonic() + job["admissionDeadline"] + try: + ticket_path, ticket = acquire_ticket(directory.parent.parent, job["jobId"], deadline) + with (directory / "runner.log").open("a") as log: + with deferred_signals((signal.SIGTERM, signal.SIGINT)) as pending: + runner = subprocess.Popen([sys.executable, "-c", script, str(Path(__file__).parent), + str(directory), str(live.fileno()), str(lock_path), + str(ticket.fileno()), str(ticket_path), str(deadline)], + pass_fds=(live.fileno(), ticket.fileno()), start_new_session=True, + stdin=subprocess.DEVNULL, stdout=log, stderr=subprocess.STDOUT) + # A blocked receipt writer must not keep a finished job alive. + cleanup_step(close_errors, "close submitted ticket fd", ticket.close) + cleanup_step(close_errors, "close submitted liveness fd", live.close) + if pending: + raise Cancelled(pending[0]) + if close_errors: + raise RuntimeError("; ".join(close_errors)) + print(json.dumps(dict(jobId=job["jobId"], jobDir=str(directory), + resultFile=str(directory / "job.json"), pid=runner.pid)), flush=True) + except Cancelled: + return 130 + except Exception as error: + if runner is not None: + raise + job.update(status="admission_timeout" if isinstance(error, DeadlineExpired) else "error", + exitCode=75 if isinstance(error, DeadlineExpired) else 125, + error=f"cannot submit runner: {error}", finishedAt=timestamp()) + write_job(directory, job) + return exit_code(job) + finally: + # Popen returning proves handoff, even if a later operation fails. + if runner is not None: + cleanup_step(close_errors, "close submitted ticket fd", ticket.close) + else: + cleanup_step(close_errors, "release unsubmitted ticket", + lambda: close_errors.extend(release_ticket(ticket_path, ticket))) + cleanup_step(close_errors, "close submitted liveness fd", live.close) + if runner is None and close_errors: + job.update(status="cleanup_failed", exitCode=125, cleanupError="; ".join(close_errors)) + cleanup_step(close_errors, "write submission cleanup errors", lambda: write_job(directory, job)) + return 0 + + +def live_report(directory): + with (directory / "live.lock").open("r") as live: + try: + fcntl.flock(live, fcntl.LOCK_SH | fcntl.LOCK_NB) + held = False + except BlockingIOError: + held = True + job = read_job(directory) + job["live"] = held + job["stale"] = not held and job["status"] not in TERMINAL + if job["stale"] and job["childPgid"]: + try: + os.killpg(job["childPgid"], 0) + job["orphanGroupExists"] = True + except ProcessLookupError: + job["orphanGroupExists"] = False + except PermissionError: + job["orphanGroupExists"] = True + job["orphanGroupHasLiveMembers"] = group_has_members(job["childPgid"]) + return job + + +def wait_job(directory, max_wait): + with (directory / "live.lock").open("r") as live: + try: + with bounded(max_wait): + fcntl.flock(live, fcntl.LOCK_SH) + except DeadlineExpired: + print(json.dumps(live_report(directory))) + return 3 + job = read_job(directory) + if job["status"] not in TERMINAL: + job = live_report(directory) + print(json.dumps(job)) + return 4 + print(json.dumps(job)) + return exit_code(job) + + +def cancel_job(directory): + # Keep a shared fd open for inspection, but never signal after lock acquisition. + with (directory / "live.lock").open("r") as live: + try: + fcntl.flock(live, fcntl.LOCK_SH | fcntl.LOCK_NB) + except BlockingIOError: + job = read_job(directory) + if not job["runnerPid"] or not job["runnerStart"]: + print("runner identity is not recorded yet; no signal sent", file=sys.stderr) + return 4 + try: + matches = runner_start(job["runnerPid"]) == job["runnerStart"] + except Exception: + matches = False + if not matches: + print("runner start time does not match; no signal sent", file=sys.stderr) + return 4 + try: + # macOS needs per-job IPC to rule out PID reuse between the start-time check and this signal. + os.kill(job["runnerPid"], signal.SIGTERM) + except ProcessLookupError: + print("runner exited; no signal sent", file=sys.stderr) + return 4 + print(json.dumps(dict(jobId=job["jobId"], cancelRequested=True))) + return 0 + print(json.dumps(live_report(directory))) + return 4 + + +def positive_seconds(text): + value = float(text) + if not math.isfinite(value) or value <= 0: + raise argparse.ArgumentTypeError("seconds must be finite and greater than zero") + return value + + +def job_id(text): + if not re.fullmatch(r"[A-Za-z0-9._-]{1,80}", text): + raise argparse.ArgumentTypeError("job id must match [A-Za-z0-9._-]{1,80}") + return text + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__) + # Defaults suppressed on subparsers allow global options before or after command. + def globals_for(target, suppress=False): + target.add_argument("--state-dir", type=Path, default=argparse.SUPPRESS if suppress else Path.home() / ".codex/qa-resource") + target.add_argument("--lock", type=Path, default=argparse.SUPPRESS if suppress else Path.home() / ".codex/local-ios-qa.lock") + globals_for(parser) + sub = parser.add_subparsers(dest="command", required=True) + for name in ("run", "submit", "wait", "status", "cancel"): + p = sub.add_parser(name) + globals_for(p, True) + p.add_argument("--job-id", type=job_id, required=name != "status") + if name in ("run", "submit"): + p.add_argument("--kind", choices=("package", "render", "simulator"), required=True) + p.add_argument("--device") + p.add_argument("--cwd", type=Path, required=True) + p.add_argument("--admission-deadline", type=positive_seconds, required=True) + p.add_argument("--timeout", type=positive_seconds, required=True) + p.add_argument("argv", nargs=argparse.REMAINDER) + elif name == "wait": + p.add_argument("--max-wait", type=positive_seconds, default=86400) + args = parser.parse_args(argv) + args.state_dir = args.state_dir.expanduser().resolve() + args.lock = args.lock.expanduser().resolve() + if args.command in ("run", "submit"): + args.cwd = args.cwd.expanduser().resolve() + if not args.cwd.is_dir(): + parser.error("--cwd must be an existing directory") + if (args.kind == "simulator") != bool(args.device): + parser.error("--device is required only for simulator jobs and rejected for package/render") + args.argv = args.argv[1:] if args.argv[:1] == ["--"] else args.argv + if not args.argv: + parser.error("argv is required after --") + try: + directory, live, job = create_job(args) + except ValueError as error: + parser.error(str(error)) + if args.command == "submit": + return submit(directory, live, job, args.lock) + return run_job(directory, live, job, args.lock) + directory = args.state_dir / "jobs" / (args.job_id or "") + if args.job_id and not (directory / "job.json").is_file(): + print(f"unknown job {args.job_id}: {directory / 'job.json'}", file=sys.stderr) + return 2 + if args.command == "wait": + return wait_job(directory, args.max_wait) + if args.command == "cancel": + return cancel_job(directory) + if args.job_id: + print(json.dumps(live_report(directory))) + else: + jobs = [live_report(p) for p in sorted(directory.iterdir()) if (p / "job.json").is_file()] if directory.exists() else [] + queue = args.state_dir / "queue" + order = sorted(p.name for p in queue.iterdir() if re.match(r"\d{12}\.", p.name)) if queue.exists() else [] + print(json.dumps(dict(jobs=[{key: j[key] for key in ("jobId", "status", "live", "stale")} for j in jobs], queue=order))) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py new file mode 100644 index 000000000..f30bf658e --- /dev/null +++ b/tools/qa-resource/test_qa_resource.py @@ -0,0 +1,833 @@ +import contextlib +import fcntl +import importlib.util +import io +import json +import os +from pathlib import Path +import select +import signal +import subprocess +import sys +import tempfile +import time +import unittest +from unittest.mock import Mock, patch + +SCRIPT = Path(__file__).with_name("qa_resource.py") +spec = importlib.util.spec_from_file_location("qa_resource", SCRIPT) +qa = importlib.util.module_from_spec(spec) +spec.loader.exec_module(qa) + +# Only test-side code sees this configuration. Production has no test switches. +FAKE = r'''#!PYTHON +import json, os, subprocess, sys, time +from pathlib import Path +config = json.loads(Path(os.environ['QA_TEST_CONFIG']).read_text()) +name = Path(sys.argv[0]).name +args = sys.argv[1:] +if name == 'sysctl': + print(config.get('pressure', 1)) +elif name == 'memory_pressure': + print(config.get('memory', 'System-wide memory free percentage: 80%')) +elif name == 'df': + print('Filesystem 1024-blocks Used Available Capacity Mounted on') + print('fake 20000000 1 10000000 1% /') +elif name == 'ps': + if args == ['-axo', 'pid=,pgid=,stat=,comm=']: + for p in config.get('processes', []): + print(p, p, 'S', '/a path with spaces/blender') + else: + result = subprocess.run(['/bin/ps', *args]) + sys.exit(result.returncode) +elif name == 'xcrun': + with open(config['calls'], 'a') as log: + log.write(json.dumps(args) + '\n') + operation = args[1] + if operation == 'list': + print(json.dumps({'devices': {'fake': config.get('devices', [])}})) + elif operation == 'boot': + if config.get('bootState'): + for device in config['devices']: + if device['udid'] == args[2]: + device['state'] = config['bootState'] + config_path = Path(os.environ['QA_TEST_CONFIG']) + temporary = config_path.with_suffix('.boot.tmp') + temporary.write_text(json.dumps(config)) + temporary.replace(config_path) + time.sleep(config.get('bootSleep', 0)) + sys.exit(config.get('bootCode', 0)) + elif operation == 'bootstatus': + sys.exit(config.get('bootstatusCode', 0)) + elif operation == 'shutdown': + sys.exit(config.get('shutdownCode', 0)) + else: + sys.exit(99) +else: + sys.exit(99) +''' +BOOTSTRAP = ("import sys; sys.path.insert(0, sys.argv[1]); import qa_resource as q; " + "q.CLEANUP_GRACE_SECONDS=.2; q.RESAMPLE_SECONDS=.1; " + "sys.exit(q.main(sys.argv[2:]))") + + +class QueueTests(unittest.TestCase): + def setUp(self): + self.temporary = tempfile.TemporaryDirectory() + self.root = Path(self.temporary.name) + self.state = self.root / "state" + self.lock = self.root / "shared.lock" + self.config_path = self.root / "config.json" + self.calls_path = self.root / "simctl-calls" + self.config = dict(calls=str(self.calls_path), devices=[dict(udid="ours", state="Shutdown"), + dict(udid="other", state="Booted")]) + self.configure() + binary = self.root / "bin" + binary.mkdir() + for name in ("sysctl", "memory_pressure", "df", "ps", "xcrun"): + path = binary / name + path.write_text(FAKE.replace("PYTHON", sys.executable, 1)) + path.chmod(0o755) + self.env = dict(os.environ, PATH=str(binary) + os.pathsep + os.environ.get("PATH", ""), + QA_TEST_CONFIG=str(self.config_path)) + self.processes = [] + self.detached = [] + + def tearDown(self): + for receipt in self.detached: + directory = Path(receipt["jobDir"]) + if qa.live_report(directory)["live"]: + self.invoke(self.cli("cancel", receipt["jobId"])) + result = self.invoke(self.cli("wait", receipt["jobId"], ["--max-wait", "3"])) + if result.returncode == 3: + job = qa.read_job(directory) + if job["runnerPid"] == receipt["pid"] and qa.runner_start(receipt["pid"]) == job["runnerStart"]: + os.kill(receipt["pid"], signal.SIGKILL) + for process in self.processes: + if process.poll() is None: + process.terminate() + try: + process.wait(timeout=3) + except subprocess.TimeoutExpired: + os.killpg(process.pid, signal.SIGKILL) + process.wait(timeout=4) + for stream in (process.stdin, process.stdout, process.stderr): + if stream: + stream.close() + # Every directory here was created by this test. These groups belong to + # its spawned jobs, including the orphan deliberately made by SIGKILL. + jobs = self.state / "jobs" + if jobs.exists(): + for directory in jobs.iterdir(): + if (directory / "job.json").exists(): + pgid = qa.read_job(directory)["childPgid"] + if pgid and qa.group_has_members(pgid): + qa.signal_owned_group(pgid, signal.SIGKILL) + self.temporary.cleanup() + + def configure(self, **changes): + self.config.update(changes) + temporary = self.config_path.with_suffix(".tmp") + temporary.write_text(json.dumps(self.config)) + temporary.replace(self.config_path) + + def cli(self, command, job=None, extra=()): + args = [command, "--state-dir", str(self.state), "--lock", str(self.lock)] + if job: + args += ["--job-id", job] + return args + list(extra) + + def job_args(self, job, code="pass", kind="package", deadline=8, timeout=3, device=None): + args = ["--kind", kind, "--cwd", str(self.root), "--admission-deadline", str(deadline), + "--timeout", str(timeout)] + if device: + args += ["--device", device] + return self.cli("run", job, args + ["--", sys.executable, "-c", code]) + + def launch(self, args, bootstrap=BOOTSTRAP): + process = subprocess.Popen([sys.executable, "-c", bootstrap, str(SCRIPT.parent), *args], + env=self.env, start_new_session=True, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) + self.processes.append(process) + return process + + def invoke(self, args): + result = subprocess.run([sys.executable, str(SCRIPT), *args], env=self.env, + capture_output=True, text=True, timeout=10) + if args[0] == "submit" and result.returncode == 0: + self.detached.append(json.loads(result.stdout)) + return result + + def job(self, name): + return json.loads((self.state / "jobs" / name / "job.json").read_text()) + + def await_job(self, name, predicate, seconds=5): + deadline = time.monotonic() + seconds + while time.monotonic() < deadline: + try: + job = self.job(name) + if predicate(job): + return job + except FileNotFoundError: + pass + time.sleep(.02) + self.fail(f"job {name} did not reach expected state; last={self.job(name)}") + + def finish(self, process, name, expected): + output, error = process.communicate(timeout=8) + self.assertEqual(process.returncode, expected, (output, error, self.job(name))) + return self.job(name) + + def assert_lock_free(self): + with self.lock.open("a+") as lock: + fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) + + def calls(self): + return [json.loads(row) for row in self.calls_path.read_text().splitlines()] if self.calls_path.exists() else [] + + def sleeper(self, seconds=10): + child = subprocess.Popen([sys.executable, "-c", f"import time; time.sleep({seconds})"], + start_new_session=True) + self.processes.append(child) + return child + + def foreign_holder(self): + code = ("import fcntl,select,sys; f=open(sys.argv[1], 'a+'); " + "fcntl.flock(f, fcntl.LOCK_EX|fcntl.LOCK_NB); " + "print('held',flush=True); select.select([sys.stdin],[],[],10)") + process = subprocess.Popen([sys.executable, "-c", code, str(self.lock)], + start_new_session=True, stdin=subprocess.PIPE, + stdout=subprocess.PIPE, text=True) + self.processes.append(process) + # Readiness uses a bounded kernel wait rather than an unbounded readline. + self.assertTrue(select.select([process.stdout], [], [], 3)[0]) + self.assertEqual(process.stdout.readline().strip(), "held") + return process + + def release_holder(self, process): + process.stdin.write("release\n") + process.stdin.flush() + process.stdin.close() + process.wait(timeout=3) + + def test_two_queued_jobs_run_in_submission_order_without_overlap(self): + events = self.root / "events" + def program(name): + return ("import time; " + f"f=open({str(events)!r},'a',buffering=1); " + f"f.write('{name} start '+str(time.monotonic())+'\\n'); time.sleep(.25); " + f"f.write('{name} end '+str(time.monotonic())+'\\n')") + holder = self.foreign_holder() + first_args = self.job_args("first", program("first")) + first_args[0] = "submit" + second_args = self.job_args("second", program("second")) + second_args[0] = "submit" + self.assertEqual(self.invoke(first_args).returncode, 0) + self.assertEqual(self.invoke(second_args).returncode, 0) + self.await_job("second", lambda j: any("queued behind" in r for r in j["reasons"])) + self.release_holder(holder) + self.assertEqual(self.invoke(self.cli("wait", "first", ["--max-wait", "6"])).returncode, 0) + self.assertEqual(self.invoke(self.cli("wait", "second", ["--max-wait", "6"])).returncode, 0) + rows = [row.split() for row in events.read_text().splitlines()] + self.assertEqual([row[:2] for row in rows], [["first", "start"], ["first", "end"], + ["second", "start"], ["second", "end"]]) + self.assertLessEqual(float(rows[1][2]), float(rows[2][2])) + + def test_foreign_lock_holder_waits_then_admits(self): + holder = self.foreign_holder() + runner = self.launch(self.job_args("foreign")) + self.await_job("foreign", lambda j: j["reasons"] == ["shared QA lock held by another holder"]) + self.assertIsNone(holder.poll()) + self.release_holder(holder) + self.assertEqual(self.finish(runner, "foreign", 0)["status"], "succeeded") + + def test_foreign_lock_deadline_leaves_holder_untouched(self): + holder = self.foreign_holder() + runner = self.launch(self.job_args("deadline", deadline=.3)) + result = self.finish(runner, "deadline", 75) + self.assertEqual(result["status"], "admission_timeout") + self.assertEqual(result["reasons"], ["shared QA lock held by another holder"]) + self.assertIsNone(holder.poll()) + with self.lock.open("a+") as lock: + with self.assertRaises(BlockingIOError): + fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) + self.release_holder(holder) + + def test_normal_exit_signals_group_only_before_reaping_leader(self): + events = self.root / "process-events" + bootstrap = BOOTSTRAP.split("sys.exit")[0] + f"\nEVENTS = {str(events)!r}\n" + ''' +OriginalPopen = q.subprocess.Popen +original_killpg = q.os.killpg +reaped = set() +def record(kind, pid): + with open(EVENTS, 'a') as output: + output.write(q.json.dumps([kind, pid]) + '\\n') +class ObservedPopen(OriginalPopen): + def wait(self, *args, **kwargs): + code = super().wait(*args, **kwargs) + reaped.add(self.pid) + record('wait', self.pid) + return code + def poll(self): + code = super().poll() + if code is not None: + reaped.add(self.pid) + record('poll', self.pid) + return code +def observed_killpg(pgid, signum): + if pgid in reaped: + raise AssertionError('group signaled after leader reap') + record('signal', pgid) + return original_killpg(pgid, signum) +q.subprocess.Popen = ObservedPopen +q.os.killpg = observed_killpg +sys.exit(q.main(sys.argv[2:])) +''' + runner = self.launch(self.job_args("pinned"), bootstrap) + result = self.finish(runner, "pinned", 0) + roles = [kind for kind, pid in map(json.loads, events.read_text().splitlines()) + if pid == result["childPgid"]] + self.assertIn("signal", roles) + self.assertNotIn("poll", roles) + self.assertEqual(roles[-1], "wait") + self.assertEqual(roles.count("wait"), 1) + self.assert_lock_free() + + def test_cancel_between_payload_spawn_and_assignment_cleans_payload(self): + spawned = self.root / "spawned-pgid" + bootstrap = BOOTSTRAP.split("sys.exit")[0] + f"\nSPAWNED = {str(spawned)!r}\n" + ''' +original = q.subprocess.Popen +def interrupted_spawn(argv, *args, **kwargs): + child = original(argv, *args, **kwargs) + if 'spawn-token' in ' '.join(argv): + with open(SPAWNED, 'w') as output: + output.write(str(child.pid)) + q.signal.raise_signal(q.signal.SIGTERM) + return child +q.subprocess.Popen = interrupted_spawn +sys.exit(q.main(sys.argv[2:])) +''' + runner = self.launch(self.job_args("spawn-cancel", "import time; token='spawn-token'; time.sleep(10)"), bootstrap) + try: + result = self.finish(runner, "spawn-cancel", 130) + self.assertEqual(result["status"], "cancelled") + self.assertEqual(result["childPgid"], int(spawned.read_text())) + self.assertFalse(qa.group_has_members(result["childPgid"])) + self.assert_lock_free() + finally: + if spawned.exists(): + pgid = int(spawned.read_text()) + if qa.group_has_members(pgid): + qa.signal_owned_group(pgid, signal.SIGKILL) + + def test_submit_cancel_at_spawn_keeps_handoff_owned_by_runner(self): + bootstrap = BOOTSTRAP.split("sys.exit")[0] + ''' +original = q.subprocess.Popen +def interrupted_spawn(argv, *args, **kwargs): + runner = original(argv, *args, **kwargs) + if 'q.resume' in ' '.join(argv): + q.signal.raise_signal(q.signal.SIGTERM) + return runner +q.subprocess.Popen = interrupted_spawn +sys.exit(q.main(sys.argv[2:])) +''' + args = self.job_args("submit-cancel") + args[0] = "submit" + submitter = self.launch(args, bootstrap) + self.finish(submitter, "submit-cancel", 130) + result = self.invoke(self.cli("wait", "submit-cancel", ["--max-wait", "6"])) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(json.loads(result.stdout)["status"], "succeeded") + self.assert_lock_free() + + def test_blocked_submit_receipt_does_not_hold_finished_jobs_liveness(self): + reader, writer = os.pipe() + try: + os.set_blocking(writer, False) + for _ in range(256): + try: + os.write(writer, b"x" * 4096) + except BlockingIOError: + break + else: + self.fail("test pipe did not fill within 1 MiB") + os.set_blocking(writer, True) + args = self.job_args("blocked-receipt") + args[0] = "submit" + submitter = subprocess.Popen([sys.executable, str(SCRIPT), *args], env=self.env, + start_new_session=True, stdout=writer, stderr=subprocess.PIPE, text=True) + self.processes.append(submitter) + os.close(writer) + writer = None + self.await_job("blocked-receipt", lambda j: j["status"] in qa.TERMINAL) + result = self.invoke(self.cli("wait", "blocked-receipt", ["--max-wait", "1"])) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIsNone(submitter.poll()) + with (self.state / "jobs/blocked-receipt/live.lock").open("r") as live: + fcntl.flock(live, fcntl.LOCK_SH | fcntl.LOCK_NB) + submitter.terminate() + submitter.wait(timeout=3) + finally: + if writer is not None: + os.close(writer) + os.close(reader) + + def test_timeout_kills_descendant_ignoring_sigterm_and_releases_lock(self): + program = """import os, signal, time +if os.fork() == 0: + signal.signal(signal.SIGTERM, signal.SIG_IGN) + print(os.getpid(), flush=True) +time.sleep(10) +""" + runner = self.launch(self.job_args("timeout", program, timeout=.4)) + result = self.finish(runner, "timeout", 124) + self.assertEqual(result["status"], "timed_out") + self.assertFalse(qa.group_has_members(result["childPgid"])) + self.assert_lock_free() + + def test_cancel_running_cleans_group_and_releases_lock(self): + runner = self.launch(self.job_args("cancel-run", "import time; time.sleep(10)")) + self.await_job("cancel-run", lambda j: j["status"] == "running") + cancel = self.invoke(self.cli("cancel", "cancel-run")) + self.assertEqual(cancel.returncode, 0, cancel.stderr) + result = self.finish(runner, "cancel-run", 130) + self.assertEqual(result["status"], "cancelled") + self.assertFalse(qa.group_has_members(result["childPgid"])) + self.assert_lock_free() + + def test_cancel_queued_never_starts_child(self): + holder = self.foreign_holder() + first = self.launch(self.job_args("head")) + self.await_job("head", lambda j: bool(j["reasons"])) + runner = self.launch(self.job_args("cancel-queue")) + self.await_job("cancel-queue", lambda j: any("queued behind" in r for r in j["reasons"])) + cancel = self.invoke(self.cli("cancel", "cancel-queue")) + self.assertEqual(cancel.returncode, 0, cancel.stderr) + result = self.finish(runner, "cancel-queue", 130) + self.assertEqual(result["status"], "cancelled") + self.assertIsNone(result["childPgid"]) + self.release_holder(holder) + self.finish(first, "head", 0) + self.assert_lock_free() + + def test_stale_status_wait_cancel_and_later_admission(self): + runner = self.launch(self.job_args("stale", "import time; time.sleep(10)")) + job = self.await_job("stale", lambda j: j["status"] == "running") + runner.kill() + runner.wait(timeout=3) + status = self.invoke(self.cli("status", "stale")) + self.assertTrue(json.loads(status.stdout)["stale"]) + self.assertTrue(json.loads(status.stdout)["orphanGroupExists"]) + self.assertTrue(json.loads(status.stdout)["orphanGroupHasLiveMembers"]) + self.assertEqual(self.invoke(self.cli("wait", "stale", ["--max-wait", "1"])).returncode, 4) + with patch.object(qa.os, "kill") as send: + with contextlib.redirect_stdout(io.StringIO()): + self.assertEqual(qa.cancel_job(self.state / "jobs/stale"), 4) + send.assert_not_called() + self.assertTrue(qa.group_has_members(job["childPgid"])) + next_runner = self.launch(self.job_args("later")) + self.finish(next_runner, "later", 0) + self.assert_lock_free() + + def test_boot_failure_is_error_without_shutdown(self): + self.configure(bootCode=9) + runner = self.launch(self.job_args("boot-fail", kind="simulator", device="ours")) + result = self.finish(runner, "boot-fail", 125) + self.assertEqual(result["status"], "error") + self.assertIn("boot ours exited 9", result["error"]) + self.assertFalse(result["bootedByJob"]) + self.assertFalse(any(row[1] == "shutdown" for row in self.calls())) + self.assert_lock_free() + + def test_boot_timeout_reports_fresh_device_state_without_shutdown(self): + self.configure(bootSleep=2, bootState="Booting") + bootstrap = BOOTSTRAP.replace("sys.exit(q.main", "q.BOOT_TIMEOUT_SECONDS=1; sys.exit(q.main") + runner = self.launch(self.job_args("boot-timeout", kind="simulator", device="ours"), bootstrap) + result = self.finish(runner, "boot-timeout", 125) + self.assertEqual(result["status"], "error") + self.assertIn("boot of ours did not finish in 1 s; device is Booting", result["error"]) + self.assertIn("cannot prove it booted it", result["error"]) + self.assertFalse(result["bootedByJob"]) + self.assertIsNone(result["childPgid"]) + self.assertEqual(sum(row[1] == "list" for row in self.calls()), 2) + self.assertFalse(any(row[1] in ("shutdown", "bootstatus") for row in self.calls())) + self.assert_lock_free() + + def test_cancel_during_boot_persists_ownership_before_shutdown(self): + self.configure(bootSleep=.4, bootState="Booting") + runner = self.launch(self.job_args("boot-cancel", kind="simulator", device="ours")) + self.await_job("boot-cancel", lambda j: any(row[1] == "boot" for row in self.calls())) + os.kill(runner.pid, signal.SIGTERM) + result = self.finish(runner, "boot-cancel", 130) + self.assertEqual(result["status"], "cancelled") + self.assertTrue(result["bootedByJob"]) + self.assertIsNone(result["childPgid"]) + self.assertEqual([row for row in self.calls() if row[1] == "shutdown"], [["simctl", "shutdown", "ours"]]) + self.assertFalse(any(row[1] == "bootstatus" for row in self.calls())) + self.assert_lock_free() + + def test_child_signal_maps_to_128_plus_signal_for_run_and_wait(self): + runner = self.launch(self.job_args("signal-exit", "import os,signal; os.kill(os.getpid(), signal.SIGTERM)")) + result = self.finish(runner, "signal-exit", 128 + signal.SIGTERM) + self.assertEqual(result["status"], "failed") + self.assertEqual(result["exitCode"], -signal.SIGTERM) + self.assertEqual(result["signal"], signal.SIGTERM) + waited = self.invoke(self.cli("wait", "signal-exit", ["--max-wait", "1"])) + self.assertEqual(waited.returncode, 128 + signal.SIGTERM) + self.assertEqual(json.loads(waited.stdout)["exitCode"], -signal.SIGTERM) + self.assert_lock_free() + + def test_hangup_cancels_and_repeated_signals_do_not_interrupt_cleanup(self): + program = ("import signal,time; signal.signal(signal.SIGTERM,signal.SIG_IGN); " + "print('ready',flush=True); time.sleep(10)") + runner = self.launch(self.job_args("hangup", program)) + job = self.await_job("hangup", lambda j: j["status"] == "running" + and "ready" in Path(j["logs"]["output"]).read_text()) + os.kill(runner.pid, signal.SIGHUP) + time.sleep(.03) + os.kill(runner.pid, signal.SIGTERM) + os.kill(runner.pid, signal.SIGINT) + result = self.finish(runner, "hangup", 130) + self.assertEqual(result["status"], "cancelled") + self.assertEqual(result["signal"], signal.SIGHUP) + self.assertNotIn("Traceback", Path(result["logs"]["runner"]).read_text()) + self.assertFalse(qa.group_has_members(job["childPgid"])) + self.assert_lock_free() + + def test_memory_unreadable_is_error(self): + self.configure(memory="unparseable") + runner = self.launch(self.job_args("memory")) + result = self.finish(runner, "memory", 125) + self.assertEqual(result["status"], "error") + self.assertIn("cannot read memory", result["error"]) + self.assert_lock_free() + + def test_package_never_invokes_simctl_with_booted_device(self): + self.configure(devices=[dict(udid="ours", state="Booted")]) + runner = self.launch(self.job_args("package")) + result = self.finish(runner, "package", 0) + self.assertEqual(self.calls(), []) + self.assertNotIn("shared QA lock held by another holder", Path(result["logs"]["runner"]).read_text()) + + def test_simulator_shuts_down_only_device_booted_by_job(self): + runner = self.launch(self.job_args("sim", kind="simulator", device="ours")) + result = self.finish(runner, "sim", 0) + self.assertTrue(result["bootedByJob"]) + self.assertEqual([row for row in self.calls() if row[1] == "shutdown"], [["simctl", "shutdown", "ours"]]) + self.assertEqual(result["shutdownExitCode"], 0) + + def test_busy_device_times_out_without_boot_or_shutdown(self): + self.configure(devices=[dict(udid="ours", state="Booted")]) + runner = self.launch(self.job_args("busy", kind="simulator", device="ours", deadline=2)) + result = self.finish(runner, "busy", 75) + self.assertIn("device ours is Booted", result["reasons"]) + self.assertTrue(all(row[1] == "list" for row in self.calls())) + + def test_missing_device_is_error_without_boot(self): + self.configure(devices=[]) + runner = self.launch(self.job_args("missing", kind="simulator", device="ours")) + result = self.finish(runner, "missing", 125) + self.assertEqual(result["error"], "device ours does not exist; no device is created") + self.assertTrue(all(row[1] == "list" for row in self.calls())) + + def test_heavy_process_exits_naturally_before_admission(self): + sleeper = self.sleeper(1.2) + self.configure(processes=[sleeper.pid]) + runner = self.launch(self.job_args("heavy")) + self.await_job("heavy", lambda j: any("heavy process" in r for r in j["reasons"])) + # The fake lister must omit the exited process, just as real ps does. + sleeper.wait(timeout=3) + self.configure(processes=[]) + result = self.finish(runner, "heavy", 0) + self.assertEqual(sleeper.returncode, 0) + self.assertEqual(result["status"], "succeeded") + + def test_heavy_process_deadline_leaves_process_alive(self): + sleeper = self.sleeper() + self.configure(processes=[sleeper.pid]) + runner = self.launch(self.job_args("heavy-timeout", deadline=2)) + result = self.finish(runner, "heavy-timeout", 75) + self.assertTrue(any("heavy process" in r for r in result["reasons"])) + self.assertIsNone(sleeper.poll()) + + def test_submit_wait_returns_child_exit_code(self): + args = self.job_args("submitted", "import sys,time; time.sleep(.2); sys.exit(7)") + args[0] = "submit" + submit = self.invoke(args) + self.assertEqual(submit.returncode, 0, submit.stderr) + receipt = json.loads(submit.stdout) + self.assertEqual(receipt["jobId"], "submitted") + result = self.invoke(self.cli("wait", "submitted", ["--max-wait", "6"])) + self.assertEqual(result.returncode, 7, result.stderr) + self.assertEqual(json.loads(result.stdout)["status"], "failed") + self.assert_lock_free() + + def test_submit_wait_max_wait_does_not_affect_job(self): + args = self.job_args("short-wait", "import time; time.sleep(.5)") + args[0] = "submit" + self.assertEqual(self.invoke(args).returncode, 0) + result = self.invoke(self.cli("wait", "short-wait", ["--max-wait", ".05"])) + self.assertEqual(result.returncode, 3, result.stderr) + self.assertTrue(json.loads(result.stdout)["live"]) + final = self.invoke(self.cli("wait", "short-wait", ["--max-wait", "6"])) + self.assertEqual(final.returncode, 0, final.stderr) + self.assertEqual(json.loads(final.stdout)["status"], "succeeded") + + def test_shutdown_failure_records_cleanup_failed_and_releases_lock(self): + self.configure(shutdownCode=8) + runner = self.launch(self.job_args("shutdown-fail", kind="simulator", device="ours")) + result = self.finish(runner, "shutdown-fail", 125) + self.assertEqual(result["status"], "cleanup_failed") + self.assertEqual(result["shutdownExitCode"], 8) + self.assertIn("shutdown ours exited 8", result["cleanupError"]) + self.assert_lock_free() + + def test_bootstatus_failure_still_shuts_down_owned_device(self): + self.configure(bootstatusCode=6) + runner = self.launch(self.job_args("bootstatus-fail", kind="simulator", device="ours")) + result = self.finish(runner, "bootstatus-fail", 125) + self.assertEqual(result["status"], "error") + self.assertEqual([row for row in self.calls() if row[1] == "shutdown"], [["simctl", "shutdown", "ours"]]) + + def test_unknown_job_duplicate_id_and_cli_validation(self): + unknown = self.invoke(self.cli("wait", "unknown")) + self.assertEqual(unknown.returncode, 2) + self.assertIn("unknown job unknown", unknown.stderr) + runner = self.launch(self.job_args("unique")) + self.finish(runner, "unique", 0) + old = (self.state / "jobs/unique/job.json").read_bytes() + self.assertEqual(self.invoke(self.job_args("unique")).returncode, 2) + self.assertEqual((self.state / "jobs/unique/job.json").read_bytes(), old) + for args in (self.job_args("bad", device="ours"), + self.job_args("bad", kind="simulator"), + self.job_args("bad", deadline=float("nan"))): + self.assertEqual(self.invoke(args).returncode, 2) + + +class PureTests(unittest.TestCase): + def test_agent_device_classification_both_only_testing_forms(self): + for flag in (f"-only-testing {qa.SERVICE_TEST}", f"-only-testing:{qa.SERVICE_TEST}"): + process = dict(comm="/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild", + args=("/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild " + f"test-without-building {flag} -destination " + "platform=iOS Simulator,id=A4EE0B20-1654-4B67-B54E-D071DA1E9601")) + for kind in ("package", "render"): + self.assertEqual(qa.classify_process(process, kind), "ignored") + self.assertEqual(qa.classify_process(process, "simulator", "A4EE0B20-1654-4B67-B54E-D071DA1E9601"), "blocker") + self.assertEqual(qa.classify_process(process, "simulator", "other"), "ignored") + + def test_xcodebuild_without_all_markers_blocks(self): + for args in ("xcodebuild", f"xcodebuild -only-testing {qa.SERVICE_TEST}", + "xcodebuild test-without-building -only-testing WrongTest", + f"xcodebuild test-without-building echo {qa.SERVICE_TEST}"): + self.assertEqual(qa.classify_process(dict(comm="xcodebuild", args=args), "package"), "blocker") + self.assertEqual(qa.classify_process(dict(comm="swift-build", args="", stat="Z"), "package"), "irrelevant") + self.assertEqual(qa.classify_process(dict(comm="/Applications/T3 Code App", args="blender"), "package"), "irrelevant") + + def test_memory_and_disk_threshold_boundaries(self): + for pressure in (1, 2): + self.assertEqual(qa.resource_blockers(pressure, 35, 5 * 1024**3), []) + self.assertTrue(qa.resource_blockers(4, 80, 6 * 1024**3)) + self.assertTrue(qa.resource_blockers(1, 34, 6 * 1024**3)) + self.assertTrue(qa.resource_blockers(1, 35, 5 * 1024**3 - 1)) + + def test_eperm_zombie_and_uncertain_owned_group(self): + with patch.object(qa.os, "killpg", side_effect=PermissionError), \ + patch.object(qa, "group_has_members", return_value=False): + self.assertFalse(qa.signal_owned_group(123, signal.SIGTERM)) + with patch.object(qa.os, "killpg", side_effect=PermissionError), \ + patch.object(qa, "group_has_members", return_value=True): + with self.assertRaisesRegex(RuntimeError, "live members remain"): + qa.signal_owned_group(123, signal.SIGTERM) + + def test_cancel_pid_start_mismatch_never_signals(self): + with tempfile.TemporaryDirectory() as temp: + directory = Path(temp) + (directory / "live.lock").touch() + qa.write_job(directory, dict(runnerPid=123, runnerStart="old")) + with (directory / "live.lock").open("a+") as live: + fcntl.flock(live, fcntl.LOCK_EX) + with patch.object(qa, "runner_start", return_value="new"), \ + patch.object(qa.os, "kill") as send, contextlib.redirect_stderr(io.StringIO()): + self.assertEqual(qa.cancel_job(directory), 4) + send.assert_not_called() + + def test_release_ticket_is_idempotent_and_records_unlink_failure(self): + with tempfile.TemporaryDirectory() as temp: + path = Path(temp) / "ticket" + ticket = path.open("a+") + fcntl.flock(ticket, fcntl.LOCK_EX | fcntl.LOCK_NB) + with patch.object(Path, "unlink", side_effect=OSError("unlink refused")): + errors = qa.release_ticket(path, ticket) + self.assertIn("unlink refused", "; ".join(errors)) + self.assertTrue(ticket.closed) + self.assertIn("unlink refused", "; ".join(qa.release_ticket(path, ticket))) + self.assertEqual(qa.release_ticket(path, ticket), []) + self.assertEqual(qa.release_ticket(path, None), []) + self.assertFalse(path.exists()) + + def test_cleanup_hold_keeps_lock_until_group_gone_or_deadline(self): + for members_remain in (False, True): + with self.subTest(members_remain=members_remain), tempfile.TemporaryDirectory() as temp: + root = Path(temp) + args = Mock(state_dir=root / "state", job_id="hold", kind="package", argv=["unused"], + cwd=root, device=None, admission_deadline=3, timeout=1) + directory, live, job = qa.create_job(args) + clock = [0] + lock_path = root / "lock" + def sleep(seconds): + self.assertEqual(qa.read_job(directory)["status"], "cleanup_failed") + with lock_path.open("a+") as probe: + with self.assertRaises(BlockingIOError): + fcntl.flock(probe, fcntl.LOCK_EX | fcntl.LOCK_NB) + clock[0] += seconds + def members(pgid): + return members_remain or clock[0] < 1 + with patch.object(qa.time, "monotonic", side_effect=lambda: clock[0]), \ + patch.object(qa.time, "sleep", side_effect=sleep), \ + patch.object(qa, "CLEANUP_HOLD_SECONDS", 2), \ + patch.object(qa, "runner_start", return_value="test identity"), \ + patch.object(qa, "sense_memory", return_value=(1, 80)), \ + patch.object(qa, "sense_disk", return_value=10 * 1024**3), \ + patch.object(qa, "list_processes", return_value=[]), \ + patch.object(qa, "wait_child_exit", return_value=True), \ + patch.object(qa, "group_has_members", side_effect=members), \ + patch.object(qa.subprocess, "Popen", return_value=Mock(pid=123, wait=Mock(return_value=0))) as spawn, \ + patch.object(qa, "stop_owned_command", side_effect=RuntimeError("members survive SIGKILL")): + self.assertEqual(qa.run_job(directory, live, job, lock_path, echo=False), 125) + result = qa.read_job(directory) + self.assertEqual(result["status"], "cleanup_failed") + expected = "live members remained" if members_remain else "no live members remained" + self.assertIn(expected, result["cleanupError"]) + self.assertEqual(clock[0], 2 if members_remain else 1) + self.assertNotIn("pass_fds", spawn.call_args.kwargs) + with lock_path.open("a+") as probe: + fcntl.flock(probe, fcntl.LOCK_EX | fcntl.LOCK_NB) + + def test_finalizer_failures_do_not_skip_other_release_steps(self): + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + args = Mock(state_dir=root / "state", job_id="finalizer", kind="package", argv=["unused"], + cwd=root, device=None, admission_deadline=3, timeout=1) + directory, live, job = qa.create_job(args) + lock_path = root / "lock" + events = [] + original_open = Path.open + original_flock = qa.fcntl.flock + original_signal = qa.signal.signal + original_release = qa.release_ticket + previous_signals = {sig: qa.signal.getsignal(sig) for sig in qa.RUNNER_SIGNALS} + class CloseSpy: + def __init__(self, stream, label, fail=False): + self.stream, self.label, self.fail = stream, label, fail + def __getattr__(self, name): + return getattr(self.stream, name) + def close(self): + events.append(self.label) + self.stream.close() + if self.fail: + raise OSError(self.label + " failed") + def opened(path, *args, **kwargs): + stream = original_open(path, *args, **kwargs) + if path == lock_path: + return CloseSpy(stream, "close global") + if path == directory / "runner.log": + return CloseSpy(stream, "close log", fail=True) + return stream + def flock(fd, operation): + if getattr(fd, "name", None) == str(lock_path) and operation == fcntl.LOCK_UN: + events.append("unlock global") + raise OSError("unlock global failed") + return original_flock(fd, operation) + def restore(sig, handler): + value = original_signal(sig, handler) + if sig in previous_signals and handler == previous_signals[sig]: + events.append(f"restore {sig}") + if sig == signal.SIGTERM: + raise OSError("restore SIGTERM failed") + return value + releases = [0] + def release(path, ticket): + releases[0] += 1 + if releases[0] == 2: + events.append("release ticket") + raise OSError("release ticket failed") + return original_release(path, ticket) + with patch.object(Path, "open", new=opened), \ + patch.object(qa.fcntl, "flock", side_effect=flock), \ + patch.object(qa.signal, "signal", side_effect=restore), \ + patch.object(qa, "release_ticket", side_effect=release), \ + patch.object(qa, "runner_start", return_value="test identity"), \ + patch.object(qa, "sense_memory", return_value=(1, 80)), \ + patch.object(qa, "sense_disk", return_value=10 * 1024**3), \ + patch.object(qa, "list_processes", return_value=[]), \ + patch.object(qa, "wait_child_exit", return_value=True), \ + patch.object(qa.subprocess, "Popen", return_value=Mock(pid=123)), \ + patch.object(qa, "stop_owned_command", return_value=0): + self.assertEqual(qa.run_job(directory, CloseSpy(live, "close live", fail=True), job, + lock_path, echo=False), 125) + result = qa.read_job(directory) + self.assertEqual(result["status"], "cleanup_failed") + for label in ("unlock global", "release ticket", "close log", "close live", "restore SIGTERM"): + self.assertIn(label + " failed", result["cleanupError"]) + for label in ("close global", f"restore {signal.SIGINT}", f"restore {signal.SIGHUP}"): + self.assertIn(label, events) + self.assertTrue(live.closed) + with lock_path.open("a+") as probe: + fcntl.flock(probe, fcntl.LOCK_EX | fcntl.LOCK_NB) + + def test_simulator_boot_has_a_separate_clock_from_admission(self): + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + args = Mock(state_dir=root / "state", job_id="boot-clock", kind="simulator", argv=["unused"], + cwd=root, device="ours", admission_deadline=1, timeout=1) + directory, live, job = qa.create_job(args) + clock = [0] + def fake_simctl(operation, device, **kwargs): + if operation in ("boot", "bootstatus"): + self.assertEqual(kwargs["timeout"], qa.BOOT_TIMEOUT_SECONDS) + clock[0] += 2 + return Mock(returncode=0) + with patch.object(qa.time, "monotonic", side_effect=lambda: clock[0]), \ + patch.object(qa, "runner_start", return_value="test identity"), \ + patch.object(qa, "sense_memory", return_value=(1, 80)), \ + patch.object(qa, "sense_disk", return_value=10 * 1024**3), \ + patch.object(qa, "list_processes", return_value=[]), \ + patch.object(qa, "sense_devices", return_value=[dict(udid="ours", state="Shutdown")]), \ + patch.object(qa, "simctl", side_effect=fake_simctl) as simctl, \ + patch.object(qa, "wait_child_exit", return_value=True), \ + patch.object(qa.subprocess, "Popen", return_value=Mock(pid=123, wait=Mock(return_value=0))) as spawn, \ + patch.object(qa, "stop_owned_command", return_value=0): + self.assertEqual(qa.run_job(directory, live, job, root / "lock", echo=False, deadline=1), 0) + self.assertTrue(spawn.called) + self.assertEqual([call.args[0] for call in simctl.call_args_list], ["boot", "bootstatus", "shutdown"]) + self.assertEqual(qa.read_job(directory)["status"], "succeeded") + + def test_cleanup_error_still_shutdowns_owned_device_and_releases_lock(self): + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + args = Mock(state_dir=root / "state", job_id="cleanup", kind="simulator", argv=["unused"], + cwd=root, device="ours", admission_deadline=3, timeout=1) + directory, live, job = qa.create_job(args) + lock_path = root / "lock" + with patch.object(qa, "runner_start", return_value="test identity"), \ + patch.object(qa, "sense_memory", return_value=(1, 80)), \ + patch.object(qa, "sense_disk", return_value=10 * 1024**3), \ + patch.object(qa, "list_processes", return_value=[]), \ + patch.object(qa, "sense_devices", return_value=[dict(udid="ours", state="Shutdown")]), \ + patch.object(qa, "simctl", return_value=Mock(returncode=0)) as simctl, \ + patch.object(qa, "wait_child_exit", return_value=True), \ + patch.object(qa, "group_has_members", return_value=False), \ + patch.object(qa.subprocess, "Popen", return_value=Mock(pid=123, wait=Mock(return_value=0))), \ + patch.object(qa, "stop_owned_command", side_effect=RuntimeError("cannot confirm owned group gone")): + self.assertEqual(qa.run_job(directory, live, job, lock_path, echo=False), 125) + result = qa.read_job(directory) + self.assertEqual(result["status"], "cleanup_failed") + self.assertIn("cannot confirm", result["cleanupError"]) + self.assertEqual(simctl.call_args.args, ("shutdown", "ours")) + with lock_path.open("a+") as lock: + fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) + + +if __name__ == "__main__": + unittest.main() From b65fe9e34e36af219eee0f3a0868f9731d7822da Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Thu, 1 Oct 2026 06:40:35 -0500 Subject: [PATCH 02/15] tools: stop a job's descendants that leave its process group Cancelling a `swift test` job killed the job's process group but left swiftpm-testing-helper running. SwiftPM starts that helper in its own process group, and once swift-test died launchd adopted it, while the runner had already released the shared lock. Timeout and a normal exit had the same gap. Each job's child now carries a per-job owner token in its environment. At cleanup the runner finds escaped processes by that token, read through KERN_PROCARGS2, or by live parent ancestry. It pins each one to its PID, kernel start time in microseconds and uid, and rechecks that identity before signaling it by PID, never by another group. Success needs two consecutive complete empty scans. Anything it cannot confirm gone makes the job cleanup_failed and keeps the lock held. Admission waits on any live token-bearing process, and nested jobs are refused. Co-Authored-By: Claude Opus 5.5 (1M context) --- tools/qa-resource/README.md | 22 +- tools/qa-resource/qa_resource.py | 493 +++++++++++++++++++++-- tools/qa-resource/test_qa_resource.py | 553 +++++++++++++++++++++++++- 3 files changed, 1023 insertions(+), 45 deletions(-) diff --git a/tools/qa-resource/README.md b/tools/qa-resource/README.md index 6185920db..fab32e2d9 100644 --- a/tools/qa-resource/README.md +++ b/tools/qa-resource/README.md @@ -26,9 +26,27 @@ Admission requires memory pressure level 1 or 2, at least 35% memory free, and a Heavy-process exit waits use kqueue. Memory recovery, disk space, and device state have no stdlib kernel notification here, so they are resampled every 30 seconds. The global lock stays held during resource waits. The admission deadline covers queueing and resource checks. Simulator boot and bootstatus each have a fixed 300-second limit, an operational limit rather than a measured startup time. Cancellation waits for the boot result and ownership record. If boot times out, the runner reports the device state but never shuts it down without proof of ownership. The child timeout starts separately. +## Escaped descendants + +Each payload gets a random `QA_RESOURCE_OWNER_=` environment entry. At admission and cleanup, the runner reads process memory with Darwin `KERN_PROCARGS2`. An exact entry or live-parent ancestry proves ownership at discovery. The parser checks every NUL-separated string after the executable because empty `argv[0]` is indistinguishable from padding. Admission accepts only owner headers with a 32-character lowercase hexadecimal token. + +On cancellation, timeout, and normal exit, the runner signals discovered escapes before its pinned child group. Discovery proof stays attached to PID, microsecond start time, and uid even after reparenting or token removal. Each per-PID signal rechecks that identity with `KERN_PROC_PID` and excludes zombies and other uids. The runner never signals an escaped process's new group. The leader stays unreaped until checks finish. + +`escapedProcesses` in `job.json` and `runner.log` records `pid`, `start_us`, `uid`, human-readable `start`, executable at discovery, proof, and outcome. `terminated` and `killed` mean the runner sent that signal and then confirmed the identity was no longer live. `exited` means no signal was sent; `retained` means cleanup could not confirm exit. + +Confirmation requires two consecutive complete empty passes at least 0.1 seconds apart, after the last observed owned process. Incomplete reads or expired inspection budgets never count as empty. Discovery errors do not skip signaling the pinned group, but the job reports `cleanup_failed` and enters the cleanup hold. + +Admission waits for live same-user owner entries, including orphans from dead runners or expired holds. It never signals these blockers. The runner and its ancestors are excluded as a defense. The payload does not inherit the global lock descriptor. `run` and `submit` reject an inherited owner entry before creating a job, returning 2 with `nested qa-resource jobs are not supported: this process belongs to job X`. + ## What the tool cannot guarantee -The queue cannot identify a foreign shared-lock holder or attribute a heavy process to a project. FIFO ordering applies only to this tool's tickets; `qa-slot.py` and `package-run.py` still race for the shared flock. Cancellation signals only a verified live runner, which stops its own child group and shuts down only the device it booted. The runner handles SIGTERM, SIGINT, and SIGHUP; later signals cannot interrupt cleanup. A stale job's recorded group and booted-device ownership are reported, never cleaned. Inspect those manually before reusing a device. +The token is a cooperative marker in mutable process memory, not an authenticated exec record. A process can overwrite its environment strings, exec without the token, or copy the token. Copying it onto an unrelated same-uid process makes cleanup treat that process as owned and signal it. + +Invisible descendants are not stopped. Darwin omits the environment of `cs_restricted` processes without an error, other-uid environments are unreadable, and already-reparented tokenless helpers have no ancestry after natural exit. An unreadable environment without ancestry is not ownership proof. A descendant under another uid cannot be signaled; if ancestry finds it, cleanup fails. `succeeded` means no observable owned process remained, not that every descendant was stopped. + +A chain whose processes each live shorter than one scan interval can evade two-pass confirmation. Microsecond start times avoid coarse `lstart` identity checks for escaped PIDs, but the check-to-signal PID reuse window remains. + +The queue cannot identify a foreign shared-lock holder or attribute a heavy process to a project. FIFO ordering applies only to this tool's tickets; `qa-slot.py` and `package-run.py` still race for the shared flock. Cancellation signals only a verified live runner, which stops its own child group and proven escaped descendants, and shuts down only the device it booted. The runner handles SIGTERM, SIGINT, and SIGHUP; later signals cannot interrupt cleanup. A stale job's recorded group and booted-device ownership are reported, never cleaned. Inspect those manually before reusing a device. Cancel can signal a reused PID if reuse happens between the runner start-time check and SIGTERM; eliminating that window on macOS needs per-job IPC. @@ -36,4 +54,4 @@ Boot ownership does not identify a simulator session; if another lane shuts down A stuck helper is killed at its subprocess timeout, but `subprocess.run` then reaps it without a deadline, so that failure can exceed the stated limits. -If group cleanup fails, the runner holds the shared lock for up to 300 more seconds, an operational limit rather than a measured recovery time. It releases the lock when no live members remain or the hold expires, records the outcome in `cleanupError`, and keeps status `cleanup_failed`. +If group or escaped-process cleanup fails, the runner holds the shared lock for up to 300 more seconds, an operational limit rather than a measured recovery time. It releases the lock after two complete empty passes or when the hold expires, records retained PID identities and the hold outcome in `cleanupError`, and keeps status `cleanup_failed`. Same-user token-bearing survivors still block later admission. diff --git a/tools/qa-resource/qa_resource.py b/tools/qa-resource/qa_resource.py index 40c5234d3..3b94f481a 100644 --- a/tools/qa-resource/qa_resource.py +++ b/tools/qa-resource/qa_resource.py @@ -2,15 +2,18 @@ """Serialize heavyweight local QA work without taking ownership of other jobs.""" import argparse from contextlib import closing, contextmanager +import ctypes from datetime import datetime, timezone import errno import fcntl +from functools import lru_cache import json import math import os from pathlib import Path import re import select +import secrets import shlex import signal import subprocess @@ -30,6 +33,12 @@ # Operational boot limit, not a measured simulator startup time. BOOT_TIMEOUT_SECONDS = 300 RUNNER_SIGNALS = (signal.SIGTERM, signal.SIGINT, signal.SIGHUP) +OWNER_ENV_PREFIX = "QA_RESOURCE_OWNER_" +_LIBC = ctypes.CDLL(None, use_errno=True) +_SYSCTL = _LIBC.sysctl +_SYSCTL.argtypes = [ctypes.POINTER(ctypes.c_int), ctypes.c_uint, ctypes.c_void_p, + ctypes.POINTER(ctypes.c_size_t), ctypes.c_void_p, ctypes.c_size_t] +_SYSCTL.restype = ctypes.c_int HEAVY_NAMES = {"xcodebuild", "swift-build", "swift-test", "swift-frontend", "swift-driver", "blender"} SERVICE_TEST = "AgentDeviceRunnerUITests/RunnerTests/testCommand" @@ -46,6 +55,25 @@ class DeadlineExpired(Exception): pass +class SnapshotIncomplete(RuntimeError): + def __init__(self, message, processes=()): + super().__init__(message) + self.processes = list(processes) + + +class SnapshotDeadline(SnapshotIncomplete): + pass + + +def inspection_timeout(deadline): + if deadline is None: + return SENSING_TIMEOUT + remaining = deadline - time.monotonic() + if remaining <= 0: + raise SnapshotDeadline("inspection deadline expired; completeness uncertain") + return min(SENSING_TIMEOUT, remaining) + + def timestamp(): return datetime.now(timezone.utc).isoformat() @@ -69,9 +97,9 @@ def alarm(signum, frame): signal.setitimer(signal.ITIMER_REAL, *previous) -def command_output(argv): +def command_output(argv, timeout=SENSING_TIMEOUT): # subprocess.run kills a timed-out helper, but its subsequent reap is unbounded. - result = subprocess.run(argv, capture_output=True, text=True, timeout=SENSING_TIMEOUT) + result = subprocess.run(argv, capture_output=True, text=True, timeout=timeout) if result.returncode: raise RuntimeError(f"{' '.join(argv)} exited {result.returncode}: {result.stderr.strip()}") return result.stdout.strip() @@ -104,6 +132,223 @@ def sense_disk(): return int(match[1]) * 1024 +def _sysctl_read(mib, buffer): + size = ctypes.c_size_t(ctypes.sizeof(buffer)) + name = (ctypes.c_int * len(mib))(*mib) + if _SYSCTL(name, len(mib), ctypes.byref(buffer), ctypes.byref(size), None, 0): + code = ctypes.get_errno() + raise OSError(code, f"sysctl {mib}: {os.strerror(code)}") + return size.value + + +@lru_cache(maxsize=1) +def _argmax(): + value = ctypes.c_int() + _sysctl_read([1, 8], value) # CTL_KERN, KERN_ARGMAX, from Darwin sys/sysctl.h. + if value.value <= 0: + raise OSError(errno.EINVAL, "KERN_ARGMAX returned a nonpositive buffer size") + return value.value + + +def parse_process_env(raw, pid): + """Return NUL-separated candidates after the executable, including argv strings.""" + end = raw.find(b"\0", ctypes.sizeof(ctypes.c_int)) + if len(raw) < ctypes.sizeof(ctypes.c_int) or end < 0: + raise OSError(errno.EINVAL, f"malformed KERN_PROCARGS2 data for process {pid}") + # Empty argv[0] is indistinguishable from padding. Never use argc to skip it. + # These are cooperative markers in mutable process memory, not exec records. + return [os.fsdecode(entry) for entry in raw[end + 1:].split(b"\0") if entry] + + +def process_env(pid): + # Darwin can omit cs_restricted environments without reporting an error. + buffer = ctypes.create_string_buffer(_argmax()) + size = _sysctl_read([1, 49, pid], buffer) # CTL_KERN, KERN_PROCARGS2. + return parse_process_env(ctypes.string_at(buffer, size), pid) + + +class ProcessTimeval(ctypes.Structure): + _fields_ = [("seconds", ctypes.c_long), ("microseconds", ctypes.c_int)] + + +def kernel_identity(pid): + # 64-bit Darwin SDK sizeof/offsetof: kinfo_proc=648, start=0, stat=36, + # pid=40, kp_eproc.e_ucred.cr_uid=420. Fail closed if the runtime ABI differs. + if ctypes.sizeof(ctypes.c_void_p) != 8 or ctypes.sizeof(ProcessTimeval) != 16: + raise RuntimeError("process identity requires the 64-bit Darwin kinfo_proc ABI") + buffer = ctypes.create_string_buffer(648) + try: + size = _sysctl_read([1, 14, 1, pid], buffer) # CTL_KERN, KERN_PROC, KERN_PROC_PID. + except OSError as error: + if error.errno == errno.ESRCH: + return None + raise + if size == 0: + return None + if size != ctypes.sizeof(buffer): + raise RuntimeError(f"kinfo_proc size mismatch for PID {pid}: expected 648, got {size}") + actual_pid = ctypes.c_int.from_buffer(buffer, 40).value + if actual_pid != pid: + raise RuntimeError(f"kinfo_proc PID mismatch: requested {pid}, got {actual_pid}") + start = ProcessTimeval.from_buffer(buffer) + if start.seconds <= 0 or not 0 <= start.microseconds < 1_000_000: + raise RuntimeError(f"invalid kinfo_proc start time for PID {pid}") + return dict(pid=pid, start_us=start.seconds * 1_000_000 + start.microseconds, + uid=ctypes.c_uint.from_buffer(buffer, 420).value, + zombie=ctypes.c_byte.from_buffer(buffer, 36).value == 5) + + +def process_snapshot(deadline=None): + rows = command_output(["ps", "-axo", "pid=,ppid=,pgid=,uid=,stat="], + timeout=inspection_timeout(deadline)) + result = {} + for row in rows.splitlines(): + pid, ppid, pgid, uid, state = row.split() + result[int(pid)] = dict(pid=int(pid), ppid=int(ppid), pgid=int(pgid), + uid=int(uid), stat=state) + return result + + +def process_identity(pid, details=False, deadline=None): + columns = "pid=,ppid=,pgid=,uid=,stat=,lstart=,comm=" if details else "lstart=,comm=" + result = subprocess.run(["ps", "-o", columns, "-p", str(pid)], capture_output=True, + text=True, timeout=inspection_timeout(deadline)) + if result.returncode == 1 and not result.stdout.strip(): + return None + if result.returncode: + raise RuntimeError(f"cannot inspect identity for process {pid}: {result.stderr.strip()}") + text = result.stdout.strip() + identity = dict(pid=pid) + if details: + fields = text.split(maxsplit=5) + if len(fields) != 6: + raise RuntimeError(f"malformed process details for {pid}: {text!r}") + identity.update(pid=int(fields[0]), ppid=int(fields[1]), pgid=int(fields[2]), + uid=int(fields[3]), stat=fields[4]) + text = fields[5] + match = re.fullmatch(r"(\S+\s+\S+\s+\d+\s+\d{2}:\d{2}:\d{2}\s+\d{4})\s+(.+)", text) + if match is None: + raise RuntimeError(f"malformed lstart/comm for process {pid}: {text!r}") + identity.update(start=match[1], comm=match[2]) + return identity + + +def process_ancestors(pid, rows): + ancestors = set() + while pid in rows: + pid = rows[pid]["ppid"] + if not pid or pid in ancestors: + break + ancestors.add(pid) + return ancestors + + +def owner_entry(job): + token = job.get("ownerToken") + return f"{OWNER_ENV_PREFIX}{token}={job['jobId']}" if token else None + + +def owned_processes(job, child_pid, deadline=None, known=None, observed=None): + deadline = time.monotonic() + SENSING_TIMEOUT if deadline is None else deadline + owned = [] + try: + rows = process_snapshot(deadline) + root = rows.get(child_pid) + # After natural exit, already-reparented tokenless helpers have no ancestry. + root_alive = root is not None and not root["stat"].startswith("Z") + entry = owner_entry(job) + for pid, row in rows.items(): + inspection_timeout(deadline) + if pid == os.getpid() or row["stat"].startswith("Z"): + continue + ancestry = root_alive and (pid == child_pid or child_pid in process_ancestors(pid, rows)) + if row["uid"] != os.getuid() and not ancestry: + continue + before = kernel_identity(pid) + inspection_timeout(deadline) + if before is None or before["zombie"]: + continue + key = identity_key(before) + if known is not None and key in known: + owned.append(dict(known[key], pgid=row["pgid"], ppid=row["ppid"], stat=row["stat"])) + continue # Historical proof is attached to this exact execution. + token = False + if before["uid"] == os.getuid() and entry is not None: + try: + token = entry in process_env(pid) + except OSError as error: + if error.errno == errno.ESRCH: + continue # Enumeration-to-env disappearance is settled-gone. + # Restricted or unreadable environments alone are not proof. + inspection_timeout(deadline) + if not token and not ancestry: + continue + if observed is not None: + observed.append(pid) + identity = process_identity(pid, deadline=deadline) + after = kernel_identity(pid) + inspection_timeout(deadline) + if after is None or after["zombie"] or after["start_us"] != before["start_us"]: + continue # This identity is settled-gone, not a live empty read. + if identity is None or after["uid"] != before["uid"]: + raise SnapshotIncomplete(f"unsettled owned identity for PID {pid}", owned) + owned.append(dict(row, **{k: identity[k] for k in ("start", "comm")}, + start_us=after["start_us"], uid=after["uid"], + proof="token" if token else "ancestry")) + inspection_timeout(deadline) + except SnapshotIncomplete as error: + error.processes = owned + raise + except subprocess.TimeoutExpired as error: + failure = SnapshotDeadline if time.monotonic() >= deadline else SnapshotIncomplete + raise failure(f"process inspection timed out: {error}; completeness uncertain", owned) from error + except Exception as error: + raise SnapshotIncomplete(f"process discovery failed: {error}", owned) from error + return owned + + +def owner_jobs(entries): + pattern = re.compile(r"^" + re.escape(OWNER_ENV_PREFIX) + r"[0-9a-f]{32}=") + return [entry[match.end():] for entry in entries if (match := pattern.match(entry))] + + +def owner_blockers(): + rows = process_snapshot() + excluded = process_ancestors(os.getpid(), rows) | {os.getpid()} + pids, reasons = [], [] + for pid, row in rows.items(): + if pid in excluded or row["uid"] != os.getuid() or row["stat"].startswith("Z"): + continue + try: + owners = owner_jobs(process_env(pid)) + except OSError: + continue + if not owners: + continue + identity = process_identity(pid) + if identity is None: + continue + pids.append(pid) + reasons.extend(f"process {pid} ({identity['comm']}) still carries owner token of job {owner}" + for owner in owners) + return pids, reasons + + +def signal_owned_process(identity, signum): + """Ownership proof survives reparenting; recheck the exact proven identity.""" + current = kernel_identity(identity["pid"]) + if (current is None or current["start_us"] != identity["start_us"] + or current["uid"] != identity["uid"] or current["uid"] != os.getuid() + or current["zombie"]): + return False + try: + # Microsecond identity prevents coarse lstart reuse, not this final race. + os.kill(current["pid"], signum) + return True + except (ProcessLookupError, PermissionError): + return False + + def list_processes(): rows = command_output(["ps", "-axo", "pid=,pgid=,stat=,comm="]) processes = [] @@ -203,8 +448,9 @@ def wait_process_exit(pids, seconds): queue.control(None, len(pids), seconds) -def group_has_members(pgid): - rows = command_output(["ps", "-axo", "pgid=,stat="]) +def group_has_members(pgid, deadline=None): + rows = command_output(["ps", "-axo", "pgid=,stat="], + timeout=inspection_timeout(deadline)) return any(int(group) == pgid and not state.startswith("Z") for group, state in (row.split() for row in rows.splitlines())) @@ -237,39 +483,194 @@ def wait_child_exit(child, seconds): return True -def stop_owned_command(child): +def identity_key(identity): + return identity["pid"], identity["start_us"], identity["uid"] + + +class OwnedProcesses: + """Keep discovery proof attached to an exact identity until that execution ends.""" + def __init__(self, job, child_pid, report=None): + self.job, self.child_pid, self.report = job, child_pid, report + self.identities, self.sent, self.escaped = {}, {}, {} + self.errors = [] + self.last_incomplete = None + self.saw_owned = False + + def error(self, message): + if message not in self.errors: + self.errors.append(message) + + def record(self, identity, outcome): + key = identity_key(identity) + if self.escaped.get(key) != outcome: + self.escaped[key] = outcome + if self.report is not None: + self.report(identity, outcome) + + def snapshot(self, deadline): + complete = True + self.last_incomplete = None + observed = [] + try: + fresh = owned_processes(self.job, self.child_pid, deadline, + known=self.identities, observed=observed) + except SnapshotIncomplete as error: + fresh, complete = error.processes, False + self.last_incomplete = str(error) + if not isinstance(error, SnapshotDeadline): + self.error(str(error)) + except Exception as error: + fresh, complete = [], False + self.last_incomplete = str(error) + self.error(str(error)) + current = {identity_key(p): p for p in fresh} + for key, previous in list(self.identities.items()): + if key in current: + continue + current[key] = previous # An uninspected identity is not an empty result. + try: + inspection_timeout(deadline) + identity = kernel_identity(previous["pid"]) + inspection_timeout(deadline) + except Exception as error: + complete = False + self.last_incomplete = str(error) + if not isinstance(error, SnapshotDeadline): + self.error(str(error)) + if isinstance(error, SnapshotDeadline): + # Retain the rest without inspecting beyond this pass's budget. + current.update({k: p for k, p in self.identities.items() if k not in current}) + break + continue + if identity is None or identity["zombie"] or identity["start_us"] != previous["start_us"]: + current.pop(key) + self.identities.pop(key) + if key in self.escaped: + outcome = {signal.SIGTERM: "terminated", signal.SIGKILL: "killed"}.get( + self.sent.get(key), "exited") + self.record(previous, outcome) + self.sent.pop(key, None) + for key, identity in current.items(): + self.identities[key] = identity + if identity["pgid"] != self.child_pid and key not in self.escaped: + self.record(identity, "retained") + if time.monotonic() >= deadline: + complete = False + self.last_incomplete = "inspection deadline expired; completeness uncertain" + self.saw_owned = bool(observed) or bool(current) + return list(current.values()), complete + + def signal(self, identities, signum): + for identity in identities: + key = identity_key(identity) + if identity["pgid"] == self.child_pid or self.sent.get(key) == signum: + continue + try: + if signal_owned_process(identity, signum): + self.sent[key] = signum + except Exception as error: + self.error(f"signal escaped PID {identity['pid']}: {error}") + + def retained(self, identities): + for identity in identities: + if identity_key(identity) in self.escaped: + self.record(identity, "retained") + return "; ".join(f"PID {p['pid']} ({p['comm']}) start {p['start']} " + f"start_us {p['start_us']} uid {p['uid']}" for p in identities) + + +def owned_state(child, owned, deadline): + remaining, complete = owned.snapshot(deadline) + try: + group_live = group_has_members(child.pid, deadline=deadline) + except Exception as error: + group_live, complete = None, False + owned.last_incomplete = str(error) + if not isinstance(error, SnapshotDeadline) and not ( + isinstance(error, subprocess.TimeoutExpired) and time.monotonic() >= deadline): + owned.error(f"inspect pinned group: {error}") + if time.monotonic() >= deadline: + complete = False + owned.last_incomplete = "inspection deadline expired; completeness uncertain" + return remaining, group_live, complete + + +def confirm_owned_exit(child, owned, deadline, escalate=True, poll_seconds=.1): + empty_at = None + while True: + remaining, group_live, complete = owned_state(child, owned, deadline) + now = time.monotonic() + if now < deadline and complete and not remaining and not group_live and not owned.saw_owned: + if empty_at is not None and now - empty_at >= .1: + return + empty_at = now if empty_at is None else empty_at + else: + empty_at = None + if escalate: + owned.signal(remaining, signal.SIGKILL) + if now >= deadline: + retained = owned.retained(remaining) + raise RuntimeError("completeness uncertain; " + + (f"live members remained; retained: {retained or 'pinned group'}" + if remaining or group_live else ("live membership unknown; no two complete empty passes" + if not complete else "no two complete empty passes")) + + (f"; {owned.last_incomplete}" if owned.last_incomplete else "")) + # Two complete empty passes reduce successor races, but a chain shorter + # than a scan interval can still be invisible between both passes. + delay = .1 if empty_at is not None else poll_seconds + time.sleep(min(delay, deadline - now)) + + +def stop_owned_command(child, owned=None): if child is None: return None - # Never poll or reap before the last group operation: the leader pins the pgid. - if signal_owned_group(child.pid, signal.SIGTERM): - deadline = time.monotonic() + CLEANUP_GRACE_SECONDS - while time.monotonic() < deadline: - if not group_has_members(child.pid): - break - time.sleep(0.05) - else: - signal_owned_group(child.pid, signal.SIGKILL) - deadline = time.monotonic() + CLEANUP_CONFIRM_SECONDS - while group_has_members(child.pid): + owned = owned if owned is not None else OwnedProcesses({}, child.pid) + remaining, complete = owned.snapshot(time.monotonic() + SENSING_TIMEOUT) + if not complete: + owned.error("initial discovery incomplete: " + (owned.last_incomplete or "unknown reason")) + # Signal discovered escapes first. Their identity stays proven even if their + # parents die on the group SIGTERM and launchd adopts them immediately. + owned.signal(remaining, signal.SIGTERM) + # Discovery failures must never skip either signal to the pinned group. + try: + signal_owned_group(child.pid, signal.SIGTERM) + except Exception as error: + owned.error(f"SIGTERM pinned group: {error}") + deadline = time.monotonic() + CLEANUP_GRACE_SECONDS + while True: + remaining, group_live, complete = owned_state(child, owned, deadline) + if complete and not group_live and not remaining: + break + owned.signal(remaining, signal.SIGTERM) if time.monotonic() >= deadline: - raise RuntimeError(f"could not confirm owned group {child.pid} gone") - time.sleep(0.05) + owned.signal(remaining, signal.SIGKILL) + try: + signal_owned_group(child.pid, signal.SIGKILL) + except Exception as error: + owned.error(f"SIGKILL pinned group: {error}") + break + time.sleep(min(.1, max(0, deadline - time.monotonic()))) + try: + confirm_owned_exit(child, owned, time.monotonic() + CLEANUP_CONFIRM_SECONDS) + except Exception as error: + if owned.errors: + raise RuntimeError(f"{error}; " + "; ".join(owned.errors)) from error + raise + if owned.errors: + raise RuntimeError("cleanup discovery or signaling failed: " + "; ".join(owned.errors)) + # Never poll or reap before the final complete pass: the leader pins its pgid. return child.wait(timeout=CLEANUP_CONFIRM_SECONDS) -def hold_owned_group(child): - deadline = time.monotonic() + CLEANUP_HOLD_SECONDS - while True: - try: - if not group_has_members(child.pid): - return "no live members remained when the shared QA lock was released" - remaining = "live members remained" - except Exception as error: - remaining = f"live membership was unknown ({error})" - seconds = deadline - time.monotonic() - if seconds <= 0: - return f"{remaining} when the shared QA lock was released after the cleanup hold expired" - time.sleep(min(1, seconds)) +def hold_owned_group(child, owned=None): + owned = owned if owned is not None else OwnedProcesses({}, child.pid) + try: + confirm_owned_exit(child, owned, time.monotonic() + CLEANUP_HOLD_SECONDS, + escalate=False, poll_seconds=1) + return "no live members remained when the shared QA lock was released" + except Exception as error: + return (f"{error} when the shared QA lock was released " + "after the cleanup hold expired") def cleanup_step(errors, label, action): @@ -335,6 +736,7 @@ def create_job(args): signal=None, error=None, shutdownExitCode=None, cleanupError=None, queuedAt=timestamp(), admittedAt=None, finishedAt=None, admissionDeadline=args.admission_deadline, timeout=args.timeout, + ownerToken=secrets.token_hex(16), escapedProcesses=[], ignoredProcesses=[], logs={name: str(directory / (name + ".log")) for name in ("output", "runner")}) (directory / "output.log").touch() @@ -370,7 +772,7 @@ def release_ticket(path, ticket): def run_job(directory, live, job, lock_path, echo=True, owned_ticket=None, deadline=None): - global_lock = child = None + global_lock = child = owned = None child_finished = False cleanup_errors = [] ticket_path, ticket = owned_ticket if owned_ticket else (None, None) @@ -384,6 +786,14 @@ def update(**fields): log.write(line + "\n") if echo: print(line, flush=True) + def report_escaped(identity, outcome): + record = {key: identity[key] for key in ("pid", "start_us", "uid", "start", "comm", "proof")} + record["outcome"] = outcome + records = job.setdefault("escapedProcesses", []) + records[:] = [p for p in records if (p["pid"], p.get("start_us"), p.get("uid")) != + identity_key(record)] + [record] + cleanup_step(cleanup_errors, "record escaped process", + lambda: update(escapedProcesses=records)) def interrupted(signum, frame): # A second signal must not escape while the first exception is handled. for sig in RUNNER_SIGNALS: @@ -421,7 +831,9 @@ def interrupted(signum, frame): with bounded(deadline - time.monotonic()): pressure, memory = sense_memory() reasons = resource_blockers(pressure, memory, sense_disk()) - heavy, ignored = [], [] + heavy, owner_reasons = owner_blockers() + reasons.extend(owner_reasons) + ignored = [] for process in list_processes(): classification = classify_process(process, job["kind"], job["device"]) if classification == "blocker": @@ -483,8 +895,14 @@ def interrupted(signum, frame): with (directory / "output.log").open("a") as output: # Record cancellation without blocking signals inherited by the payload. with deferred_signals(RUNNER_SIGNALS) as pending: + environment = os.environ.copy() + if job.get("ownerToken") is None: + job["ownerToken"] = secrets.token_hex(16) + environment[f"{OWNER_ENV_PREFIX}{job['ownerToken']}"] = job["jobId"] child = subprocess.Popen(job["argv"], cwd=job["cwd"], start_new_session=True, - stdin=subprocess.DEVNULL, stdout=output, stderr=subprocess.STDOUT) + env=environment, stdin=subprocess.DEVNULL, + stdout=output, stderr=subprocess.STDOUT) + owned = OwnedProcesses(job, child.pid, report_escaped) child_started = time.monotonic() job["childPgid"] = child.pid write_job(directory, job) @@ -510,7 +928,7 @@ def mark_cleanup_errors(): for sig in old_signals: cleanup_step(cleanup_errors, f"ignore signal {sig}", lambda sig=sig: signal.signal(sig, signal.SIG_IGN)) try: - code = stop_owned_command(child) + code = stop_owned_command(child, owned) if child_finished and job["status"] == "running": job.update(status="succeeded" if code == 0 else "failed", exitCode=code, signal=-code if code < 0 else None) @@ -520,7 +938,7 @@ def mark_cleanup_errors(): cleanup_step(cleanup_errors, "write cleanup hold status", lambda: write_job(directory, job)) if child is not None: try: - cleanup_errors.append(hold_owned_group(child)) + cleanup_errors.append(hold_owned_group(child, owned)) except Exception as hold_error: cleanup_errors.append(f"cleanup hold: {hold_error}; live membership unknown at lock release") # All group signaling and inspection are over before this reap. @@ -720,6 +1138,9 @@ def globals_for(target, suppress=False): args.state_dir = args.state_dir.expanduser().resolve() args.lock = args.lock.expanduser().resolve() if args.command in ("run", "submit"): + owners = owner_jobs(f"{key}={value}" for key, value in os.environ.items()) + if owners: + parser.error(f"nested qa-resource jobs are not supported: this process belongs to job {owners[0]}") args.cwd = args.cwd.expanduser().resolve() if not args.cwd.is_dir(): parser.error("--cwd must be an existing directory") diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py index f30bf658e..b71e857c6 100644 --- a/tools/qa-resource/test_qa_resource.py +++ b/tools/qa-resource/test_qa_resource.py @@ -6,6 +6,7 @@ import os from pathlib import Path import select +import secrets import signal import subprocess import sys @@ -18,6 +19,8 @@ spec = importlib.util.spec_from_file_location("qa_resource", SCRIPT) qa = importlib.util.module_from_spec(spec) spec.loader.exec_module(qa) +TEST_OWNER_PREFIX = f"QA_RESOURCE_TEST_{secrets.token_hex(16)}_" +qa.OWNER_ENV_PREFIX = TEST_OWNER_PREFIX # Only test-side code sees this configuration. Production has no test switches. FAKE = r'''#!PYTHON @@ -66,6 +69,19 @@ else: sys.exit(99) ''' +TEST_SETUP = '''import sys; sys.path.insert(0, sys.argv[1]); import qa_resource as q +q.OWNER_ENV_PREFIX = q.os.environ['QA_TEST_OWNER_PREFIX'] +class TestPopen(q.subprocess.Popen): + def __init__(self, args, *positional, **keywords): + if (isinstance(args, (list, tuple)) and len(args) > 2 and args[1] == '-c' + and args[2].startswith('import sys; sys.path.insert(0, sys.argv[1]); import qa_resource as q; sys.exit(q.resume(')): + args = list(args) + settings = ('q.OWNER_ENV_PREFIX=' + repr(q.OWNER_ENV_PREFIX) + '; ' + 'q.CLEANUP_GRACE_SECONDS=.2; q.RESAMPLE_SECONDS=.1; ') + args[2] = args[2].replace('import qa_resource as q; ', 'import qa_resource as q; ' + settings) + super().__init__(args, *positional, **keywords) +q.subprocess.Popen = TestPopen +''' BOOTSTRAP = ("import sys; sys.path.insert(0, sys.argv[1]); import qa_resource as q; " "q.CLEANUP_GRACE_SECONDS=.2; q.RESAMPLE_SECONDS=.1; " "sys.exit(q.main(sys.argv[2:]))") @@ -89,9 +105,10 @@ def setUp(self): path.write_text(FAKE.replace("PYTHON", sys.executable, 1)) path.chmod(0o755) self.env = dict(os.environ, PATH=str(binary) + os.pathsep + os.environ.get("PATH", ""), - QA_TEST_CONFIG=str(self.config_path)) + QA_TEST_CONFIG=str(self.config_path), QA_TEST_OWNER_PREFIX=TEST_OWNER_PREFIX) self.processes = [] self.detached = [] + self.escaped_targets = [] def tearDown(self): for receipt in self.detached: @@ -123,6 +140,10 @@ def tearDown(self): pgid = qa.read_job(directory)["childPgid"] if pgid and qa.group_has_members(pgid): qa.signal_owned_group(pgid, signal.SIGKILL) + for pid, start in self.escaped_targets: + identity = qa.process_identity(pid, details=True) + if identity and identity["start"] == start and not identity["stat"].startswith("Z"): + os.kill(pid, signal.SIGKILL) # Only an individually recorded test-spawned PID. self.temporary.cleanup() def configure(self, **changes): @@ -145,14 +166,14 @@ def job_args(self, job, code="pass", kind="package", deadline=8, timeout=3, devi return self.cli("run", job, args + ["--", sys.executable, "-c", code]) def launch(self, args, bootstrap=BOOTSTRAP): - process = subprocess.Popen([sys.executable, "-c", bootstrap, str(SCRIPT.parent), *args], + process = subprocess.Popen([sys.executable, "-c", TEST_SETUP + bootstrap, str(SCRIPT.parent), *args], env=self.env, start_new_session=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) self.processes.append(process) return process def invoke(self, args): - result = subprocess.run([sys.executable, str(SCRIPT), *args], env=self.env, + result = subprocess.run([sys.executable, "-c", TEST_SETUP + BOOTSTRAP, str(SCRIPT.parent), *args], env=self.env, capture_output=True, text=True, timeout=10) if args[0] == "submit" and result.returncode == 0: self.detached.append(json.loads(result.stdout)) @@ -210,6 +231,301 @@ def release_holder(self, process): process.stdin.close() process.wait(timeout=3) + def await_path(self, path, seconds=5): + deadline = time.monotonic() + seconds + while time.monotonic() < deadline: + if path.exists(): + return + time.sleep(.02) + self.fail(f"test payload did not create {path}") + + def escaped_pid(self, path): + self.await_path(path) + pid = int(path.read_text()) + identity = qa.process_identity(pid, details=True) + if identity is not None: + self.escaped_targets.append((pid, identity["start"])) + return pid + + def escape_program(self, files, ignores, parent_live=True, ancestry=False, parents_ignore=True): + programs = [] + for path, ignore in zip(files, ignores): + programs.append("import os,signal,time; from pathlib import Path; " + + ("signal.signal(signal.SIGTERM,signal.SIG_IGN); " if ignore else "") + + f"Path({str(path)!r}).write_text(str(os.getpid())); time.sleep(30)") + middle = "import os,signal,subprocess,sys,time; from pathlib import Path\n" + if ancestry and parents_ignore: + middle += "signal.signal(signal.SIGTERM,signal.SIG_IGN)\n" + middle += "environment=dict(os.environ)\n" + if ancestry: + middle += f"environment={{k:v for k,v in environment.items() if not k.startswith({TEST_OWNER_PREFIX!r})}}\n" + for code in programs: + middle += f"subprocess.Popen([sys.executable,'-c',{code!r}], start_new_session=True,env=environment)\n" + middle += (f"files={list(map(str, files))!r}\nend=time.monotonic()+4\n" + "while not all(Path(p).exists() for p in files):\n" + " if time.monotonic()>=end: raise RuntimeError('grandchild startup deadline')\n" + " time.sleep(.01)\n") + if ancestry: + middle += "time.sleep(30)\n" + parent = ("import signal,subprocess,sys,time; from pathlib import Path\n" + + ("signal.signal(signal.SIGTERM,signal.SIG_IGN)\n" if ancestry and parents_ignore else "") + + f"middle=subprocess.Popen([sys.executable,'-c',{middle!r}])\n") + if ancestry: + parent += (f"files={list(map(str, files))!r}\nend=time.monotonic()+4\n" + "while not all(Path(p).exists() for p in files):\n" + " if time.monotonic()>=end: raise RuntimeError('ancestry startup deadline')\n" + " time.sleep(.01)\n") + else: + parent += "middle.wait(timeout=5)\n" + parent += f"Path({str(self.root / 'parent-ready')!r}).touch()\n" + if parent_live: + parent += "time.sleep(30)\n" + return parent + + def escaped_gone(self, pid): + identity = qa.process_identity(pid, details=True) + return identity is None or identity["stat"].startswith("Z") + + def assert_escaped_result(self, job, expected): + records = {p["pid"]: p for p in job["escapedProcesses"]} + self.assertEqual(set(records), set(expected)) + for pid, (proof, outcome) in expected.items(): + self.assertEqual(records[pid]["proof"], proof) + self.assertEqual(records[pid]["outcome"], outcome) + self.assertTrue(records[pid]["start"]) + self.assertTrue(records[pid]["comm"]) + self.assertTrue(self.escaped_gone(pid), records[pid]) + log = Path(job["logs"]["runner"]).read_text() + self.assertIn('"escapedProcesses"', log) + self.assert_lock_free() + + def test_cancel_reparented_session_descendants_and_preserve_sentinel(self): + sentinel = self.sleeper(30) + paths = [self.root / "normal", self.root / "ignores"] + runner = self.launch(self.job_args("escape-cancel", self.escape_program(paths, [False, True]), timeout=10)) + self.await_path(self.root / "parent-ready") + pids = [self.escaped_pid(p) for p in paths] + for pid in pids: + identity = qa.process_identity(pid, details=True) + self.assertEqual(identity["ppid"], 1) + self.assertEqual(identity["pgid"], pid) + self.assertEqual(self.invoke(self.cli("cancel", "escape-cancel")).returncode, 0) + result = self.finish(runner, "escape-cancel", 130) + self.assert_escaped_result(result, {pids[0]: ("token", "terminated"), pids[1]: ("token", "killed")}) + self.assertIsNone(sentinel.poll()) + + def test_timeout_kills_escaped_ignoring_descendant_not_sentinel(self): + sentinel = self.sleeper(30) + path = self.root / "escaped" + runner = self.launch(self.job_args("escape-timeout", self.escape_program([path], [True]), timeout=.8)) + pid = self.escaped_pid(path) + result = self.finish(runner, "escape-timeout", 124) + self.assertEqual(result["status"], "timed_out") + self.assert_escaped_result(result, {pid: ("token", "killed")}) + self.assertIsNone(sentinel.poll()) + + def test_natural_exit_cleans_detached_helper_before_lock_release(self): + path = self.root / "escaped" + runner = self.launch(self.job_args("escape-natural", self.escape_program([path], [True], parent_live=False))) + pid = self.escaped_pid(path) + deadline = time.monotonic() + 6 + with self.lock.open("a+") as probe: + while True: + try: + fcntl.flock(probe, fcntl.LOCK_EX | fcntl.LOCK_NB) + self.assertTrue(self.escaped_gone(pid), "escaped helper lived past lock release") + break + except BlockingIOError: + if time.monotonic() >= deadline: + self.fail("natural-exit cleanup did not release the test lock") + time.sleep(.01) + result = self.finish(runner, "escape-natural", 0) + self.assertEqual(result["status"], "succeeded") + self.assert_escaped_result(result, {pid: ("token", "killed")}) + + def test_tokenless_session_descendant_is_owned_by_live_ancestry(self): + path = self.root / "ancestry" + runner = self.launch(self.job_args("escape-ancestry", self.escape_program([path], [True], ancestry=True), timeout=10)) + self.await_path(self.root / "parent-ready") + pid = self.escaped_pid(path) + self.assertFalse(any(entry.startswith(TEST_OWNER_PREFIX) for entry in qa.process_env(pid))) + self.assertNotEqual(qa.process_identity(pid, details=True)["ppid"], 1) + self.assertEqual(self.invoke(self.cli("cancel", "escape-ancestry")).returncode, 0) + result = self.finish(runner, "escape-ancestry", 130) + self.assert_escaped_result(result, {pid: ("ancestry", "killed")}) + + def test_ineffective_escaped_kill_records_retained_and_holds_lock(self): + path = self.root / "retained" + args = Mock(state_dir=self.state, job_id="retained", kind="package", + argv=[sys.executable, "-c", self.escape_program([path], [True], parent_live=False)], + cwd=self.root, device=None, admission_deadline=5, timeout=3) + directory, live, job = qa.create_job(args) + real_kill, real_sleep = os.kill, time.sleep + held = [] + def ineffective(pid, sig): + if path.exists() and pid == int(path.read_text()): + return None + return real_kill(pid, sig) + def observe_hold(seconds): + if qa.read_job(directory)["status"] == "cleanup_failed": + with self.lock.open("a+") as probe: + with self.assertRaises(BlockingIOError): + fcntl.flock(probe, fcntl.LOCK_EX | fcntl.LOCK_NB) + held.append(time.monotonic()) + real_sleep(seconds) + with patch.object(qa, "sense_memory", return_value=(1, 80)), \ + patch.object(qa, "sense_disk", return_value=10 * 1024**3), \ + patch.object(qa, "list_processes", return_value=[]), \ + patch.object(qa, "owner_blockers", return_value=([], [])), \ + patch.object(qa, "CLEANUP_GRACE_SECONDS", .1), \ + patch.object(qa, "CLEANUP_CONFIRM_SECONDS", .1), \ + patch.object(qa, "CLEANUP_HOLD_SECONDS", .2), \ + patch.object(qa.os, "kill", side_effect=ineffective), \ + patch.object(qa.time, "sleep", side_effect=observe_hold): + self.assertEqual(qa.run_job(directory, live, job, self.lock, echo=False), 125) + pid = self.escaped_pid(path) + result = qa.read_job(directory) + self.assertEqual(result["status"], "cleanup_failed") + self.assertTrue(held) + self.assertIn(str(pid), result["cleanupError"]) + self.assertIn("hold expired", result["cleanupError"]) + record = result["escapedProcesses"][0] + self.assertEqual(record["outcome"], "retained") + for field in ("comm", "start"): + self.assertIn(record[field], result["cleanupError"]) + self.assertFalse(self.escaped_gone(pid)) + self.assert_lock_free() + + def test_token_orphan_admission_deadline_never_signals_orphan(self): + token = secrets.token_hex(16) + orphan = subprocess.Popen([sys.executable, "-c", "import time; time.sleep(30)"], + env=dict(self.env, **{TEST_OWNER_PREFIX + token: "older-job"}), + start_new_session=True) + self.processes.append(orphan) + runner = self.launch(self.job_args("orphan-blocked", deadline=1)) + self.await_job("orphan-blocked", lambda j: any("owner token of job older-job" in r for r in j["reasons"])) + result = self.finish(runner, "orphan-blocked", 75) + self.assertEqual(result["status"], "admission_timeout") + self.assertIsNone(orphan.poll()) + self.assertIsNone(result["childPgid"]) + self.assert_lock_free() + + def test_token_bearing_ancestor_is_excluded_when_runner_has_no_token(self): + outer_name = TEST_OWNER_PREFIX + secrets.token_hex(16) + self.env[outer_name] = "ancestor-job" + path = self.root / "inherited" + code = ("import os,json; from pathlib import Path; " + f"Path({str(path)!r}).write_text(json.dumps({{k:v for k,v in os.environ.items() if k.startswith({TEST_OWNER_PREFIX!r})}}))") + bootstrap = BOOTSTRAP.replace("sys.exit(q.main", f"q.os.environ.pop({outer_name!r}); sys.exit(q.main") + argv = [sys.executable, "-c", TEST_SETUP + bootstrap, str(SCRIPT.parent), *self.job_args("nested", code)] + parent = self.launch([], bootstrap=f"import subprocess,sys; sys.exit(subprocess.call({argv!r}))") + output, error = parent.communicate(timeout=8) + self.assertEqual(parent.returncode, 0, (output, error)) + result = self.job("nested") + entries = json.loads(path.read_text()) + self.assertNotIn(outer_name, entries) + self.assertEqual(entries[TEST_OWNER_PREFIX + result["ownerToken"]], "nested") + self.assertEqual(len(entries), 1) + self.assertEqual(result["status"], "succeeded") + + def test_old_metadata_without_ownership_fields_supports_status_wait_cancel(self): + runner = self.launch(self.job_args("old-fields", "import time; time.sleep(30)", timeout=40)) + self.await_job("old-fields", lambda j: j["status"] == "running") + directory = self.state / "jobs" / "old-fields" + old = qa.read_job(directory) + old.pop("ownerToken") + old.pop("escapedProcesses") + qa.write_job(directory, old) + self.assertEqual(self.invoke(self.cli("status", "old-fields")).returncode, 0) + self.assertEqual(self.invoke(self.cli("wait", "old-fields", ["--max-wait", ".1"])).returncode, 3) + self.assertEqual(self.invoke(self.cli("cancel", "old-fields")).returncode, 0) + self.finish(runner, "old-fields", 130) + terminal = qa.read_job(directory) + terminal.pop("ownerToken", None) + terminal.pop("escapedProcesses", None) + qa.write_job(directory, terminal) + self.assertEqual(self.invoke(self.cli("wait", "old-fields", ["--max-wait", "1"])).returncode, 130) + + def test_tokenless_escape_survives_parent_term_but_not_durable_cleanup(self): + sentinel = self.sleeper(30) + path = self.root / "tokenless-normal-parents" + code = self.escape_program([path], [True], ancestry=True, parents_ignore=False) + runner = self.launch(self.job_args("durable-ancestry", code, timeout=10)) + self.await_path(self.root / "parent-ready") + pid = self.escaped_pid(path) + self.assertFalse(any(entry.startswith(TEST_OWNER_PREFIX) for entry in qa.process_env(pid))) + self.assertEqual(self.invoke(self.cli("cancel", "durable-ancestry")).returncode, 0) + result = self.finish(runner, "durable-ancestry", 130) + self.assert_escaped_result(result, {pid: ("ancestry", "killed")}) + self.assertGreater(result["escapedProcesses"][0]["start_us"], 0) + self.assertIsNone(sentinel.poll()) + + def test_empty_argv_sleep_is_found_on_cancel_and_natural_exit(self): + for natural in (False, True): + with self.subTest(natural=natural): + path = self.root / f"empty-argv-{natural}" + code = ("import os,subprocess,time; from pathlib import Path; " + f"env={{k:v for k,v in os.environ.items() if k.startswith({TEST_OWNER_PREFIX!r})}}; " + "child=subprocess.Popen(['','30'], executable='/bin/sleep',env=env,start_new_session=True); " + f"Path({str(path)!r}).write_text(str(child.pid)); " + + ("time.sleep(30)" if not natural else "pass")) + name = f"empty-argv-{natural}" + runner = self.launch(self.job_args(name, code, timeout=10)) + pid = self.escaped_pid(path) + if not natural: + entry = qa.owner_entry(self.job(name)) + self.assertIn(entry, qa.process_env(pid)) + self.assertEqual(self.invoke(self.cli("cancel", name)).returncode, 0) + result = self.finish(runner, name, 0 if natural else 130) + self.assert_escaped_result(result, {pid: ("token", "terminated")}) + + def test_failed_discovery_still_term_and_kills_pinned_group_and_holds_lock(self): + code = "import signal,time; signal.signal(signal.SIGTERM,signal.SIG_IGN); time.sleep(30)" + args = Mock(state_dir=self.state, job_id="snapshot-failure", kind="package", + argv=[sys.executable, "-c", code], cwd=self.root, device=None, + admission_deadline=5, timeout=.2) + directory, live, job = qa.create_job(args) + real_killpg, real_sleep = os.killpg, time.sleep + held = [] + def observe(seconds): + if qa.read_job(directory)["status"] == "cleanup_failed": + with self.lock.open("a+") as probe: + with self.assertRaises(BlockingIOError): + fcntl.flock(probe, fcntl.LOCK_EX | fcntl.LOCK_NB) + held.append(True) + real_sleep(seconds) + with patch.object(qa, "sense_memory", return_value=(1, 80)), \ + patch.object(qa, "sense_disk", return_value=10 * 1024**3), \ + patch.object(qa, "list_processes", return_value=[]), \ + patch.object(qa, "owner_blockers", return_value=([], [])), \ + patch.object(qa, "owned_processes", side_effect=RuntimeError("injected ps snapshot failure")), \ + patch.object(qa, "CLEANUP_GRACE_SECONDS", .1), \ + patch.object(qa, "CLEANUP_CONFIRM_SECONDS", .1), \ + patch.object(qa, "CLEANUP_HOLD_SECONDS", .2), \ + patch.object(qa.os, "killpg", wraps=real_killpg) as killpg, \ + patch.object(qa.time, "sleep", side_effect=observe): + self.assertEqual(qa.run_job(directory, live, job, self.lock, echo=False), 125) + result = qa.read_job(directory) + self.assertEqual(result["status"], "cleanup_failed") + self.assertIn("injected ps snapshot failure", result["cleanupError"]) + self.assertTrue(held) + self.assertEqual([call.args for call in killpg.call_args_list], + [(result["childPgid"], signal.SIGTERM), (result["childPgid"], signal.SIGKILL)]) + self.assertFalse(qa.group_has_members(result["childPgid"])) + self.assert_lock_free() + + def test_run_and_submit_refuse_inherited_owner_before_creating_job(self): + self.env[TEST_OWNER_PREFIX + secrets.token_hex(16)] = "outer-job" + for command in ("run", "submit"): + args = self.job_args("nested-refused") + args[0] = command + result = self.invoke(args) + self.assertEqual(result.returncode, 2, result.stderr) + self.assertIn("nested qa-resource jobs are not supported: this process belongs to job outer-job", + result.stderr) + self.assertFalse(self.state.exists()) + self.assertFalse(self.lock.exists()) + def test_two_queued_jobs_run_in_submission_order_without_overlap(self): events = self.root / "events" def program(name): @@ -354,7 +670,7 @@ def test_blocked_submit_receipt_does_not_hold_finished_jobs_liveness(self): os.set_blocking(writer, True) args = self.job_args("blocked-receipt") args[0] = "submit" - submitter = subprocess.Popen([sys.executable, str(SCRIPT), *args], env=self.env, + submitter = subprocess.Popen([sys.executable, "-c", TEST_SETUP + BOOTSTRAP, str(SCRIPT.parent), *args], env=self.env, start_new_session=True, stdout=writer, stderr=subprocess.PIPE, text=True) self.processes.append(submitter) os.close(writer) @@ -411,7 +727,10 @@ def test_cancel_queued_never_starts_child(self): self.assert_lock_free() def test_stale_status_wait_cancel_and_later_admission(self): - runner = self.launch(self.job_args("stale", "import time; time.sleep(10)")) + natural_exit = self.root / "orphan-natural-exit" + code = ("import time; from pathlib import Path; time.sleep(2.5); " + f"Path({str(natural_exit)!r}).touch()") + runner = self.launch(self.job_args("stale", code)) job = self.await_job("stale", lambda j: j["status"] == "running") runner.kill() runner.wait(timeout=3) @@ -426,7 +745,9 @@ def test_stale_status_wait_cancel_and_later_admission(self): send.assert_not_called() self.assertTrue(qa.group_has_members(job["childPgid"])) next_runner = self.launch(self.job_args("later")) + self.await_job("later", lambda j: any("owner token of job stale" in r for r in j["reasons"])) self.finish(next_runner, "later", 0) + self.assertTrue(natural_exit.exists(), "admission must not signal the orphan") self.assert_lock_free() def test_boot_failure_is_error_without_shutdown(self): @@ -604,7 +925,225 @@ def test_unknown_job_duplicate_id_and_cli_validation(self): self.assertEqual(self.invoke(args).returncode, 2) +class OwnershipTests(unittest.TestCase): + def test_procargs_candidates_preserve_padding_and_empty_arguments(self): + raw = ((3).to_bytes(4, sys.byteorder, signed=True) + b"/path with spaces/python\0\0\0" + + b"python\0\0-c\0A=one=two\0OWNER=value\0\0not=environment\0") + self.assertEqual(qa.parse_process_env(raw, 123), ["python", "-c", "A=one=two", "OWNER=value", "not=environment"]) + for raw in (b"", (2).to_bytes(4, sys.byteorder, signed=True) + b"missing-exec-nul"): + with self.subTest(raw=raw), self.assertRaisesRegex(OSError, "malformed KERN_PROCARGS2"): + qa.parse_process_env(raw, 123) + + def test_sysctl_error_preserves_errno(self): + with patch.object(qa, "_SYSCTL", return_value=-1), \ + patch.object(qa.ctypes, "get_errno", return_value=qa.errno.EPERM): + with self.assertRaises(OSError) as error: + qa._sysctl_read([1, 49, 123], qa.ctypes.create_string_buffer(16)) + self.assertEqual(error.exception.errno, qa.errno.EPERM) + self.assertIn("sysctl [1, 49, 123]", str(error.exception)) + + def test_identity_parses_command_with_spaces_and_rejects_bad_dates(self): + line = "Thu Oct 1 12:34:56 2026 /an executable with spaces/python\n" + with patch.object(qa.subprocess, "run", return_value=Mock(returncode=0, stdout=line)): + self.assertEqual(qa.process_identity(123), + dict(pid=123, start="Thu Oct 1 12:34:56 2026", + comm="/an executable with spaces/python")) + with patch.object(qa.subprocess, "run", return_value=Mock(returncode=0, stdout="bad date python")): + with self.assertRaisesRegex(RuntimeError, "malformed lstart/comm"): + qa.process_identity(123) + + def test_changed_microsecond_start_uid_and_zombie_never_receive_signal(self): + identity = dict(pid=123, start_us=123_000_001, uid=os.getuid(), + start="old", comm="python", proof="token") + current = dict(pid=123, start_us=identity["start_us"], uid=os.getuid(), zombie=False) + for changes in (dict(start_us=123_000_002), dict(uid=os.getuid() + 1), dict(zombie=True)): + with self.subTest(changes=changes), \ + patch.object(qa, "kernel_identity", return_value=dict(current, **changes)), \ + patch.object(qa.os, "kill") as kill, patch.object(qa.os, "killpg") as killpg: + self.assertFalse(qa.signal_owned_process(identity, signal.SIGKILL)) + kill.assert_not_called() + killpg.assert_not_called() + + def test_token_match_is_exact_and_unreadable_nonancestry_is_not_owned(self): + job = dict(jobId="job", ownerToken="a" * 32) + rows = {pid: dict(pid=pid, ppid=1, pgid=pid, uid=os.getuid(), stat="S") for pid in (123, 124, 125)} + def env(pid): + if pid == 125: + raise OSError(qa.errno.EPERM, "unreadable") + return [qa.owner_entry(job) + ("-other" if pid == 123 else "")] + with patch.object(qa, "process_snapshot", return_value=rows), \ + patch.object(qa, "process_env", side_effect=env), \ + patch.object(qa, "kernel_identity", side_effect=lambda pid: dict(pid=pid, start_us=pid * 1_000_000, uid=rows[pid]["uid"], zombie=False)), \ + patch.object(qa, "process_identity", side_effect=lambda pid, **unused: dict(pid=pid, start="date", comm="python")): + self.assertEqual([p["pid"] for p in qa.owned_processes(job, 99)], [124]) + + def test_foreign_uid_descendant_is_retained_without_environment_read_or_signal(self): + job = dict(jobId="job", ownerToken="a" * 32) + rows = {123: dict(pid=123, ppid=1, pgid=123, uid=os.getuid(), stat="S"), + 124: dict(pid=124, ppid=123, pgid=124, uid=os.getuid() + 1, stat="S")} + with patch.object(qa, "process_snapshot", return_value=rows), \ + patch.object(qa, "process_env", return_value=[]) as env, \ + patch.object(qa, "kernel_identity", side_effect=lambda pid: dict(pid=pid, start_us=pid * 1_000_000, uid=rows[pid]["uid"], zombie=False)), \ + patch.object(qa, "process_identity", side_effect=lambda pid, **unused: dict(pid=pid, start="date", comm="python")): + found = qa.owned_processes(job, 123) + self.assertEqual([(p["pid"], p["proof"]) for p in found], [(123, "ancestry"), (124, "ancestry")]) + env.assert_called_once_with(123) + with patch.object(qa, "kernel_identity", return_value=dict(found[1], zombie=False)), \ + patch.object(qa, "process_env") as env, patch.object(qa.os, "kill") as kill: + self.assertFalse(qa.signal_owned_process(found[1], signal.SIGTERM)) + env.assert_not_called() + kill.assert_not_called() + + def test_proven_identity_survives_lost_token_and_reparenting(self): + identity = dict(pid=124, uid=os.getuid(), start_us=124_000_001, + start="child-start", comm="python", proof="ancestry") + with patch.object(qa, "kernel_identity", return_value=dict(identity, zombie=False)), \ + patch.object(qa, "process_identity") as ps, \ + patch.object(qa, "process_env", return_value=[]) as env, patch.object(qa.os, "kill") as kill: + self.assertTrue(qa.signal_owned_process(identity, signal.SIGTERM)) + env.assert_not_called() + ps.assert_not_called() + kill.assert_called_once_with(124, signal.SIGTERM) + + def test_kernel_start_microseconds_agree_with_live_ps_lstart(self): + identity = qa.kernel_identity(os.getpid()) + human = qa.runner_start(os.getpid()) + seconds = int(time.mktime(time.strptime(human, "%a %b %d %H:%M:%S %Y"))) + self.assertEqual(identity["pid"], os.getpid()) + self.assertEqual(identity["uid"], os.getuid()) + self.assertEqual(identity["start_us"] // 1_000_000, seconds) + self.assertFalse(identity["zombie"]) + + def test_kinfo_pid_and_size_mismatch_fail_loudly(self): + def read(mib, buffer, returned=648, pid=123): + self.assertEqual(mib, [1, 14, 1, 123]) + value = qa.ProcessTimeval.from_buffer(buffer) + value.seconds, value.microseconds = 1_700_000_000, 123456 + qa.ctypes.c_int.from_buffer(buffer, 40).value = pid + qa.ctypes.c_uint.from_buffer(buffer, 420).value = os.getuid() + qa.ctypes.c_byte.from_buffer(buffer, 36).value = 2 + return returned + with patch.object(qa, "_sysctl_read", side_effect=read): + self.assertEqual(qa.kernel_identity(123)["start_us"], 1_700_000_000_123456) + for size, pid, message in ((647, 123, "size mismatch"), (648, 124, "PID mismatch")): + with self.subTest(size=size, pid=pid), \ + patch.object(qa, "_sysctl_read", side_effect=lambda mib, buf: read(mib, buf, size, pid)): + with self.assertRaisesRegex(RuntimeError, message): + qa.kernel_identity(123) + + def test_dead_identity_is_dropped_after_one_inspection(self): + previous = dict(pid=123, start_us=123_000_001, uid=os.getuid(), pgid=123, + start="date", comm="python", proof="token") + for current in (None, dict(previous, start_us=123_000_002, zombie=False), dict(previous, zombie=True)): + with self.subTest(current=current): + owned = qa.OwnedProcesses({}, 99) + key = qa.identity_key(previous) + owned.identities[key] = previous + owned.escaped[key] = "retained" + with patch.object(qa, "owned_processes", return_value=[]), \ + patch.object(qa, "kernel_identity", return_value=current) as inspect: + self.assertEqual(owned.snapshot(time.monotonic() + 1), ([], True)) + self.assertEqual(owned.snapshot(time.monotonic() + 1), ([], True)) + inspect.assert_called_once_with(123) + self.assertNotIn(key, owned.identities) + self.assertEqual(owned.escaped[key], "exited") + + def test_slow_identity_snapshot_stops_at_deadline_and_is_incomplete(self): + job = dict(jobId="slow", ownerToken="a" * 32) + rows = {pid: dict(pid=pid, ppid=1, pgid=pid, uid=os.getuid(), stat="S") for pid in range(123, 133)} + clock = [0.0] + def human(pid, **unused): + clock[0] += .2 + return dict(pid=pid, start="date", comm="python") + with patch.object(qa.time, "monotonic", side_effect=lambda: clock[0]), \ + patch.object(qa, "process_snapshot", return_value=rows), \ + patch.object(qa, "kernel_identity", side_effect=lambda pid: dict(pid=pid, start_us=pid, uid=os.getuid(), zombie=False)), \ + patch.object(qa, "process_env", return_value=[qa.owner_entry(job)]), \ + patch.object(qa, "process_identity", side_effect=human) as inspect: + owned = qa.OwnedProcesses(job, 99) + remaining, complete = owned.snapshot(.35) + self.assertFalse(complete) + self.assertEqual(inspect.call_count, 2) + self.assertAlmostEqual(clock[0], .4) + self.assertTrue(remaining) + self.assertIn("deadline", owned.last_incomplete) + + def test_candidate_disappearance_is_settled_or_incomplete(self): + job = dict(jobId="job", ownerToken="a" * 32) + row = dict(pid=123, ppid=1, pgid=123, uid=os.getuid(), stat="S") + native = dict(pid=123, start_us=123_000_001, uid=os.getuid(), zombie=False) + for after in (None, native): + with self.subTest(after=after), patch.object(qa, "process_snapshot", return_value={123: row}), \ + patch.object(qa, "kernel_identity", side_effect=[native, after]), \ + patch.object(qa, "process_env", return_value=[qa.owner_entry(job)]), \ + patch.object(qa, "process_identity", return_value=None): + if after is None: + observed = [] + self.assertEqual(qa.owned_processes(job, 99, observed=observed), []) + self.assertEqual(observed, [123]) + else: + with self.assertRaisesRegex(qa.SnapshotIncomplete, "unsettled owned identity"): + qa.owned_processes(job, 99) + with patch.object(qa, "process_snapshot", return_value={123: row}), \ + patch.object(qa, "kernel_identity", return_value=native), \ + patch.object(qa, "process_env", side_effect=OSError(qa.errno.ESRCH, "gone")): + self.assertEqual(qa.owned_processes(job, 99), []) + + def test_confirmation_needs_two_complete_empty_passes_after_every_reset(self): + previous = dict(pid=123, start_us=123, uid=os.getuid(), pgid=123, + start="date", comm="python", proof="token") + sequence = [([previous], True), ([], True), ([], False), ([], True), + ([previous], True), ([], True), ([], True)] + owned, child, clock = qa.OwnedProcesses({}, 99), Mock(pid=99), [0.0] + def sleep(seconds): + clock[0] += seconds + .001 + with patch.object(owned, "snapshot", side_effect=sequence) as snapshots, \ + patch.object(owned, "signal") as signal_owned, \ + patch.object(qa, "group_has_members", return_value=False), \ + patch.object(qa.time, "monotonic", side_effect=lambda: clock[0]), \ + patch.object(qa.time, "sleep", side_effect=sleep): + qa.confirm_owned_exit(child, owned, 1) + self.assertEqual(snapshots.call_count, 7) + self.assertGreaterEqual(clock[0], .6) + self.assertEqual(signal_owned.call_args_list[0].args, ([previous], signal.SIGKILL)) + clock[0] = 0 + with patch.object(owned, "snapshot", return_value=([], False)), \ + patch.object(qa, "group_has_members", return_value=False), \ + patch.object(qa.time, "monotonic", side_effect=lambda: clock[0]), \ + patch.object(qa.time, "sleep", side_effect=sleep): + with self.assertRaisesRegex(RuntimeError, "completeness uncertain"): + qa.confirm_owned_exit(child, owned, .3) + + def test_transient_owned_process_resets_empty_confirmation(self): + owned, child, clock = qa.OwnedProcesses({}, 99), Mock(pid=99), [0.0] + activity = iter([False, True, False, False]) + def snapshot(deadline): + owned.saw_owned = next(activity) + return [], True + def sleep(seconds): + clock[0] += seconds + .001 + with patch.object(owned, "snapshot", side_effect=snapshot) as snapshots, \ + patch.object(qa, "group_has_members", return_value=False), \ + patch.object(qa.time, "monotonic", side_effect=lambda: clock[0]), \ + patch.object(qa.time, "sleep", side_effect=sleep): + qa.confirm_owned_exit(child, owned, 1) + self.assertEqual(snapshots.call_count, 4) + + def test_owner_header_requires_exact_lowercase_32_hex_token(self): + valid = TEST_OWNER_PREFIX + "a" * 32 + "=job" + invalid = [TEST_OWNER_PREFIX + "a" * 31 + "=short", TEST_OWNER_PREFIX + "a" * 33 + "=long", + TEST_OWNER_PREFIX + "A" * 32 + "=uppercase", TEST_OWNER_PREFIX + "name=not-token"] + self.assertEqual(qa.owner_jobs([valid, *invalid]), ["job"]) + + class PureTests(unittest.TestCase): + def setUp(self): + # These tests mock payload Popen; never inspect or signal their fictitious PIDs. + for name in ("owner_blockers", "owned_processes"): + mock = patch.object(qa, name, return_value=([], []) if name == "owner_blockers" else []) + mock.start() + self.addCleanup(mock.stop) + def test_agent_device_classification_both_only_testing_forms(self): for flag in (f"-only-testing {qa.SERVICE_TEST}", f"-only-testing:{qa.SERVICE_TEST}"): process = dict(comm="/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild", @@ -681,7 +1220,7 @@ def sleep(seconds): with self.assertRaises(BlockingIOError): fcntl.flock(probe, fcntl.LOCK_EX | fcntl.LOCK_NB) clock[0] += seconds - def members(pgid): + def members(pgid, **unused): return members_remain or clock[0] < 1 with patch.object(qa.time, "monotonic", side_effect=lambda: clock[0]), \ patch.object(qa.time, "sleep", side_effect=sleep), \ @@ -699,7 +1238,7 @@ def members(pgid): self.assertEqual(result["status"], "cleanup_failed") expected = "live members remained" if members_remain else "no live members remained" self.assertIn(expected, result["cleanupError"]) - self.assertEqual(clock[0], 2 if members_remain else 1) + self.assertAlmostEqual(clock[0], 2 if members_remain else 1.1) self.assertNotIn("pass_fds", spawn.call_args.kwargs) with lock_path.open("a+") as probe: fcntl.flock(probe, fcntl.LOCK_EX | fcntl.LOCK_NB) From 484a4c161dd1b3a870d338869091c395413b0084 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Thu, 1 Oct 2026 14:44:22 -0500 Subject: [PATCH 03/15] tools: count Blender.app as heavy and replace the queue counter atomically Admission compared executable names case-sensitively, so Blender run from /Applications/Blender.app (executable "Blender") did not block a second heavy job. Names are now compared in lower case. The ticket counter was rewritten in place. A kill or a full disk between truncation and write left it empty, and every later submission failed parsing it. It is now written to counter.tmp and moved over with os.replace, as job.json already is. Co-Authored-By: Claude Opus 5.5 (1M context) --- tools/qa-resource/qa_resource.py | 14 +++++++++++--- tools/qa-resource/test_qa_resource.py | 27 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/tools/qa-resource/qa_resource.py b/tools/qa-resource/qa_resource.py index 3b94f481a..59413ddb5 100644 --- a/tools/qa-resource/qa_resource.py +++ b/tools/qa-resource/qa_resource.py @@ -46,6 +46,11 @@ "error", "cleanup_failed"} + +def is_heavy(comm): + # Compare case-insensitively: Blender.app's executable is .../Contents/MacOS/Blender. + return Path(comm).name.lower() in HEAVY_NAMES + class Cancelled(Exception): def __init__(self, signum): self.signum = signum @@ -354,7 +359,7 @@ def list_processes(): processes = [] for row in rows.splitlines(): pid, pgid, state, comm = row.strip().split(maxsplit=3) - if int(pid) == os.getpid() or state.startswith("Z") or Path(comm).name not in HEAVY_NAMES: + if int(pid) == os.getpid() or state.startswith("Z") or not is_heavy(comm): continue # comm may contain spaces. Only the first three columns are split. # A timed-out ps is killed, but subprocess.run's subsequent reap is unbounded. @@ -381,7 +386,7 @@ def option_values(tokens, option): def classify_process(process, kind, device=None): """Return blocker, ignored, or irrelevant without inspecting the machine.""" - if process.get("stat", "").startswith("Z") or Path(process["comm"]).name not in HEAVY_NAMES: + if process.get("stat", "").startswith("Z") or not is_heavy(process["comm"]): return "irrelevant" try: tokens = shlex.split(process["args"]) @@ -753,7 +758,10 @@ def acquire_ticket(state_dir, job_id, deadline): fcntl.flock(guard, fcntl.LOCK_EX) counter = queue / "counter" seq = int(counter.read_text()) + 1 if counter.exists() else 1 - counter.write_text(str(seq)) + # Replace atomically: a truncated counter would fail every later submission. + temporary = queue / "counter.tmp" + temporary.write_text(str(seq)) + os.replace(temporary, counter) path = queue / f"{seq:012d}.{job_id}" ticket = path.open("a+") fcntl.flock(ticket, fcntl.LOCK_EX | fcntl.LOCK_NB) diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py index b71e857c6..6c1d59091 100644 --- a/tools/qa-resource/test_qa_resource.py +++ b/tools/qa-resource/test_qa_resource.py @@ -1137,6 +1137,33 @@ def test_owner_header_requires_exact_lowercase_32_hex_token(self): class PureTests(unittest.TestCase): + def test_blender_app_bundle_executable_blocks_package_job(self): + process = dict(comm="/Applications/Blender.app/Contents/MacOS/Blender", + args="/Applications/Blender.app/Contents/MacOS/Blender -b scene.blend -f 1") + self.assertEqual(qa.classify_process(process, "package"), "blocker") + self.assertTrue(qa.is_heavy("/usr/local/bin/blender")) + self.assertFalse(qa.is_heavy("/Applications/T3 Code.app/Contents/MacOS/T3 Code")) + + def test_counter_survives_failed_write_and_next_ticket_continues(self): + with tempfile.TemporaryDirectory() as temp: + state = Path(temp) + path, ticket = qa.acquire_ticket(state, "first", time.monotonic() + 3) + qa.release_ticket(path, ticket) + counter = state / "queue" / "counter" + self.assertEqual(counter.read_text(), "1") + real_write = Path.write_text + def failing(self, *args, **kwargs): + if self.name == "counter.tmp": + raise OSError(28, "No space left on device") + return real_write(self, *args, **kwargs) + with patch.object(Path, "write_text", failing): + with self.assertRaises(OSError): + qa.acquire_ticket(state, "second", time.monotonic() + 3) + self.assertEqual(counter.read_text(), "1") + path, ticket = qa.acquire_ticket(state, "third", time.monotonic() + 3) + self.assertEqual(path.name, "000000000002.third") + qa.release_ticket(path, ticket) + def setUp(self): # These tests mock payload Popen; never inspect or signal their fictitious PIDs. for name in ("owner_blockers", "owned_processes"): From 00d516c118dc86755036179a6251626ce7688250 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Thu, 1 Oct 2026 15:23:09 -0500 Subject: [PATCH 04/15] tools: cancel queue jobs through a per-job FIFO, never a runner PID `cancel` checked the runner's start time and then sent SIGTERM to its PID, so a PID reused in between could receive the signal. Each runner now holds the read end of cancel.fifo in its job directory, and a listener thread turns a byte there into SIGTERM for the runner's own main thread. `cancel` writes that byte. With no runner reading, the open fails with ENXIO and nothing is sent. Jobs started by an older runner have no FIFO and are refused. An interrupted `submit` used to exit 130 without a receipt while its detached runner kept going. The runner now starts with SIGTERM, SIGINT and SIGHUP blocked and unblocks them once its handlers are installed. `submit` keeps SIGCHLD at its default so the runner stays its unreaped child, cancels it on any of those signals, writes the receipt unbuffered with cancelRequested, and returns 130. Cleanup tolerates a cancel that lands as it begins, and stops the listener before handlers are restored. The listener's fd is never closed while its thread could still be blocked reading it. Co-Authored-By: Claude Opus 5.5 (1M context) --- tools/qa-resource/README.md | 5 +- tools/qa-resource/qa_resource.py | 195 ++++++++++++++++++++++---- tools/qa-resource/test_qa_resource.py | 153 +++++++++++++++++++- 3 files changed, 319 insertions(+), 34 deletions(-) diff --git a/tools/qa-resource/README.md b/tools/qa-resource/README.md index fab32e2d9..49cbe81bc 100644 --- a/tools/qa-resource/README.md +++ b/tools/qa-resource/README.md @@ -20,6 +20,8 @@ Read `~/.codex/qa-resource/jobs/ID/job.json` for status, reasons, timestamps, ow Nothing pushes a completion event to T3 or a manager session. A parent learns the result in one of two ways. It can block in `wait`, running it again with the same id if its shell tool's time limit is shorter than the job. Or it can start `run` as a background command and let its host report the exit. Either way, `job.json` holds the result. +`cancel` never signals a PID. Each runner reads a per-job FIFO, `cancel.fifo` in the job directory, and cancels itself when a byte arrives. When no runner holds the read end, `cancel` cannot open the FIFO, reports that nothing was sent, and returns 4, so a reused runner PID can never receive the request. Jobs started by an older runner have no FIFO, and `cancel` refuses them. If `submit` is interrupted after starting the runner, it cancels that runner, which is still its own unreaped child, and prints the receipt with `cancelRequested: true` before returning 130, so `wait` can follow the job to `cancelled`. + Read `job.json`, not just the exit code. `run` and `wait` return the child's code for success or failure, 124 for child timeout, 130 for cancellation, 75 for admission timeout, and 125 for sensing or cleanup errors. `wait` returns 3 when its own deadline expires without affecting the job, 4 for a runner that died without a terminal result, and 2 for an unknown job. Child exit codes can overlap these values. If a child dies from signal N, `run` and `wait` return 128+N. `job.json` keeps the negative child return code in `exitCode` and N in `signal`. Admission requires memory pressure level 1 or 2, at least 35% memory free, and at least 5 GiB free on the HOME volume. These are operational limits copied from `qa-slot.py`, not calibrated measurements. Unreadable resources fail admission. Heavy build and render processes block admission, except a verified agent-device `testCommand` service. That service blocks a simulator job only on the same device. The named simulator must exist and be Shutdown; other booted devices do not block. Package and render jobs never call `simctl`. @@ -46,9 +48,8 @@ Invisible descendants are not stopped. Darwin omits the environment of `cs_restr A chain whose processes each live shorter than one scan interval can evade two-pass confirmation. Microsecond start times avoid coarse `lstart` identity checks for escaped PIDs, but the check-to-signal PID reuse window remains. -The queue cannot identify a foreign shared-lock holder or attribute a heavy process to a project. FIFO ordering applies only to this tool's tickets; `qa-slot.py` and `package-run.py` still race for the shared flock. Cancellation signals only a verified live runner, which stops its own child group and proven escaped descendants, and shuts down only the device it booted. The runner handles SIGTERM, SIGINT, and SIGHUP; later signals cannot interrupt cleanup. A stale job's recorded group and booted-device ownership are reported, never cleaned. Inspect those manually before reusing a device. +The queue cannot identify a foreign shared-lock holder or attribute a heavy process to a project. FIFO ordering applies only to this tool's tickets; `qa-slot.py` and `package-run.py` still race for the shared flock. Cancellation stops only the job's own child group and proven escaped descendants, and shuts down only the device the job booted. The runner handles SIGTERM, SIGINT, and SIGHUP; later signals cannot interrupt cleanup. A stale job's recorded group and booted-device ownership are reported, never cleaned. Inspect those manually before reusing a device. -Cancel can signal a reused PID if reuse happens between the runner start-time check and SIGTERM; eliminating that window on macOS needs per-job IPC. Boot ownership does not identify a simulator session; if another lane shuts down and re-boots the same UDID mid-job, final shutdown affects their session. diff --git a/tools/qa-resource/qa_resource.py b/tools/qa-resource/qa_resource.py index 59413ddb5..96638c34b 100644 --- a/tools/qa-resource/qa_resource.py +++ b/tools/qa-resource/qa_resource.py @@ -16,8 +16,10 @@ import secrets import shlex import signal +import stat import subprocess import sys +import threading import time # Operational limits copied from qa-slot.py, not calibrated measurements. @@ -42,6 +44,8 @@ HEAVY_NAMES = {"xcodebuild", "swift-build", "swift-test", "swift-frontend", "swift-driver", "blender"} SERVICE_TEST = "AgentDeviceRunnerUITests/RunnerTests/testCommand" +CANCEL_FIFO = "cancel.fifo" +STOP_LISTENER = b"q" TERMINAL = {"succeeded", "failed", "timed_out", "cancelled", "admission_timeout", "error", "cleanup_failed"} @@ -741,15 +745,77 @@ def create_job(args): signal=None, error=None, shutdownExitCode=None, cleanupError=None, queuedAt=timestamp(), admittedAt=None, finishedAt=None, admissionDeadline=args.admission_deadline, timeout=args.timeout, - ownerToken=secrets.token_hex(16), escapedProcesses=[], + ownerToken=secrets.token_hex(16), escapedProcesses=[], cancelChannel=CANCEL_FIFO, ignoredProcesses=[], logs={name: str(directory / (name + ".log")) for name in ("output", "runner")}) (directory / "output.log").touch() (directory / "runner.log").touch() + os.mkfifo(directory / CANCEL_FIFO, 0o600) write_job(directory, job) return directory, live, job +class CancelListener: + """Turn a byte on the job's FIFO into SIGTERM for this runner's main thread. + + `cancel` never signals a PID. It can write only while this process holds the read end; + once the runner is gone the open fails with ENXIO, so a reused PID is never reached. + The caller owns this object as soon as it exists, so a cancel that interrupts + start() still reaches stop() in cleanup. + """ + + def __init__(self, directory): + # O_RDWR keeps a writer open too, so read blocks instead of returning EOF. Not inheritable. + self.fd = os.open(directory / CANCEL_FIFO, os.O_RDWR) + self.thread = None + self.active = True + self.guard = threading.Lock() + self.main = threading.main_thread().ident + + def _listen(self): + while True: + try: + byte = os.read(self.fd, 1) + except OSError: + return + if not byte or byte == STOP_LISTENER: + return + with self.guard: + # stop() clears this first, so nothing is delivered once cleanup has begun. + if self.active: + signal.pthread_kill(self.main, signal.SIGTERM) + + def start(self): + # Start the thread with every signal blocked, so SIGALRM, SIGTERM and SIGINT keep + # interrupting the main thread's flock, sleep and kqueue waits. A cancel read before + # the mask is restored is raised here, after self.thread is recorded. + previous = signal.pthread_sigmask(signal.SIG_BLOCK, signal.valid_signals()) + try: + self.thread = threading.Thread(target=self._listen, name="qa-resource-cancel", daemon=True) + self.thread.start() + finally: + signal.pthread_sigmask(signal.SIG_SETMASK, previous) + + def stop(self): + with self.guard: + self.active = False + if self.thread is None: + os.close(self.fd) + return + # Closing an fd while another thread is blocked reading it can hang on macOS, so + # close only after the thread has provably exited. Otherwise leave it for process exit. + os.write(self.fd, STOP_LISTENER) + self.thread.join(timeout=CLEANUP_CONFIRM_SECONDS) + if self.thread.is_alive(): + raise RuntimeError("cancel listener did not exit; its FIFO fd stays open until the runner exits") + os.close(self.fd) + + +def ignore_runner_signals(signals): + for sig in signals: + signal.signal(sig, signal.SIG_IGN) + + def acquire_ticket(state_dir, job_id, deadline): queue = state_dir / "queue" queue.mkdir(parents=True, exist_ok=True) @@ -808,7 +874,14 @@ def interrupted(signum, frame): signal.signal(sig, signal.SIG_IGN) raise Cancelled(signum) old_signals = {sig: signal.signal(sig, interrupted) for sig in RUNNER_SIGNALS} + listener = None try: + # submit starts the runner with these blocked; a cancel that arrived meanwhile is + # raised here, inside the try, so it still gets a terminal result and cleanup. + signal.pthread_sigmask(signal.SIG_UNBLOCK, RUNNER_SIGNALS) + if (directory / CANCEL_FIFO).exists(): + listener = CancelListener(directory) + listener.start() update(runnerPid=os.getpid(), runnerStart=runner_start(os.getpid())) if ticket is None: ticket_path, ticket = acquire_ticket(directory.parent.parent, job["jobId"], deadline) @@ -933,8 +1006,14 @@ def interrupted(signum, frame): def mark_cleanup_errors(): if cleanup_errors: job.update(status="cleanup_failed", exitCode=125, cleanupError="; ".join(cleanup_errors)) - for sig in old_signals: - cleanup_step(cleanup_errors, f"ignore signal {sig}", lambda sig=sig: signal.signal(sig, signal.SIG_IGN)) + try: + ignore_runner_signals(old_signals) + except Cancelled: + # The one-shot handler fired as cleanup began. It already ignores further signals, + # and the job's outcome was settled before cleanup, so cleanup simply continues. + ignore_runner_signals(old_signals) + if listener is not None: + cleanup_step(cleanup_errors, "stop cancel listener", listener.stop) try: code = stop_owned_command(child, owned) if child_finished and job["status"] == "running": @@ -1000,22 +1079,66 @@ def submit(directory, live, job, lock_path): deadline = time.monotonic() + job["admissionDeadline"] try: ticket_path, ticket = acquire_ticket(directory.parent.parent, job["jobId"], deadline) - with (directory / "runner.log").open("a") as log: - with deferred_signals((signal.SIGTERM, signal.SIGINT)) as pending: - runner = subprocess.Popen([sys.executable, "-c", script, str(Path(__file__).parent), - str(directory), str(live.fileno()), str(lock_path), - str(ticket.fileno()), str(ticket_path), str(deadline)], - pass_fds=(live.fileno(), ticket.fileno()), start_new_session=True, - stdin=subprocess.DEVNULL, stdout=log, stderr=subprocess.STDOUT) + # Keep SIGCHLD at its default so an exited runner stays an unreaped zombie and its PID + # cannot be reused before an interrupted submit signals it. An inherited SIG_IGN + # would make the kernel reap it at once. + previous_sigchld = signal.signal(signal.SIGCHLD, signal.SIG_DFL) + try: + pending = [] + def record(signum, frame): + if not pending: + pending.append(signum) + def interrupt(signum, frame): + # One-shot, so a second signal cannot escape the cancellation below. + for sig in RUNNER_SIGNALS: + signal.signal(sig, signal.SIG_IGN) + raise Cancelled(signum) + previous_handlers = {sig: signal.signal(sig, record) for sig in RUNNER_SIGNALS} + try: + with (directory / "runner.log").open("a") as log: + # The runner inherits this mask and unblocks after installing its handlers, + # so a SIGTERM sent to it right after spawn still gets a clean cancellation. + previous_mask = signal.pthread_sigmask(signal.SIG_BLOCK, RUNNER_SIGNALS) + try: + runner = subprocess.Popen([sys.executable, "-c", script, str(Path(__file__).parent), + str(directory), str(live.fileno()), str(lock_path), + str(ticket.fileno()), str(ticket_path), str(deadline)], + pass_fds=(live.fileno(), ticket.fileno()), start_new_session=True, + stdin=subprocess.DEVNULL, stdout=log, stderr=subprocess.STDOUT) + finally: + signal.pthread_sigmask(signal.SIG_SETMASK, previous_mask) # A blocked receipt writer must not keep a finished job alive. cleanup_step(close_errors, "close submitted ticket fd", ticket.close) cleanup_step(close_errors, "close submitted liveness fd", live.close) - if pending: - raise Cancelled(pending[0]) + receipt = dict(jobId=job["jobId"], jobDir=str(directory), + resultFile=str(directory / "job.json"), pid=runner.pid) + printing = False + # From here an interruption cancels the job instead of abandoning it, and a + # submitter blocked on a full stdout pipe can still be stopped. + for sig in RUNNER_SIGNALS: + signal.signal(sig, interrupt) + try: + if pending: + raise Cancelled(pending[0]) + printing = True + write_receipt(receipt) + except Cancelled: + # Still our unreaped child (SIGCHLD is default and nothing here waits on + # it), so this PID cannot belong to another process. + os.kill(runner.pid, signal.SIGTERM) + if not printing: + receipt["cancelRequested"] = True + for sig, handler in previous_handlers.items(): + signal.signal(sig, handler) + write_receipt(receipt) + return 130 + finally: + for sig, handler in previous_handlers.items(): + signal.signal(sig, handler) if close_errors: raise RuntimeError("; ".join(close_errors)) - print(json.dumps(dict(jobId=job["jobId"], jobDir=str(directory), - resultFile=str(directory / "job.json"), pid=runner.pid)), flush=True) + finally: + signal.signal(signal.SIGCHLD, previous_sigchld) except Cancelled: return 130 except Exception as error: @@ -1040,6 +1163,14 @@ def submit(directory, live, job, lock_path): return 0 +def write_receipt(receipt): + # Unbuffered, so an interrupted write leaves nothing for exit to flush into a full pipe. + sys.stdout.flush() + data = (json.dumps(receipt) + "\n").encode() + while data: + data = data[os.write(sys.stdout.fileno(), data):] + + def live_report(directory): with (directory / "live.lock").open("r") as live: try: @@ -1080,28 +1211,36 @@ def wait_job(directory, max_wait): def cancel_job(directory): - # Keep a shared fd open for inspection, but never signal after lock acquisition. + # Cancellation never signals a PID. It writes one byte to the job's FIFO, which only the + # live runner reads; with no reader the open fails, so nothing else can receive it. with (directory / "live.lock").open("r") as live: try: fcntl.flock(live, fcntl.LOCK_SH | fcntl.LOCK_NB) except BlockingIOError: job = read_job(directory) - if not job["runnerPid"] or not job["runnerStart"]: - print("runner identity is not recorded yet; no signal sent", file=sys.stderr) + if not job.get("cancelChannel"): + print(f"job {job.get('jobId', directory.name)} was started by a runner without a cancel channel; " + "this version never signals a runner PID, so use the qa_resource.py that " + "started it; no signal sent", file=sys.stderr) return 4 try: - matches = runner_start(job["runnerPid"]) == job["runnerStart"] - except Exception: - matches = False - if not matches: - print("runner start time does not match; no signal sent", file=sys.stderr) - return 4 + fd = os.open(directory / CANCEL_FIFO, os.O_WRONLY | os.O_NONBLOCK) + except OSError as error: + if error.errno in (errno.ENXIO, errno.ENOENT): + print("runner is not listening for cancellation; no signal sent", file=sys.stderr) + return 4 + raise try: - # macOS needs per-job IPC to rule out PID reuse between the start-time check and this signal. - os.kill(job["runnerPid"], signal.SIGTERM) - except ProcessLookupError: - print("runner exited; no signal sent", file=sys.stderr) - return 4 + if not stat.S_ISFIFO(os.fstat(fd).st_mode): + print(f"{directory / CANCEL_FIFO} is not a FIFO; nothing sent", file=sys.stderr) + return 4 + try: + os.write(fd, b"c") + except BrokenPipeError: + print("runner stopped listening for cancellation; no signal sent", file=sys.stderr) + return 4 + finally: + os.close(fd) print(json.dumps(dict(jobId=job["jobId"], cancelRequested=True))) return 0 print(json.dumps(live_report(directory))) diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py index 6c1d59091..8240ff8ad 100644 --- a/tools/qa-resource/test_qa_resource.py +++ b/tools/qa-resource/test_qa_resource.py @@ -647,13 +647,70 @@ def interrupted_spawn(argv, *args, **kwargs): q.subprocess.Popen = interrupted_spawn sys.exit(q.main(sys.argv[2:])) ''' - args = self.job_args("submit-cancel") + args = self.job_args("submit-cancel", "import time; time.sleep(30)", timeout=40) args[0] = "submit" submitter = self.launch(args, bootstrap) - self.finish(submitter, "submit-cancel", 130) + output, error = submitter.communicate(timeout=8) + self.assertEqual(submitter.returncode, 130, error) + # An interrupted submit still reports the job it started, and cancels it. + receipt = json.loads(output) + self.assertEqual(receipt["jobId"], "submit-cancel") + self.assertTrue(receipt["cancelRequested"]) result = self.invoke(self.cli("wait", "submit-cancel", ["--max-wait", "6"])) - self.assertEqual(result.returncode, 0, result.stderr) - self.assertEqual(json.loads(result.stdout)["status"], "succeeded") + self.assertEqual(result.returncode, 130, result.stderr) + job = json.loads(result.stdout) + self.assertEqual(job["status"], "cancelled") + # Usually the cancel lands before admission; if the payload did start, it must be gone. + if job["childPgid"] is not None: + self.assertFalse(qa.group_has_members(job["childPgid"])) + self.assert_lock_free() + + def test_submit_hangup_during_spawn_with_ignored_sigchld_cancels_and_reports(self): + bootstrap = BOOTSTRAP.split("sys.exit")[0] + ''' +q.signal.signal(q.signal.SIGCHLD, q.signal.SIG_IGN) +original = q.subprocess.Popen +def interrupted_spawn(argv, *args, **kwargs): + if 'q.resume' in ' '.join(argv): + # The runner PID stays pinned only if SIGCHLD is not ignored while submit can signal it. + assert q.signal.getsignal(q.signal.SIGCHLD) == q.signal.SIG_DFL + runner = original(argv, *args, **kwargs) + if 'q.resume' in ' '.join(argv): + q.signal.raise_signal(q.signal.SIGHUP) + return runner +q.subprocess.Popen = interrupted_spawn +sys.exit(q.main(sys.argv[2:])) +''' + args = self.job_args("submit-hup", "import time; time.sleep(30)", timeout=40) + args[0] = "submit" + submitter = self.launch(args, bootstrap) + output, error = submitter.communicate(timeout=8) + self.assertEqual(submitter.returncode, 130, error) + self.assertTrue(json.loads(output)["cancelRequested"]) + result = self.invoke(self.cli("wait", "submit-hup", ["--max-wait", "6"])) + self.assertEqual(result.returncode, 130, result.stderr) + job = json.loads(result.stdout) + self.assertEqual(job["status"], "cancelled") + if job["childPgid"] is not None: + self.assertFalse(qa.group_has_members(job["childPgid"])) + self.assert_lock_free() + + def test_cancel_at_cleanup_entry_still_finishes_cleanup(self): + runner_bootstrap = BOOTSTRAP.split("sys.exit")[0] + ''' +original = q.ignore_runner_signals +calls = [] +def cancelled_on_entry(signals): + calls.append(1) + if len(calls) == 1: + raise q.Cancelled(q.signal.SIGTERM) + return original(signals) +q.ignore_runner_signals = cancelled_on_entry +sys.exit(q.main(sys.argv[2:])) +''' + runner = self.launch(self.job_args("cleanup-entry", "import time; time.sleep(.2)"), runner_bootstrap) + result = self.finish(runner, "cleanup-entry", 0) + self.assertEqual(result["status"], "succeeded") + self.assertIsNotNone(result["finishedAt"]) + self.assertFalse(qa.group_has_members(result["childPgid"])) self.assert_lock_free() def test_blocked_submit_receipt_does_not_hold_finished_jobs_liveness(self): @@ -1206,6 +1263,94 @@ def test_eperm_zombie_and_uncertain_owned_group(self): with self.assertRaisesRegex(RuntimeError, "live members remain"): qa.signal_owned_group(123, signal.SIGTERM) + def test_cancel_writes_fifo_and_never_signals_a_pid(self): + with tempfile.TemporaryDirectory() as temp: + directory = Path(temp) + (directory / "live.lock").touch() + os.mkfifo(directory / qa.CANCEL_FIFO, 0o600) + qa.write_job(directory, dict(jobId="fifo", runnerPid=os.getpid(), runnerStart="x", + cancelChannel=qa.CANCEL_FIFO)) + with (directory / "live.lock").open("a+") as live: + fcntl.flock(live, fcntl.LOCK_EX) + with patch.object(qa.os, "kill") as kill, patch.object(qa.os, "killpg") as killpg, \ + contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()): + # Live lock held but no runner reading: nothing is sent anywhere. + self.assertEqual(qa.cancel_job(directory), 4) + reader = os.open(directory / qa.CANCEL_FIFO, os.O_RDONLY | os.O_NONBLOCK) + try: + self.assertEqual(qa.cancel_job(directory), 0) + self.assertEqual(os.read(reader, 8), b"c") + finally: + os.close(reader) + kill.assert_not_called() + killpg.assert_not_called() + + def test_cancel_reports_reader_gone_between_open_and_write(self): + with tempfile.TemporaryDirectory() as temp: + directory = Path(temp) + (directory / "live.lock").touch() + os.mkfifo(directory / qa.CANCEL_FIFO, 0o600) + qa.write_job(directory, dict(jobId="gone", cancelChannel=qa.CANCEL_FIFO)) + reader = os.open(directory / qa.CANCEL_FIFO, os.O_RDONLY | os.O_NONBLOCK) + try: + with (directory / "live.lock").open("a+") as live: + fcntl.flock(live, fcntl.LOCK_EX) + with patch.object(qa.os, "write", side_effect=BrokenPipeError(32, "Broken pipe")), \ + contextlib.redirect_stderr(io.StringIO()): + self.assertEqual(qa.cancel_job(directory), 4) + finally: + os.close(reader) + + def test_listener_stop_failure_never_hangs_and_disables_delivery(self): + with tempfile.TemporaryDirectory() as temp: + directory = Path(temp) + os.mkfifo(directory / qa.CANCEL_FIFO, 0o600) + listener = qa.CancelListener(directory) + listener.start() + with patch.object(qa.os, "write", side_effect=OSError(5, "Input/output error")): + with self.assertRaises(OSError): + listener.stop() + self.assertFalse(listener.active) + # A cancel that arrives after cleanup began is read but never delivered. + with patch.object(qa.signal, "pthread_kill") as deliver: + writer = os.open(directory / qa.CANCEL_FIFO, os.O_WRONLY | os.O_NONBLOCK) + os.write(writer, b"c") + os.write(writer, qa.STOP_LISTENER) + os.close(writer) + listener.thread.join(timeout=3) + deliver.assert_not_called() + self.assertFalse(listener.thread.is_alive()) + os.close(listener.fd) + + def test_listener_is_owned_before_its_start_can_be_interrupted(self): + with tempfile.TemporaryDirectory() as temp: + directory = Path(temp) + os.mkfifo(directory / qa.CANCEL_FIFO, 0o600) + listener = qa.CancelListener(directory) + with patch.object(qa.threading.Thread, "start", side_effect=qa.Cancelled(signal.SIGTERM)): + with self.assertRaises(qa.Cancelled): + listener.start() + # The caller already holds the object, so cleanup can still close its fd. + self.assertIsNotNone(listener.thread) + listener.thread = None + listener.stop() + with self.assertRaises(OSError): + os.fstat(listener.fd) + + def test_cancel_refuses_legacy_job_without_channel(self): + with tempfile.TemporaryDirectory() as temp: + directory = Path(temp) + (directory / "live.lock").touch() + qa.write_job(directory, dict(jobId="legacy", runnerPid=os.getpid(), + runnerStart=qa.runner_start(os.getpid()))) + with (directory / "live.lock").open("a+") as live: + fcntl.flock(live, fcntl.LOCK_EX) + stderr = io.StringIO() + with patch.object(qa.os, "kill") as kill, contextlib.redirect_stderr(stderr): + self.assertEqual(qa.cancel_job(directory), 4) + kill.assert_not_called() + self.assertIn("without a cancel channel", stderr.getvalue()) + def test_cancel_pid_start_mismatch_never_signals(self): with tempfile.TemporaryDirectory() as temp: directory = Path(temp) From 0ecaf300e71d7f916ff566b7db5b51dbabd796cd Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Thu, 1 Oct 2026 16:00:07 -0500 Subject: [PATCH 05/15] ci: run the QA resource queue tests on macOS The queue's 74 tests ran only when someone remembered to run them by hand. A new workflow runs them on macos-26 with Python 3.14 for pull requests that touch tools/qa-resource/ or the workflow, and for pushes to main. The tests need macOS because the queue reads Darwin process data, but they need no Xcode, simulator or build setup. The workflow can only read the repository, uses no secrets and does not persist checkout credentials. A newer commit cancels a pull request's older run, while each push to main gets its own concurrency group so a pending push is not replaced before it is tested. The job stops after 10 minutes. The --max-wait test now holds its payload on a release file instead of a 0.5 s sleep, so a slow runner cannot finish the job before the short wait times out. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/qa-resource.yml | 43 +++++++++++++++++++++++++++ tools/qa-resource/test_qa_resource.py | 8 ++++- 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/qa-resource.yml diff --git a/.github/workflows/qa-resource.yml b/.github/workflows/qa-resource.yml new file mode 100644 index 000000000..c81698922 --- /dev/null +++ b/.github/workflows/qa-resource.yml @@ -0,0 +1,43 @@ +name: QA resource queue + +on: + push: + branches: [main] + paths: + - "tools/qa-resource/**" + - ".github/workflows/qa-resource.yml" + pull_request: + paths: + - "tools/qa-resource/**" + - ".github/workflows/qa-resource.yml" + +# Pull request code runs here, so the token can only read and no secrets are used. +permissions: + contents: read + +# A newer commit cancels a pull request's older run. Each push to main gets a group of its own, +# because a shared group would replace a pending push before it was tested. +concurrency: + group: qa-resource-${{ github.event_name == 'pull_request' && github.ref || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +jobs: + qa-resource-tests: + name: QA resource queue tests + # The queue reads Darwin-only process data (sysctl KERN_PROCARGS2 and kinfo_proc) and + # uses kqueue, so its tests need macOS. They need no Xcode, simulator or build setup. + runs-on: macos-26 + # The 74 tests took about 55 s locally. The limit leaves room for runner setup. + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + # The suite was developed and passed on Python 3.14. + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.14" + - name: Run the queue tests + env: + PYTHONDONTWRITEBYTECODE: "1" + run: python -m unittest discover -s tools/qa-resource -p 'test_*.py' -v diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py index 8240ff8ad..c35b236bd 100644 --- a/tools/qa-resource/test_qa_resource.py +++ b/tools/qa-resource/test_qa_resource.py @@ -941,12 +941,18 @@ def test_submit_wait_returns_child_exit_code(self): self.assert_lock_free() def test_submit_wait_max_wait_does_not_affect_job(self): - args = self.job_args("short-wait", "import time; time.sleep(.5)") + # The payload waits for a release file, so a slow runner cannot finish it before the short wait. + release = self.root / "release-short-wait" + payload = ("import os, time\n" + f"while not os.path.exists({str(release)!r}):\n" + " time.sleep(.02)\n") + args = self.job_args("short-wait", payload, timeout=10) args[0] = "submit" self.assertEqual(self.invoke(args).returncode, 0) result = self.invoke(self.cli("wait", "short-wait", ["--max-wait", ".05"])) self.assertEqual(result.returncode, 3, result.stderr) self.assertTrue(json.loads(result.stdout)["live"]) + release.touch() final = self.invoke(self.cli("wait", "short-wait", ["--max-wait", "6"])) self.assertEqual(final.returncode, 0, final.stderr) self.assertEqual(json.loads(final.stdout)["status"], "succeeded") From 7e27301c038871b668767fae2b7a785274fed2c9 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Thu, 1 Oct 2026 17:23:59 -0500 Subject: [PATCH 06/15] tools: accept a cancel sent while the queue runner is starting `submit` printed its receipt before the detached runner opened cancel.fifo for reading. A `cancel` in that window got ENXIO, reported that nothing was listening, and the job then ran anyway. The command that creates the job now opens the FIFO's read end at once and hands it to the runner with the liveness lock and ticket. A cancel sent during startup is queued in the pipe, and the runner's listener delivers it once its handlers are installed. Once neither process holds the read end, `cancel` still cannot open it and sends nothing. Co-Authored-By: Claude Opus 5.5 (1M context) --- tools/qa-resource/README.md | 2 +- tools/qa-resource/qa_resource.py | 55 ++++++++++++++++++--------- tools/qa-resource/test_qa_resource.py | 28 ++++++++++++++ 3 files changed, 67 insertions(+), 18 deletions(-) diff --git a/tools/qa-resource/README.md b/tools/qa-resource/README.md index 49cbe81bc..554f5e240 100644 --- a/tools/qa-resource/README.md +++ b/tools/qa-resource/README.md @@ -20,7 +20,7 @@ Read `~/.codex/qa-resource/jobs/ID/job.json` for status, reasons, timestamps, ow Nothing pushes a completion event to T3 or a manager session. A parent learns the result in one of two ways. It can block in `wait`, running it again with the same id if its shell tool's time limit is shorter than the job. Or it can start `run` as a background command and let its host report the exit. Either way, `job.json` holds the result. -`cancel` never signals a PID. Each runner reads a per-job FIFO, `cancel.fifo` in the job directory, and cancels itself when a byte arrives. When no runner holds the read end, `cancel` cannot open the FIFO, reports that nothing was sent, and returns 4, so a reused runner PID can never receive the request. Jobs started by an older runner have no FIFO, and `cancel` refuses them. If `submit` is interrupted after starting the runner, it cancels that runner, which is still its own unreaped child, and prints the receipt with `cancelRequested: true` before returning 130, so `wait` can follow the job to `cancelled`. +`cancel` never signals a PID. Each runner reads a per-job FIFO, `cancel.fifo` in the job directory, and cancels itself when a byte arrives. The command that creates the job opens the read end before the runner starts and hands it to the runner, so a cancel sent during runner startup waits in the pipe and takes effect once the runner listens. When neither holds the read end, `cancel` cannot open the FIFO, reports that nothing was sent, and returns 4, so a reused runner PID can never receive the request. Jobs started by an older runner have no FIFO, and `cancel` refuses them. If `submit` is interrupted after starting the runner, it cancels that runner, which is still its own unreaped child, and prints the receipt with `cancelRequested: true` before returning 130, so `wait` can follow the job to `cancelled`. Read `job.json`, not just the exit code. `run` and `wait` return the child's code for success or failure, 124 for child timeout, 130 for cancellation, 75 for admission timeout, and 125 for sensing or cleanup errors. `wait` returns 3 when its own deadline expires without affecting the job, 4 for a runner that died without a terminal result, and 2 for an unknown job. Child exit codes can overlap these values. If a child dies from signal N, `run` and `wait` return 128+N. `job.json` keeps the negative child return code in `exitCode` and N in `signal`. diff --git a/tools/qa-resource/qa_resource.py b/tools/qa-resource/qa_resource.py index 96638c34b..b50982551 100644 --- a/tools/qa-resource/qa_resource.py +++ b/tools/qa-resource/qa_resource.py @@ -758,15 +758,16 @@ def create_job(args): class CancelListener: """Turn a byte on the job's FIFO into SIGTERM for this runner's main thread. - `cancel` never signals a PID. It can write only while this process holds the read end; - once the runner is gone the open fails with ENXIO, so a reused PID is never reached. + `cancel` never signals a PID. It can write only while the job's read end is open; once + the runner is gone the open fails with ENXIO, so a reused PID is never reached. The caller owns this object as soon as it exists, so a cancel that interrupts start() still reaches stop() in cleanup. """ - def __init__(self, directory): - # O_RDWR keeps a writer open too, so read blocks instead of returning EOF. Not inheritable. - self.fd = os.open(directory / CANCEL_FIFO, os.O_RDWR) + def __init__(self, directory, fd=None): + # The job's creator opens the read end before the runner starts and hands it over, so a + # cancel sent during runner startup waits in the pipe instead of failing with ENXIO. + self.fd = open_cancel_reader(directory) if fd is None else fd self.thread = None self.active = True self.guard = threading.Lock() @@ -811,6 +812,11 @@ def stop(self): os.close(self.fd) +def open_cancel_reader(directory): + # O_RDWR keeps a writer open too, so read blocks instead of returning EOF. Not inheritable. + return os.open(directory / CANCEL_FIFO, os.O_RDWR) + + def ignore_runner_signals(signals): for sig in signals: signal.signal(sig, signal.SIG_IGN) @@ -845,7 +851,8 @@ def release_ticket(path, ticket): return errors -def run_job(directory, live, job, lock_path, echo=True, owned_ticket=None, deadline=None): +def run_job(directory, live, job, lock_path, echo=True, owned_ticket=None, deadline=None, + cancel_fd=None): global_lock = child = owned = None child_finished = False cleanup_errors = [] @@ -879,8 +886,9 @@ def interrupted(signum, frame): # submit starts the runner with these blocked; a cancel that arrived meanwhile is # raised here, inside the try, so it still gets a terminal result and cleanup. signal.pthread_sigmask(signal.SIG_UNBLOCK, RUNNER_SIGNALS) - if (directory / CANCEL_FIFO).exists(): - listener = CancelListener(directory) + if cancel_fd is not None or (directory / CANCEL_FIFO).exists(): + listener = CancelListener(directory, cancel_fd) + cancel_fd = None listener.start() update(runnerPid=os.getpid(), runnerStart=runner_start(os.getpid())) if ticket is None: @@ -1014,6 +1022,8 @@ def mark_cleanup_errors(): ignore_runner_signals(old_signals) if listener is not None: cleanup_step(cleanup_errors, "stop cancel listener", listener.stop) + elif cancel_fd is not None: + cleanup_step(cleanup_errors, "close cancel fd", lambda: os.close(cancel_fd)) try: code = stop_owned_command(child, owned) if child_finished and job["status"] == "running": @@ -1061,19 +1071,20 @@ def mark_cleanup_errors(): return exit_code(job) -def resume(directory, fd, lock_path, ticket_fd, ticket_path, deadline): +def resume(directory, fd, lock_path, ticket_fd, ticket_path, deadline, cancel_fd): directory = Path(directory) with os.fdopen(fd, "a+") as live, os.fdopen(ticket_fd, "a+") as ticket: return run_job(directory, live, read_job(directory), Path(lock_path), echo=False, - owned_ticket=(Path(ticket_path), ticket), deadline=deadline) + owned_ticket=(Path(ticket_path), ticket), deadline=deadline, + cancel_fd=cancel_fd) -def submit(directory, live, job, lock_path): +def submit(directory, live, job, lock_path, cancel_fd): # Allocate the ticket before returning the receipt. Runner scheduling must not # reverse two completed submissions. Both locked descriptions survive exec. script = ("import sys; sys.path.insert(0, sys.argv[1]); import qa_resource as q; " "sys.exit(q.resume(sys.argv[2], int(sys.argv[3]), sys.argv[4], " - "int(sys.argv[5]), sys.argv[6], float(sys.argv[7])))") + "int(sys.argv[5]), sys.argv[6], float(sys.argv[7]), int(sys.argv[8])))") ticket_path = ticket = runner = None close_errors = [] deadline = time.monotonic() + job["admissionDeadline"] @@ -1102,14 +1113,19 @@ def interrupt(signum, frame): try: runner = subprocess.Popen([sys.executable, "-c", script, str(Path(__file__).parent), str(directory), str(live.fileno()), str(lock_path), - str(ticket.fileno()), str(ticket_path), str(deadline)], - pass_fds=(live.fileno(), ticket.fileno()), start_new_session=True, + str(ticket.fileno()), str(ticket_path), str(deadline), + str(cancel_fd)], + pass_fds=(live.fileno(), ticket.fileno(), cancel_fd), + start_new_session=True, stdin=subprocess.DEVNULL, stdout=log, stderr=subprocess.STDOUT) finally: signal.pthread_sigmask(signal.SIG_SETMASK, previous_mask) - # A blocked receipt writer must not keep a finished job alive. + # A blocked receipt writer must not keep a finished job alive, or keep the + # cancel FIFO open after its runner is gone. cleanup_step(close_errors, "close submitted ticket fd", ticket.close) cleanup_step(close_errors, "close submitted liveness fd", live.close) + cleanup_step(close_errors, "close submitted cancel fd", lambda: os.close(cancel_fd)) + cancel_fd = None receipt = dict(jobId=job["jobId"], jobDir=str(directory), resultFile=str(directory / "job.json"), pid=runner.pid) printing = False @@ -1157,6 +1173,8 @@ def interrupt(signum, frame): cleanup_step(close_errors, "release unsubmitted ticket", lambda: close_errors.extend(release_ticket(ticket_path, ticket))) cleanup_step(close_errors, "close submitted liveness fd", live.close) + if cancel_fd is not None: + cleanup_step(close_errors, "close cancel fd", lambda: os.close(cancel_fd)) if runner is None and close_errors: job.update(status="cleanup_failed", exitCode=125, cleanupError="; ".join(close_errors)) cleanup_step(close_errors, "write submission cleanup errors", lambda: write_job(directory, job)) @@ -1300,9 +1318,12 @@ def globals_for(target, suppress=False): directory, live, job = create_job(args) except ValueError as error: parser.error(str(error)) + # Hold the cancel FIFO's read end from job creation, so a cancel sent before the + # runner's listener starts is queued in the pipe rather than refused. + cancel_fd = open_cancel_reader(directory) if args.command == "submit": - return submit(directory, live, job, args.lock) - return run_job(directory, live, job, args.lock) + return submit(directory, live, job, args.lock, cancel_fd) + return run_job(directory, live, job, args.lock, cancel_fd=cancel_fd) directory = args.state_dir / "jobs" / (args.job_id or "") if args.job_id and not (directory / "job.json").is_file(): print(f"unknown job {args.job_id}: {directory / 'job.json'}", file=sys.stderr) diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py index c35b236bd..ec6ce6000 100644 --- a/tools/qa-resource/test_qa_resource.py +++ b/tools/qa-resource/test_qa_resource.py @@ -665,6 +665,34 @@ def interrupted_spawn(argv, *args, **kwargs): self.assertFalse(qa.group_has_members(job["childPgid"])) self.assert_lock_free() + def test_cancel_during_runner_startup_is_queued_not_refused(self): + # Delay the detached runner before it starts its listener. The job's creator already + # holds the FIFO's read end, so the cancel is accepted and delivered once it starts. + bootstrap = BOOTSTRAP.split("sys.exit")[0] + ''' +spawn = q.subprocess.Popen +def slow_runner(args, *positional, **keywords): + if isinstance(args, list) and len(args) > 2 and 'q.resume(' in str(args[2]): + args = list(args) + args[2] = 'import time; time.sleep(1.5); ' + args[2] + return spawn(args, *positional, **keywords) +q.subprocess.Popen = slow_runner +sys.exit(q.main(sys.argv[2:])) +''' + args = self.job_args("startup-cancel", "import time; time.sleep(30)", timeout=40) + args[0] = "submit" + submitter = self.launch(args, bootstrap) + output, error = submitter.communicate(timeout=8) + self.assertEqual(submitter.returncode, 0, error) + self.assertIsNone(self.job("startup-cancel")["runnerPid"]) + cancel = self.invoke(self.cli("cancel", "startup-cancel")) + self.assertEqual(cancel.returncode, 0, cancel.stderr) + result = self.invoke(self.cli("wait", "startup-cancel", ["--max-wait", "8"])) + self.assertEqual(result.returncode, 130, result.stderr) + job = json.loads(result.stdout) + self.assertEqual(job["status"], "cancelled") + self.assertIsNone(job["childPgid"]) + self.assert_lock_free() + def test_submit_hangup_during_spawn_with_ignored_sigchld_cancels_and_reports(self): bootstrap = BOOTSTRAP.split("sys.exit")[0] + ''' q.signal.signal(q.signal.SIGCHLD, q.signal.SIG_IGN) From 2085a61b41dc1e9f3577d665d770753be769f1bd Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Thu, 1 Oct 2026 17:32:54 -0500 Subject: [PATCH 07/15] tools: record a terminal error when queue job setup fails If creating the job's log files, making cancel.fifo, or opening its read end failed after the job directory existed, `run` or `submit` exited with a traceback and left the job `queued`. With no runner, `wait` reported it stale, and its id could not be reused without manual cleanup. These failures now write status `error` with the OS error and a finish time, release the liveness lock, and return 125, so `wait` reports the failure. Co-Authored-By: Claude Opus 5.5 (1M context) --- tools/qa-resource/qa_resource.py | 39 ++++++++++++++++++++++----- tools/qa-resource/test_qa_resource.py | 21 +++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/tools/qa-resource/qa_resource.py b/tools/qa-resource/qa_resource.py index b50982551..b30cdd7a2 100644 --- a/tools/qa-resource/qa_resource.py +++ b/tools/qa-resource/qa_resource.py @@ -748,13 +748,34 @@ def create_job(args): ownerToken=secrets.token_hex(16), escapedProcesses=[], cancelChannel=CANCEL_FIFO, ignoredProcesses=[], logs={name: str(directory / (name + ".log")) for name in ("output", "runner")}) - (directory / "output.log").touch() - (directory / "runner.log").touch() - os.mkfifo(directory / CANCEL_FIFO, 0o600) write_job(directory, job) + try: + (directory / "output.log").touch() + (directory / "runner.log").touch() + os.mkfifo(directory / CANCEL_FIFO, 0o600) + except OSError as error: + raise fail_unstarted_job(directory, live, job, f"cannot set up job files: {error}") from error return directory, live, job +class JobSetupError(RuntimeError): + pass + + +def fail_unstarted_job(directory, live, job, message): + """Record a terminal error for a job whose runner never started, then drop its lock. + + Without this the job would stay `queued` with no runner, `wait` would report it stale, + and its id could not be reused. + """ + job.update(status="error", exitCode=125, error=message, finishedAt=timestamp()) + try: + write_job(directory, job) + finally: + live.close() + return JobSetupError(f"job {job['jobId']}: {message}") + + class CancelListener: """Turn a byte on the job's FIFO into SIGTERM for this runner's main thread. @@ -1316,11 +1337,17 @@ def globals_for(target, suppress=False): parser.error("argv is required after --") try: directory, live, job = create_job(args) + # Hold the cancel FIFO's read end from job creation, so a cancel sent before the + # runner's listener starts is queued in the pipe rather than refused. + try: + cancel_fd = open_cancel_reader(directory) + except OSError as error: + raise fail_unstarted_job(directory, live, job, f"cannot open the cancel FIFO: {error}") from error except ValueError as error: parser.error(str(error)) - # Hold the cancel FIFO's read end from job creation, so a cancel sent before the - # runner's listener starts is queued in the pipe rather than refused. - cancel_fd = open_cancel_reader(directory) + except JobSetupError as error: + print(str(error), file=sys.stderr) + return 125 if args.command == "submit": return submit(directory, live, job, args.lock, cancel_fd) return run_job(directory, live, job, args.lock, cancel_fd=cancel_fd) diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py index ec6ce6000..0615d2bfd 100644 --- a/tools/qa-resource/test_qa_resource.py +++ b/tools/qa-resource/test_qa_resource.py @@ -693,6 +693,27 @@ def slow_runner(args, *positional, **keywords): self.assertIsNone(job["childPgid"]) self.assert_lock_free() + def test_setup_failure_after_job_creation_records_terminal_error(self): + failures = { + "fifo-open": "q.open_cancel_reader = lambda directory: (_ for _ in ()).throw(OSError(24, 'Too many open files'))", + "fifo-make": "q.os.mkfifo = lambda *args, **kwargs: (_ for _ in ()).throw(OSError(28, 'No space left on device'))", + } + for name, patch_line in failures.items(): + with self.subTest(name=name): + bootstrap = BOOTSTRAP.split("sys.exit")[0] + patch_line + "\nsys.exit(q.main(sys.argv[2:]))\n" + runner = self.launch(self.job_args(name), bootstrap) + output, error = runner.communicate(timeout=8) + self.assertEqual(runner.returncode, 125, error) + job = self.job(name) + self.assertEqual(job["status"], "error") + self.assertIsNotNone(job["finishedAt"]) + self.assertIn("Errno", job["error"]) + # The job is terminal, not stale: wait reports the error instead of exit 4. + result = self.invoke(self.cli("wait", name, ["--max-wait", "2"])) + self.assertEqual(result.returncode, 125, result.stderr) + self.assertIsNone(job["childPgid"]) + self.assert_lock_free() + def test_submit_hangup_during_spawn_with_ignored_sigchld_cancels_and_reports(self): bootstrap = BOOTSTRAP.split("sys.exit")[0] + ''' q.signal.signal(q.signal.SIGCHLD, q.signal.SIG_IGN) From f7d774a43308f855484670ef3d5804f817bca715 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Fri, 2 Oct 2026 04:46:42 -0500 Subject: [PATCH 08/15] docs(qa-resource): run the queue inside an outer command lock A shared Mac can make heavy commands also take an outer lock through `lockf`. Wrapping `submit` in it protects nothing: in a check with temporary locks, `lockf` released its lock when `submit` returned while the detached job kept running. Taking `lockf` inside the queue reverses the lock order. The README now says to wrap `run`, which holds both locks until cleanup ends, and to cancel through the queue. Co-Authored-By: Claude Code --- tools/qa-resource/README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tools/qa-resource/README.md b/tools/qa-resource/README.md index 554f5e240..e6604ad71 100644 --- a/tools/qa-resource/README.md +++ b/tools/qa-resource/README.md @@ -28,6 +28,16 @@ Admission requires memory pressure level 1 or 2, at least 35% memory free, and a Heavy-process exit waits use kqueue. Memory recovery, disk space, and device state have no stdlib kernel notification here, so they are resampled every 30 seconds. The global lock stays held during resource waits. The admission deadline covers queueing and resource checks. Simulator boot and bootstatus each have a fixed 300-second limit, an operational limit rather than a measured startup time. Cancellation waits for the boot result and ownership record. If boot times out, the runner reports the device state but never shuts it down without proof of ownership. The child timeout starts separately. +## Inside a machine-wide command lock + +Some machines also make every heavy command take an outer lock, such as `/usr/bin/lockf -k `. Take that lock first and run the queue's `run` inside it: + +```sh +/usr/bin/lockf -k python3 tools/qa-resource/qa_resource.py run --job-id JOB --kind package --cwd "$PWD" --admission-deadline 1800 --timeout 3600 -- +``` + +`lockf` holds its lock only until its own command exits. `run` stays in the foreground until cleanup ends, so both locks cover the whole job. `submit` returns as soon as the detached runner starts, so under `lockf` the job runs without the outer lock. Don't put `lockf` inside the queue (`... -- lockf `) either. That reverses the lock order, so a job waits on another lane that holds the outer lock while it waits for the queue. A simulator job would also boot its device before it holds the outer lock. Cancel with `cancel --job-id`, not by killing `lockf`. + ## Escaped descendants Each payload gets a random `QA_RESOURCE_OWNER_=` environment entry. At admission and cleanup, the runner reads process memory with Darwin `KERN_PROCARGS2`. An exact entry or live-parent ancestry proves ownership at discovery. The parser checks every NUL-separated string after the executable because empty `argv[0]` is indistinguishable from padding. Admission accepts only owner headers with a 32-character lowercase hexadecimal token. From 1e1f4d14cfe370ab73c6b387f09480b9990290e7 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 04:07:51 -0500 Subject: [PATCH 09/15] tools: say what an interrupted submit prints when stdout is blocked The README promised a cancelRequested receipt for any interrupted submit. When the interruption lands mid-write, stdout is what was blocking, and a second write would block again and leave the submitter unstoppable. Say so in the README and at the branch, and assert the blocked submitter exits 130. Co-Authored-By: Claude Opus 5.5 (1M context) --- tools/qa-resource/README.md | 2 +- tools/qa-resource/qa_resource.py | 3 +++ tools/qa-resource/test_qa_resource.py | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/qa-resource/README.md b/tools/qa-resource/README.md index e6604ad71..8107fa6dd 100644 --- a/tools/qa-resource/README.md +++ b/tools/qa-resource/README.md @@ -20,7 +20,7 @@ Read `~/.codex/qa-resource/jobs/ID/job.json` for status, reasons, timestamps, ow Nothing pushes a completion event to T3 or a manager session. A parent learns the result in one of two ways. It can block in `wait`, running it again with the same id if its shell tool's time limit is shorter than the job. Or it can start `run` as a background command and let its host report the exit. Either way, `job.json` holds the result. -`cancel` never signals a PID. Each runner reads a per-job FIFO, `cancel.fifo` in the job directory, and cancels itself when a byte arrives. The command that creates the job opens the read end before the runner starts and hands it to the runner, so a cancel sent during runner startup waits in the pipe and takes effect once the runner listens. When neither holds the read end, `cancel` cannot open the FIFO, reports that nothing was sent, and returns 4, so a reused runner PID can never receive the request. Jobs started by an older runner have no FIFO, and `cancel` refuses them. If `submit` is interrupted after starting the runner, it cancels that runner, which is still its own unreaped child, and prints the receipt with `cancelRequested: true` before returning 130, so `wait` can follow the job to `cancelled`. +`cancel` never signals a PID. Each runner reads a per-job FIFO, `cancel.fifo` in the job directory, and cancels itself when a byte arrives. The command that creates the job opens the read end before the runner starts and hands it to the runner, so a cancel sent during runner startup waits in the pipe and takes effect once the runner listens. When neither holds the read end, `cancel` cannot open the FIFO, reports that nothing was sent, and returns 4, so a reused runner PID can never receive the request. Jobs started by an older runner have no FIFO, and `cancel` refuses them. If `submit` is interrupted after starting the runner, it cancels that runner, which is still its own unreaped child, and prints the receipt with `cancelRequested: true` before returning 130, so `wait` can follow the job to `cancelled`. If the interruption arrives while the receipt is being written, stdout was blocked, usually by a pipe nobody reads. `submit` then still cancels the runner and returns 130, but prints no second receipt, because writing again would block again and leave it unstoppable. The caller supplied the job ID, so `wait ID` and `job.json` still report the outcome. Read `job.json`, not just the exit code. `run` and `wait` return the child's code for success or failure, 124 for child timeout, 130 for cancellation, 75 for admission timeout, and 125 for sensing or cleanup errors. `wait` returns 3 when its own deadline expires without affecting the job, 4 for a runner that died without a terminal result, and 2 for an unknown job. Child exit codes can overlap these values. If a child dies from signal N, `run` and `wait` return 128+N. `job.json` keeps the negative child return code in `exitCode` and N in `signal`. diff --git a/tools/qa-resource/qa_resource.py b/tools/qa-resource/qa_resource.py index b30cdd7a2..3bcd1a363 100644 --- a/tools/qa-resource/qa_resource.py +++ b/tools/qa-resource/qa_resource.py @@ -1163,6 +1163,9 @@ def interrupt(signum, frame): # Still our unreaped child (SIGCHLD is default and nothing here waits on # it), so this PID cannot belong to another process. os.kill(runner.pid, signal.SIGTERM) + # An interrupted write means stdout itself was blocked, usually a pipe + # nobody drains. Writing again would block again and make this submitter + # unstoppable, so only a receipt that never started is written here. if not printing: receipt["cancelRequested"] = True for sig, handler in previous_handlers.items(): diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py index 0615d2bfd..df64bae56 100644 --- a/tools/qa-resource/test_qa_resource.py +++ b/tools/qa-resource/test_qa_resource.py @@ -788,7 +788,8 @@ def test_blocked_submit_receipt_does_not_hold_finished_jobs_liveness(self): with (self.state / "jobs/blocked-receipt/live.lock").open("r") as live: fcntl.flock(live, fcntl.LOCK_SH | fcntl.LOCK_NB) submitter.terminate() - submitter.wait(timeout=3) + # The interrupted write must not be retried into the same full pipe. + self.assertEqual(submitter.wait(timeout=3), 130) finally: if writer is not None: os.close(writer) From ddd61e2d1c3aae999f139612cf11912e8d609699 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 04:20:08 -0500 Subject: [PATCH 10/15] tools: exempt the agent-device service only when it selects nothing else xcodebuild accepts repeated -only-testing. The membership check ignored a process that selected testCommand plus another suite, so package and render jobs could overlap that suite's heavy work. Require every selector to name the service. Co-Authored-By: Claude Opus 5.5 (1M context) --- tools/qa-resource/README.md | 2 +- tools/qa-resource/qa_resource.py | 3 ++- tools/qa-resource/test_qa_resource.py | 4 +++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tools/qa-resource/README.md b/tools/qa-resource/README.md index 8107fa6dd..d8078ed20 100644 --- a/tools/qa-resource/README.md +++ b/tools/qa-resource/README.md @@ -24,7 +24,7 @@ Nothing pushes a completion event to T3 or a manager session. A parent learns th Read `job.json`, not just the exit code. `run` and `wait` return the child's code for success or failure, 124 for child timeout, 130 for cancellation, 75 for admission timeout, and 125 for sensing or cleanup errors. `wait` returns 3 when its own deadline expires without affecting the job, 4 for a runner that died without a terminal result, and 2 for an unknown job. Child exit codes can overlap these values. If a child dies from signal N, `run` and `wait` return 128+N. `job.json` keeps the negative child return code in `exitCode` and N in `signal`. -Admission requires memory pressure level 1 or 2, at least 35% memory free, and at least 5 GiB free on the HOME volume. These are operational limits copied from `qa-slot.py`, not calibrated measurements. Unreadable resources fail admission. Heavy build and render processes block admission, except a verified agent-device `testCommand` service. That service blocks a simulator job only on the same device. The named simulator must exist and be Shutdown; other booted devices do not block. Package and render jobs never call `simctl`. +Admission requires memory pressure level 1 or 2, at least 35% memory free, and at least 5 GiB free on the HOME volume. These are operational limits copied from `qa-slot.py`, not calibrated measurements. Unreadable resources fail admission. Heavy build and render processes block admission, except a verified agent-device `testCommand` service, meaning an `xcodebuild test-without-building` whose every `-only-testing` selector names `testCommand`. That service blocks a simulator job only on the same device. The named simulator must exist and be Shutdown; other booted devices do not block. Package and render jobs never call `simctl`. Heavy-process exit waits use kqueue. Memory recovery, disk space, and device state have no stdlib kernel notification here, so they are resampled every 30 seconds. The global lock stays held during resource waits. The admission deadline covers queueing and resource checks. Simulator boot and bootstatus each have a fixed 300-second limit, an operational limit rather than a measured startup time. Cancellation waits for the boot result and ownership record. If boot times out, the runner reports the device state but never shuts it down without proof of ownership. The child timeout starts separately. diff --git a/tools/qa-resource/qa_resource.py b/tools/qa-resource/qa_resource.py index 3bcd1a363..9d0b54782 100644 --- a/tools/qa-resource/qa_resource.py +++ b/tools/qa-resource/qa_resource.py @@ -398,7 +398,8 @@ def classify_process(process, kind, device=None): return "blocker" service = (Path(process["comm"]).name == "xcodebuild" and "test-without-building" in tokens - and SERVICE_TEST in option_values(tokens, "-only-testing")) + # -only-testing may repeat. Any other selected suite does heavy work of its own. + and set(option_values(tokens, "-only-testing")) == {SERVICE_TEST}) if not service: return "blocker" destinations = [] diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py index df64bae56..69a944afb 100644 --- a/tools/qa-resource/test_qa_resource.py +++ b/tools/qa-resource/test_qa_resource.py @@ -1298,7 +1298,9 @@ def test_agent_device_classification_both_only_testing_forms(self): def test_xcodebuild_without_all_markers_blocks(self): for args in ("xcodebuild", f"xcodebuild -only-testing {qa.SERVICE_TEST}", "xcodebuild test-without-building -only-testing WrongTest", - f"xcodebuild test-without-building echo {qa.SERVICE_TEST}"): + f"xcodebuild test-without-building echo {qa.SERVICE_TEST}", + f"xcodebuild test-without-building -only-testing {qa.SERVICE_TEST} -only-testing OtherUITests", + f"xcodebuild test-without-building -only-testing:OtherUITests -only-testing:{qa.SERVICE_TEST}"): self.assertEqual(qa.classify_process(dict(comm="xcodebuild", args=args), "package"), "blocker") self.assertEqual(qa.classify_process(dict(comm="swift-build", args="", stat="Z"), "package"), "irrelevant") self.assertEqual(qa.classify_process(dict(comm="/Applications/T3 Code App", args="blender"), "package"), "irrelevant") From 9ef2757dcdf955ea4a08c9e975a85d0a21055741 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 04:36:11 -0500 Subject: [PATCH 11/15] tools: close four qa-resource admission and setup gaps - Exempt the agent-device service only when test-without-building is its only action; -only-testing does not constrain build actions. - In simulator mode, ignore the service only when explicit destination ids place it on other devices. A name, OS or implicit destination may resolve to the job's device while it is still Shutdown. - Record a submit interrupted while waiting for its queue ticket as cancelled, instead of leaving it queued with no runner and stale. - Remove the new job directory when its first record write fails, so the id stays reusable. Co-Authored-By: Claude Opus 5.5 (1M context) --- tools/qa-resource/README.md | 2 +- tools/qa-resource/qa_resource.py | 56 +++++++++++++++++++++------ tools/qa-resource/test_qa_resource.py | 55 +++++++++++++++++++++++++- 3 files changed, 100 insertions(+), 13 deletions(-) diff --git a/tools/qa-resource/README.md b/tools/qa-resource/README.md index d8078ed20..3ecb54e8c 100644 --- a/tools/qa-resource/README.md +++ b/tools/qa-resource/README.md @@ -24,7 +24,7 @@ Nothing pushes a completion event to T3 or a manager session. A parent learns th Read `job.json`, not just the exit code. `run` and `wait` return the child's code for success or failure, 124 for child timeout, 130 for cancellation, 75 for admission timeout, and 125 for sensing or cleanup errors. `wait` returns 3 when its own deadline expires without affecting the job, 4 for a runner that died without a terminal result, and 2 for an unknown job. Child exit codes can overlap these values. If a child dies from signal N, `run` and `wait` return 128+N. `job.json` keeps the negative child return code in `exitCode` and N in `signal`. -Admission requires memory pressure level 1 or 2, at least 35% memory free, and at least 5 GiB free on the HOME volume. These are operational limits copied from `qa-slot.py`, not calibrated measurements. Unreadable resources fail admission. Heavy build and render processes block admission, except a verified agent-device `testCommand` service, meaning an `xcodebuild test-without-building` whose every `-only-testing` selector names `testCommand`. That service blocks a simulator job only on the same device. The named simulator must exist and be Shutdown; other booted devices do not block. Package and render jobs never call `simctl`. +Admission requires memory pressure level 1 or 2, at least 35% memory free, and at least 5 GiB free on the HOME volume. These are operational limits copied from `qa-slot.py`, not calibrated measurements. Unreadable resources fail admission. Heavy build and render processes block admission, except a verified agent-device `testCommand` service, meaning an `xcodebuild test-without-building` with no other action whose every `-only-testing` selector names `testCommand`. That service blocks a simulator job unless every one of its `-destination` values names a different device by `id=`, because a name, an OS or an implicit destination may resolve to the job's device. The named simulator must exist and be Shutdown; other booted devices do not block. Package and render jobs never call `simctl`. Heavy-process exit waits use kqueue. Memory recovery, disk space, and device state have no stdlib kernel notification here, so they are resampled every 30 seconds. The global lock stays held during resource waits. The admission deadline covers queueing and resource checks. Simulator boot and bootstatus each have a fixed 300-second limit, an operational limit rather than a measured startup time. Cancellation waits for the boot result and ownership record. If boot times out, the runner reports the device state but never shuts it down without proof of ownership. The child timeout starts separately. diff --git a/tools/qa-resource/qa_resource.py b/tools/qa-resource/qa_resource.py index 9d0b54782..cf691528c 100644 --- a/tools/qa-resource/qa_resource.py +++ b/tools/qa-resource/qa_resource.py @@ -44,6 +44,9 @@ HEAVY_NAMES = {"xcodebuild", "swift-build", "swift-test", "swift-frontend", "swift-driver", "blender"} SERVICE_TEST = "AgentDeviceRunnerUITests/RunnerTests/testCommand" +# Every xcodebuild action except test-without-building, which the service runs. +XCODEBUILD_BUILD_ACTIONS = frozenset({"build", "build-for-testing", "analyze", "archive", "test", + "installsrc", "install", "clean", "docbuild"}) CANCEL_FIFO = "cancel.fifo" STOP_LISTENER = b"q" TERMINAL = {"succeeded", "failed", "timed_out", "cancelled", "admission_timeout", @@ -398,10 +401,15 @@ def classify_process(process, kind, device=None): return "blocker" service = (Path(process["comm"]).name == "xcodebuild" and "test-without-building" in tokens + # xcodebuild accepts several actions, and -only-testing constrains only test + # actions. A value spelled like an action only makes this more conservative. + and not XCODEBUILD_BUILD_ACTIONS.intersection(tokens) # -only-testing may repeat. Any other selected suite does heavy work of its own. and set(option_values(tokens, "-only-testing")) == {SERVICE_TEST}) if not service: return "blocker" + if kind != "simulator": + return "ignored" destinations = [] for index, token in enumerate(tokens): if token == "-destination": @@ -411,9 +419,11 @@ def classify_process(process, kind, device=None): while end < len(tokens) and not tokens[end].startswith("-"): end += 1 destinations.append(" ".join(tokens[index + 1:end])) - matches = any(re.search(r"(?:^|,)\s*id=" + re.escape(device or "") + r"(?:,|$)", d) - for d in destinations) - return "blocker" if kind == "simulator" and matches else "ignored" + # Only explicit ids prove the service is on another device. A name, an OS or an implicit + # destination may resolve to this one while it is still Shutdown. + ids = [re.search(r"(?:^|,)\s*id=([^,\s]+)", d) for d in destinations] + elsewhere = bool(ids) and all(match and match.group(1) != device for match in ids) + return "ignored" if elsewhere else "blocker" def resource_blockers(pressure, memory_percent, free_bytes): @@ -749,7 +759,18 @@ def create_job(args): ownerToken=secrets.token_hex(16), escapedProcesses=[], cancelChannel=CANCEL_FIFO, ignoredProcesses=[], logs={name: str(directory / (name + ".log")) for name in ("output", "runner")}) - write_job(directory, job) + try: + write_job(directory, job) + except OSError as error: + # No record exists to carry an error, and the directory alone would reject every retry + # of this id as a duplicate. This call created it, so removing it touches nothing else. + live.close() + leftovers = [] + for path in (directory / "job.json.tmp", directory / "job.json", directory / "live.lock", directory): + cleanup_step(leftovers, f"remove {path}", + lambda path=path: path.rmdir() if path == directory else path.unlink(missing_ok=True)) + detail = f"; {'; '.join(leftovers)}" if leftovers else "" + raise JobSetupError(f"job {args.job_id}: cannot write the job record: {error}{detail}") from error try: (directory / "output.log").touch() (directory / "runner.log").touch() @@ -1110,6 +1131,14 @@ def submit(directory, live, job, lock_path, cancel_fd): ticket_path = ticket = runner = None close_errors = [] deadline = time.monotonic() + job["admissionDeadline"] + def interrupt(signum, frame): + # One-shot, so a second signal cannot escape the cancellation below. + for sig in RUNNER_SIGNALS: + signal.signal(sig, signal.SIG_IGN) + raise Cancelled(signum) + # Until the runner exists nothing else will finish this job's record, so an interruption + # while waiting for the ticket must record it as cancelled rather than leave it queued. + original_handlers = {sig: signal.signal(sig, interrupt) for sig in RUNNER_SIGNALS} try: ticket_path, ticket = acquire_ticket(directory.parent.parent, job["jobId"], deadline) # Keep SIGCHLD at its default so an exited runner stays an unreaped zombie and its PID @@ -1121,12 +1150,10 @@ def submit(directory, live, job, lock_path, cancel_fd): def record(signum, frame): if not pending: pending.append(signum) - def interrupt(signum, frame): - # One-shot, so a second signal cannot escape the cancellation below. - for sig in RUNNER_SIGNALS: - signal.signal(sig, signal.SIG_IGN) - raise Cancelled(signum) - previous_handlers = {sig: signal.signal(sig, record) for sig in RUNNER_SIGNALS} + # Replace interrupt directly, leaving no gap where the default action could kill us. + for sig in RUNNER_SIGNALS: + signal.signal(sig, record) + previous_handlers = original_handlers try: with (directory / "runner.log").open("a") as log: # The runner inherits this mask and unblocks after installing its handlers, @@ -1180,7 +1207,11 @@ def interrupt(signum, frame): raise RuntimeError("; ".join(close_errors)) finally: signal.signal(signal.SIGCHLD, previous_sigchld) - except Cancelled: + except Cancelled as error: + if runner is None: + job.update(status="cancelled", exitCode=130, signal=error.signum, + error="submit was interrupted before its runner started", finishedAt=timestamp()) + write_job(directory, job) return 130 except Exception as error: if runner is not None: @@ -1203,6 +1234,9 @@ def interrupt(signum, frame): if runner is None and close_errors: job.update(status="cleanup_failed", exitCode=125, cleanupError="; ".join(close_errors)) cleanup_step(close_errors, "write submission cleanup errors", lambda: write_job(directory, job)) + # The inner block restores these once it starts. This covers an exit before it. + for sig, handler in original_handlers.items(): + signal.signal(sig, handler) return 0 diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py index 69a944afb..126f2936a 100644 --- a/tools/qa-resource/test_qa_resource.py +++ b/tools/qa-resource/test_qa_resource.py @@ -1295,12 +1295,26 @@ def test_agent_device_classification_both_only_testing_forms(self): self.assertEqual(qa.classify_process(process, "simulator", "A4EE0B20-1654-4B67-B54E-D071DA1E9601"), "blocker") self.assertEqual(qa.classify_process(process, "simulator", "other"), "ignored") + def test_service_without_explicit_ids_blocks_every_simulator(self): + service = f"xcodebuild test-without-building -only-testing:{qa.SERVICE_TEST}" + for destination in ("", " -destination platform=iOS Simulator,name=iPhone 16,OS=18.0", + " -destination platform=iOS Simulator,id=other -destination platform=iOS Simulator,name=iPhone 16"): + process = dict(comm="xcodebuild", args=service + destination) + with self.subTest(destination=destination): + self.assertEqual(qa.classify_process(process, "simulator", "ours"), "blocker") + self.assertEqual(qa.classify_process(process, "package"), "ignored") + process = dict(comm="xcodebuild", args=service + " -destination platform=iOS Simulator,id=other" + " -destination platform=iOS Simulator,id=third") + self.assertEqual(qa.classify_process(process, "simulator", "ours"), "ignored") + def test_xcodebuild_without_all_markers_blocks(self): for args in ("xcodebuild", f"xcodebuild -only-testing {qa.SERVICE_TEST}", "xcodebuild test-without-building -only-testing WrongTest", f"xcodebuild test-without-building echo {qa.SERVICE_TEST}", f"xcodebuild test-without-building -only-testing {qa.SERVICE_TEST} -only-testing OtherUITests", - f"xcodebuild test-without-building -only-testing:OtherUITests -only-testing:{qa.SERVICE_TEST}"): + f"xcodebuild test-without-building -only-testing:OtherUITests -only-testing:{qa.SERVICE_TEST}", + f"xcodebuild build test-without-building -only-testing:{qa.SERVICE_TEST}", + f"xcodebuild test-without-building build-for-testing -only-testing:{qa.SERVICE_TEST}"): self.assertEqual(qa.classify_process(dict(comm="xcodebuild", args=args), "package"), "blocker") self.assertEqual(qa.classify_process(dict(comm="swift-build", args="", stat="Z"), "package"), "irrelevant") self.assertEqual(qa.classify_process(dict(comm="/Applications/T3 Code App", args="blender"), "package"), "irrelevant") @@ -1435,6 +1449,45 @@ def test_release_ticket_is_idempotent_and_records_unlink_failure(self): self.assertEqual(qa.release_ticket(path, None), []) self.assertFalse(path.exists()) + def test_failed_first_record_write_leaves_the_id_reusable(self): + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + args = Mock(state_dir=root / "state", job_id="no-space", kind="package", argv=["unused"], + cwd=root, device=None, admission_deadline=3, timeout=1) + def full_disk(directory, job): + (directory / "job.json.tmp").write_text("partial") + raise OSError(28, "No space left on device") + with patch.object(qa, "write_job", side_effect=full_disk): + with self.assertRaisesRegex(qa.JobSetupError, "cannot write the job record: .*No space left"): + qa.create_job(args) + self.assertFalse((root / "state/jobs/no-space").exists()) + directory, live, job = qa.create_job(args) + live.close() + self.assertEqual(qa.read_job(directory)["status"], "queued") + + def test_submit_interrupted_while_waiting_for_its_ticket_records_cancelled(self): + for sig in qa.RUNNER_SIGNALS: + with self.subTest(signal=sig), tempfile.TemporaryDirectory() as temp: + root = Path(temp) + args = Mock(state_dir=root / "state", job_id="interrupted", kind="package", argv=["unused"], + cwd=root, device=None, admission_deadline=3, timeout=1) + directory, live, job = qa.create_job(args) + before = {s: signal.getsignal(s) for s in qa.RUNNER_SIGNALS} + def waiting(*unused): + # Without submit's handler this signal would kill the test runner itself. + self.assertNotIn(signal.getsignal(sig), (signal.SIG_DFL, signal.default_int_handler)) + os.kill(os.getpid(), sig) + self.fail("the signal did not interrupt the ticket wait") + with patch.object(qa, "acquire_ticket", side_effect=waiting), \ + patch.object(qa.subprocess, "Popen") as spawn: + code = qa.submit(directory, live, job, root / "lock", qa.open_cancel_reader(directory)) + self.assertEqual(code, 130) + spawn.assert_not_called() + result = qa.read_job(directory) + self.assertEqual((result["status"], result["signal"]), ("cancelled", sig)) + self.assertEqual({s: signal.getsignal(s) for s in qa.RUNNER_SIGNALS}, before) + self.assertFalse(qa.live_report(directory)["live"]) + def test_cleanup_hold_keeps_lock_until_group_gone_or_deadline(self): for members_remain in (False, True): with self.subTest(members_remain=members_remain), tempfile.TemporaryDirectory() as temp: From 955ac918429edc11479cb949f0317e742bdedb15 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 05:02:07 -0500 Subject: [PATCH 12/15] tools: prove qa-resource ownership from the kernel and close lifecycle gaps - Ancestry proof now comes from the kernel's current parent chain of the pinned process. A ps snapshot only nominates descendants, so a PID reused in between can no longer be recorded as owned and signalled. kinfo_proc supplies each link's parent and start time together, and a parent that started after its child fails the proof. - run_job resets an inherited SIGCHLD disposition while it owns the payload. Under SIG_IGN, Darwin reaped the payload, Popen mapped ECHILD to 0 and a failure was recorded as success; the PID and group were also unpinned. - main blocks SIGTERM, SIGINT and SIGHUP from job creation until run_job or submit installs handlers, so an interruption there is recorded as cancelled. - live.lock creation joins the setup rollback, keeping the id reusable. Co-Authored-By: Claude Opus 5.5 (1M context) --- tools/qa-resource/README.md | 4 +- tools/qa-resource/qa_resource.py | 83 ++++++++++++++++++------ tools/qa-resource/test_qa_resource.py | 92 +++++++++++++++++++++++++-- 3 files changed, 152 insertions(+), 27 deletions(-) diff --git a/tools/qa-resource/README.md b/tools/qa-resource/README.md index 3ecb54e8c..de26ab310 100644 --- a/tools/qa-resource/README.md +++ b/tools/qa-resource/README.md @@ -40,7 +40,7 @@ Some machines also make every heavy command take an outer lock, such as `/usr/bi ## Escaped descendants -Each payload gets a random `QA_RESOURCE_OWNER_=` environment entry. At admission and cleanup, the runner reads process memory with Darwin `KERN_PROCARGS2`. An exact entry or live-parent ancestry proves ownership at discovery. The parser checks every NUL-separated string after the executable because empty `argv[0]` is indistinguishable from padding. Admission accepts only owner headers with a 32-character lowercase hexadecimal token. +Each payload gets a random `QA_RESOURCE_OWNER_=` environment entry. At admission and cleanup, the runner reads process memory with Darwin `KERN_PROCARGS2`. An exact entry or live-parent ancestry proves ownership at discovery. A `ps` snapshot only nominates descendants, because its PIDs may have been reused since. Ancestry counts only when the kernel's current parent chain, read from the pinned process up to the payload, holds at every link, and each parent started no later than its child. Otherwise the process is not treated as owned. The runner resets an inherited `SIGCHLD` disposition to the default while it owns the payload, so the payload stays an unreaped zombie that keeps its exit status and pins its PID and process-group id. The parser checks every NUL-separated string after the executable because empty `argv[0]` is indistinguishable from padding. Admission accepts only owner headers with a 32-character lowercase hexadecimal token. On cancellation, timeout, and normal exit, the runner signals discovered escapes before its pinned child group. Discovery proof stays attached to PID, microsecond start time, and uid even after reparenting or token removal. Each per-PID signal rechecks that identity with `KERN_PROC_PID` and excludes zombies and other uids. The runner never signals an escaped process's new group. The leader stays unreaped until checks finish. @@ -58,7 +58,7 @@ Invisible descendants are not stopped. Darwin omits the environment of `cs_restr A chain whose processes each live shorter than one scan interval can evade two-pass confirmation. Microsecond start times avoid coarse `lstart` identity checks for escaped PIDs, but the check-to-signal PID reuse window remains. -The queue cannot identify a foreign shared-lock holder or attribute a heavy process to a project. FIFO ordering applies only to this tool's tickets; `qa-slot.py` and `package-run.py` still race for the shared flock. Cancellation stops only the job's own child group and proven escaped descendants, and shuts down only the device the job booted. The runner handles SIGTERM, SIGINT, and SIGHUP; later signals cannot interrupt cleanup. A stale job's recorded group and booted-device ownership are reported, never cleaned. Inspect those manually before reusing a device. +The queue cannot identify a foreign shared-lock holder or attribute a heavy process to a project. FIFO ordering applies only to this tool's tickets; `qa-slot.py` and `package-run.py` still race for the shared flock. Cancellation stops only the job's own child group and proven escaped descendants, and shuts down only the device the job booted. The runner handles SIGTERM, SIGINT, and SIGHUP; later signals cannot interrupt cleanup. `run` and `submit` hold those signals from job creation until their handlers are installed, so an interruption at any point after the job exists is recorded as `cancelled`. If the job directory or its first record cannot be created, the directory is removed and the id stays reusable. A stale job's recorded group and booted-device ownership are reported, never cleaned. Inspect those manually before reusing a device. Boot ownership does not identify a simulator session; if another lane shuts down and re-boots the same UDID mid-job, final shutdown affects their session. diff --git a/tools/qa-resource/qa_resource.py b/tools/qa-resource/qa_resource.py index cf691528c..1333b7091 100644 --- a/tools/qa-resource/qa_resource.py +++ b/tools/qa-resource/qa_resource.py @@ -185,7 +185,7 @@ class ProcessTimeval(ctypes.Structure): def kernel_identity(pid): # 64-bit Darwin SDK sizeof/offsetof: kinfo_proc=648, start=0, stat=36, - # pid=40, kp_eproc.e_ucred.cr_uid=420. Fail closed if the runtime ABI differs. + # pid=40, kp_eproc.e_ucred.cr_uid=420, kp_eproc.e_ppid=560. Fail closed if the runtime ABI differs. if ctypes.sizeof(ctypes.c_void_p) != 8 or ctypes.sizeof(ProcessTimeval) != 16: raise RuntimeError("process identity requires the 64-bit Darwin kinfo_proc ABI") buffer = ctypes.create_string_buffer(648) @@ -207,9 +207,32 @@ def kernel_identity(pid): raise RuntimeError(f"invalid kinfo_proc start time for PID {pid}") return dict(pid=pid, start_us=start.seconds * 1_000_000 + start.microseconds, uid=ctypes.c_uint.from_buffer(buffer, 420).value, + ppid=ctypes.c_int.from_buffer(buffer, 560).value, zombie=ctypes.c_byte.from_buffer(buffer, 36).value == 5) +def kernel_ancestry(identity, child_pid, deadline): + """Whether the kernel's current parent chain leads from this pinned execution to child_pid. + + A ps snapshot is not atomic, and any PID in it may have been reused since. Each link here + comes from one kinfo_proc read, which reports a process's start time and parent together. + A true parent started no later than its child. A parent PID naming a later process, or one + that is gone, means the recorded link no longer holds, so the proof fails closed. + """ + current, seen = identity, set() + while current["pid"] != child_pid: + inspection_timeout(deadline) + parent_pid = current["ppid"] + if parent_pid <= 1 or parent_pid in seen: + return False + seen.add(parent_pid) + parent = kernel_identity(parent_pid) + if parent is None or parent["start_us"] > current["start_us"]: + return False + current = parent + return True + + def process_snapshot(deadline=None): rows = command_output(["ps", "-axo", "pid=,ppid=,pgid=,uid=,stat="], timeout=inspection_timeout(deadline)) @@ -293,6 +316,11 @@ def owned_processes(job, child_pid, deadline=None, known=None, observed=None): continue # Enumeration-to-env disappearance is settled-gone. # Restricted or unreadable environments alone are not proof. inspection_timeout(deadline) + if ancestry: + # The snapshot only nominated this PID. Prove the pinned execution itself + # descends from the payload; the start-time check below then ties this + # proof to the identity that gets recorded. + ancestry = kernel_ancestry(before, child_pid, deadline) if not token and not ancestry: continue if observed is not None: @@ -748,8 +776,6 @@ def create_job(args): directory.mkdir() except FileExistsError as error: raise ValueError(f"job {args.job_id} already exists; choose a new id") from error - live = (directory / "live.lock").open("a+") - fcntl.flock(live, fcntl.LOCK_EX | fcntl.LOCK_NB) job = dict(jobId=args.job_id, kind=args.kind, argv=args.argv, cwd=str(args.cwd), device=args.device, status="queued", reasons=[], runnerPid=None, runnerStart=None, childPgid=None, bootedByJob=False, exitCode=None, @@ -759,18 +785,22 @@ def create_job(args): ownerToken=secrets.token_hex(16), escapedProcesses=[], cancelChannel=CANCEL_FIFO, ignoredProcesses=[], logs={name: str(directory / (name + ".log")) for name in ("output", "runner")}) + live = None try: + live = (directory / "live.lock").open("a+") + fcntl.flock(live, fcntl.LOCK_EX | fcntl.LOCK_NB) write_job(directory, job) except OSError as error: # No record exists to carry an error, and the directory alone would reject every retry # of this id as a duplicate. This call created it, so removing it touches nothing else. - live.close() leftovers = [] + if live is not None: + cleanup_step(leftovers, "close live lock", live.close) for path in (directory / "job.json.tmp", directory / "job.json", directory / "live.lock", directory): cleanup_step(leftovers, f"remove {path}", lambda path=path: path.rmdir() if path == directory else path.unlink(missing_ok=True)) detail = f"; {'; '.join(leftovers)}" if leftovers else "" - raise JobSetupError(f"job {args.job_id}: cannot write the job record: {error}{detail}") from error + raise JobSetupError(f"job {args.job_id}: cannot create the job record: {error}{detail}") from error try: (directory / "output.log").touch() (directory / "runner.log").touch() @@ -924,6 +954,11 @@ def interrupted(signum, frame): signal.signal(sig, signal.SIG_IGN) raise Cancelled(signum) old_signals = {sig: signal.signal(sig, interrupted) for sig in RUNNER_SIGNALS} + # An inherited SIG_IGN makes Darwin reap the payload at exit. Its status is then lost + # (Popen maps ECHILD to 0, recording a failure as success), and its PID and group id are + # no longer pinned for cleanup. Kept apart from old_signals, which cleanup sets to SIG_IGN, + # and restored only after cleanup has finished with the payload. + old_sigchld = signal.signal(signal.SIGCHLD, signal.SIG_DFL) listener = None try: # submit starts the runner with these blocked; a cancel that arrived meanwhile is @@ -1108,6 +1143,7 @@ def mark_cleanup_errors(): for sig, previous in old_signals.items(): cleanup_step(cleanup_errors, f"restore signal {sig}", lambda sig=sig, previous=previous: signal.signal(sig, previous)) + cleanup_step(cleanup_errors, "restore SIGCHLD", lambda: signal.signal(signal.SIGCHLD, old_sigchld)) if len(cleanup_errors) != recorded_errors: mark_cleanup_errors() cleanup_step(cleanup_errors, "write finalizer errors", lambda: write_job(directory, job)) @@ -1140,6 +1176,8 @@ def interrupt(signum, frame): # while waiting for the ticket must record it as cancelled rather than leave it queued. original_handlers = {sig: signal.signal(sig, interrupt) for sig in RUNNER_SIGNALS} try: + # main blocks these through job creation. A signal held since then raises here. + signal.pthread_sigmask(signal.SIG_UNBLOCK, RUNNER_SIGNALS) ticket_path, ticket = acquire_ticket(directory.parent.parent, job["jobId"], deadline) # Keep SIGCHLD at its default so an exited runner stays an unreaped zombie and its PID # cannot be reused before an interrupted submit signals it. An inherited SIG_IGN @@ -1373,22 +1411,29 @@ def globals_for(target, suppress=False): args.argv = args.argv[1:] if args.argv[:1] == ["--"] else args.argv if not args.argv: parser.error("argv is required after --") + # From job creation until run_job or submit installs its handlers, nothing would record + # an interruption, so these signals stay pending. Both unblock them inside their own + # try, where a pending one becomes a recorded cancellation. + previous_mask = signal.pthread_sigmask(signal.SIG_BLOCK, RUNNER_SIGNALS) try: - directory, live, job = create_job(args) - # Hold the cancel FIFO's read end from job creation, so a cancel sent before the - # runner's listener starts is queued in the pipe rather than refused. try: - cancel_fd = open_cancel_reader(directory) - except OSError as error: - raise fail_unstarted_job(directory, live, job, f"cannot open the cancel FIFO: {error}") from error - except ValueError as error: - parser.error(str(error)) - except JobSetupError as error: - print(str(error), file=sys.stderr) - return 125 - if args.command == "submit": - return submit(directory, live, job, args.lock, cancel_fd) - return run_job(directory, live, job, args.lock, cancel_fd=cancel_fd) + directory, live, job = create_job(args) + # Hold the cancel FIFO's read end from job creation, so a cancel sent before the + # runner's listener starts is queued in the pipe rather than refused. + try: + cancel_fd = open_cancel_reader(directory) + except OSError as error: + raise fail_unstarted_job(directory, live, job, f"cannot open the cancel FIFO: {error}") from error + except ValueError as error: + parser.error(str(error)) + except JobSetupError as error: + print(str(error), file=sys.stderr) + return 125 + if args.command == "submit": + return submit(directory, live, job, args.lock, cancel_fd) + return run_job(directory, live, job, args.lock, cancel_fd=cancel_fd) + finally: + signal.pthread_sigmask(signal.SIG_SETMASK, previous_mask) directory = args.state_dir / "jobs" / (args.job_id or "") if args.job_id and not (directory / "job.json").is_file(): print(f"unknown job {args.job_id}: {directory / 'job.json'}", file=sys.stderr) diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py index 126f2936a..d6b0d8e8c 100644 --- a/tools/qa-resource/test_qa_resource.py +++ b/tools/qa-resource/test_qa_resource.py @@ -795,6 +795,14 @@ def test_blocked_submit_receipt_does_not_hold_finished_jobs_liveness(self): os.close(writer) os.close(reader) + def test_run_inheriting_ignored_sigchld_still_records_payload_failure(self): + # Under SIG_IGN, Darwin reaps the payload at exit and Popen reports ECHILD as status 0. + ignoring = "import signal; signal.signal(signal.SIGCHLD, signal.SIG_IGN); " + BOOTSTRAP + runner = self.launch(self.job_args("ignored-sigchld", "import sys; sys.exit(3)"), bootstrap=ignoring) + result = self.finish(runner, "ignored-sigchld", 3) + self.assertEqual((result["status"], result["exitCode"]), ("failed", 3)) + self.assert_lock_free() + def test_timeout_kills_descendant_ignoring_sigterm_and_releases_lock(self): program = """import os, signal, time if os.fork() == 0: @@ -1096,7 +1104,8 @@ def test_foreign_uid_descendant_is_retained_without_environment_read_or_signal(s 124: dict(pid=124, ppid=123, pgid=124, uid=os.getuid() + 1, stat="S")} with patch.object(qa, "process_snapshot", return_value=rows), \ patch.object(qa, "process_env", return_value=[]) as env, \ - patch.object(qa, "kernel_identity", side_effect=lambda pid: dict(pid=pid, start_us=pid * 1_000_000, uid=rows[pid]["uid"], zombie=False)), \ + patch.object(qa, "kernel_identity", side_effect=lambda pid: dict(pid=pid, start_us=pid * 1_000_000, uid=rows[pid]["uid"], + ppid=rows[pid]["ppid"], zombie=False)), \ patch.object(qa, "process_identity", side_effect=lambda pid, **unused: dict(pid=pid, start="date", comm="python")): found = qa.owned_processes(job, 123) self.assertEqual([(p["pid"], p["proof"]) for p in found], [(123, "ancestry"), (124, "ancestry")]) @@ -1107,6 +1116,44 @@ def test_foreign_uid_descendant_is_retained_without_environment_read_or_signal(s env.assert_not_called() kill.assert_not_called() + def test_kernel_identity_reports_the_real_parent(self): + self.assertEqual(qa.kernel_identity(os.getpid())["ppid"], os.getppid()) + + def test_ancestry_is_proven_from_the_kernel_chain_of_the_pinned_execution(self): + # The snapshot nominates 124 and 125 as descendants of payload 123. Each schedule is + # what the kernel reports once the PID is pinned, after any reuse. + job = dict(jobId="job", ownerToken="a" * 32) + uid = os.getuid() + rows = {123: dict(pid=123, ppid=1, pgid=123, uid=uid, stat="S"), + 124: dict(pid=124, ppid=123, pgid=123, uid=uid, stat="S"), + 125: dict(pid=125, ppid=124, pgid=123, uid=uid, stat="S")} + def kernel(changes=None): + table = {pid: dict(pid=pid, start_us=pid * 1_000_000, uid=uid, ppid=row["ppid"], zombie=False) + for pid, row in rows.items()} + for pid, fields in (changes or {}).items(): + table[pid].update(fields) + return lambda pid: dict(table[pid]) if pid in table else None + schedules = { + "consistent": (kernel(), [123, 124, 125]), + # 124 exited and an unrelated process now holds its PID; 125 was reparented. + "reused leaf": (kernel({124: dict(ppid=1, start_us=900_000_000), 125: dict(ppid=1)}), [123]), + # 124's PID now names a process started after 125, so 125's recorded parent is gone. + "reused link": (kernel({124: dict(start_us=900_000_000)}), [123, 124]), + # The descendant's PID was reused by a process whose real parent is elsewhere. + "reused pid": (kernel({125: dict(ppid=77, start_us=901_000_000)}), [123, 124]), + } + for name, (identity, expected) in schedules.items(): + with self.subTest(schedule=name), \ + patch.object(qa, "process_snapshot", return_value=rows), \ + patch.object(qa, "process_env", return_value=[]), \ + patch.object(qa, "kernel_identity", side_effect=identity), \ + patch.object(qa, "process_identity", side_effect=lambda pid, **unused: dict(pid=pid, start="date", comm="python")), \ + patch.object(qa.os, "kill") as kill: + found = qa.owned_processes(job, 123) + self.assertEqual([p["pid"] for p in found], expected) + self.assertTrue(all(p["proof"] == "ancestry" for p in found)) + kill.assert_not_called() + def test_proven_identity_survives_lost_token_and_reparenting(self): identity = dict(pid=124, uid=os.getuid(), start_us=124_000_001, start="child-start", comm="python", proof="ancestry") @@ -1449,7 +1496,7 @@ def test_release_ticket_is_idempotent_and_records_unlink_failure(self): self.assertEqual(qa.release_ticket(path, None), []) self.assertFalse(path.exists()) - def test_failed_first_record_write_leaves_the_id_reusable(self): + def test_failed_job_record_setup_leaves_the_id_reusable(self): with tempfile.TemporaryDirectory() as temp: root = Path(temp) args = Mock(state_dir=root / "state", job_id="no-space", kind="package", argv=["unused"], @@ -1457,10 +1504,17 @@ def test_failed_first_record_write_leaves_the_id_reusable(self): def full_disk(directory, job): (directory / "job.json.tmp").write_text("partial") raise OSError(28, "No space left on device") - with patch.object(qa, "write_job", side_effect=full_disk): - with self.assertRaisesRegex(qa.JobSetupError, "cannot write the job record: .*No space left"): - qa.create_job(args) - self.assertFalse((root / "state/jobs/no-space").exists()) + original_open = Path.open + def full_disk_lock(path, *rest, **keywords): + if path.name == "live.lock": + raise OSError(28, "No space left on device") + return original_open(path, *rest, **keywords) + for name, failure in (("record", patch.object(qa, "write_job", side_effect=full_disk)), + ("live lock", patch.object(Path, "open", autospec=True, side_effect=full_disk_lock))): + with self.subTest(failure=name), failure: + with self.assertRaisesRegex(qa.JobSetupError, "cannot create the job record: .*No space left"): + qa.create_job(args) + self.assertFalse((root / "state/jobs/no-space").exists()) directory, live, job = qa.create_job(args) live.close() self.assertEqual(qa.read_job(directory)["status"], "queued") @@ -1488,6 +1542,32 @@ def waiting(*unused): self.assertEqual({s: signal.getsignal(s) for s in qa.RUNNER_SIGNALS}, before) self.assertFalse(qa.live_report(directory)["live"]) + def test_interruption_between_job_creation_and_handlers_is_recorded(self): + real_reader = qa.open_cancel_reader + for command in ("run", "submit"): + for sig in qa.RUNNER_SIGNALS: + with self.subTest(command=command, signal=sig), tempfile.TemporaryDirectory() as temp: + root = Path(temp) + def reader(directory): + # Unblocked, this signal would take the default action in the test runner. + self.assertIn(sig, signal.pthread_sigmask(signal.SIG_BLOCK, [])) + os.kill(os.getpid(), sig) + return real_reader(directory) + handlers = {s: signal.getsignal(s) for s in (*qa.RUNNER_SIGNALS, signal.SIGCHLD)} + mask = signal.pthread_sigmask(signal.SIG_BLOCK, []) + argv = [command, "--state-dir", str(root / "state"), "--lock", str(root / "lock"), + "--job-id", "handoff", "--kind", "package", "--cwd", str(root), + "--admission-deadline", "3", "--timeout", "1", "--", sys.executable, "-c", "pass"] + with patch.object(qa, "open_cancel_reader", side_effect=reader), \ + patch.object(qa.subprocess, "Popen") as spawn: + self.assertEqual(qa.main(argv), 130) + spawn.assert_not_called() + result = qa.read_job(root / "state/jobs/handoff") + self.assertEqual((result["status"], result["signal"]), ("cancelled", sig)) + self.assertFalse(qa.live_report(root / "state/jobs/handoff")["live"]) + self.assertEqual({s: signal.getsignal(s) for s in handlers}, handlers) + self.assertEqual(signal.pthread_sigmask(signal.SIG_BLOCK, []), mask) + def test_cleanup_hold_keeps_lock_until_group_gone_or_deadline(self): for members_remain in (False, True): with self.subTest(members_remain=members_remain), tempfile.TemporaryDirectory() as temp: From 1ceb11002875eeac53aedc1fe3d1bf60fe37eeb6 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 05:17:39 -0500 Subject: [PATCH 13/15] tools: close the qa-resource lifecycle gaps found in review - Build the job record, including its token, before the directory exists. - When a setup failure cannot be recorded either, remove the new job's files and directory instead of leaving a queued record with no runner. - Open the runner log inside run_job's protected region, so a descriptor failure is recorded and releases the lock, FIFO and ticket. - Install submit's post-spawn handlers inside the region that cancels the runner, so a signal between the installs cannot abandon it. - Test PID reuse between the pin and the final identity read, and that a caller's ignored SIGCHLD is restored after a completed run. Co-Authored-By: Claude Opus 5.5 (1M context) --- tools/qa-resource/README.md | 2 +- tools/qa-resource/qa_resource.py | 50 ++++++++---- tools/qa-resource/test_qa_resource.py | 111 +++++++++++++++++++++++++- 3 files changed, 146 insertions(+), 17 deletions(-) diff --git a/tools/qa-resource/README.md b/tools/qa-resource/README.md index de26ab310..e143c2470 100644 --- a/tools/qa-resource/README.md +++ b/tools/qa-resource/README.md @@ -58,7 +58,7 @@ Invisible descendants are not stopped. Darwin omits the environment of `cs_restr A chain whose processes each live shorter than one scan interval can evade two-pass confirmation. Microsecond start times avoid coarse `lstart` identity checks for escaped PIDs, but the check-to-signal PID reuse window remains. -The queue cannot identify a foreign shared-lock holder or attribute a heavy process to a project. FIFO ordering applies only to this tool's tickets; `qa-slot.py` and `package-run.py` still race for the shared flock. Cancellation stops only the job's own child group and proven escaped descendants, and shuts down only the device the job booted. The runner handles SIGTERM, SIGINT, and SIGHUP; later signals cannot interrupt cleanup. `run` and `submit` hold those signals from job creation until their handlers are installed, so an interruption at any point after the job exists is recorded as `cancelled`. If the job directory or its first record cannot be created, the directory is removed and the id stays reusable. A stale job's recorded group and booted-device ownership are reported, never cleaned. Inspect those manually before reusing a device. +The queue cannot identify a foreign shared-lock holder or attribute a heavy process to a project. FIFO ordering applies only to this tool's tickets; `qa-slot.py` and `package-run.py` still race for the shared flock. Cancellation stops only the job's own child group and proven escaped descendants, and shuts down only the device the job booted. The runner handles SIGTERM, SIGINT, and SIGHUP; later signals cannot interrupt cleanup. `run` and `submit` hold those signals from job creation until their handlers are installed, so an interruption at any point after the job exists is recorded as `cancelled`. If the job directory or its first record cannot be created, or a later setup failure cannot be recorded, the directory is removed and the id stays reusable. A stale job's recorded group and booted-device ownership are reported, never cleaned. Inspect those manually before reusing a device. Boot ownership does not identify a simulator session; if another lane shuts down and re-boots the same UDID mid-job, final shutdown affects their session. diff --git a/tools/qa-resource/qa_resource.py b/tools/qa-resource/qa_resource.py index 1333b7091..232e2e6d8 100644 --- a/tools/qa-resource/qa_resource.py +++ b/tools/qa-resource/qa_resource.py @@ -771,11 +771,7 @@ def exit_code(job): def create_job(args): directory = args.state_dir / "jobs" / args.job_id - directory.parent.mkdir(parents=True, exist_ok=True) - try: - directory.mkdir() - except FileExistsError as error: - raise ValueError(f"job {args.job_id} already exists; choose a new id") from error + # Built before the directory exists, so nothing here can fail after it does. job = dict(jobId=args.job_id, kind=args.kind, argv=args.argv, cwd=str(args.cwd), device=args.device, status="queued", reasons=[], runnerPid=None, runnerStart=None, childPgid=None, bootedByJob=False, exitCode=None, @@ -785,6 +781,11 @@ def create_job(args): ownerToken=secrets.token_hex(16), escapedProcesses=[], cancelChannel=CANCEL_FIFO, ignoredProcesses=[], logs={name: str(directory / (name + ".log")) for name in ("output", "runner")}) + directory.parent.mkdir(parents=True, exist_ok=True) + try: + directory.mkdir() + except FileExistsError as error: + raise ValueError(f"job {args.job_id} already exists; choose a new id") from error live = None try: live = (directory / "live.lock").open("a+") @@ -796,9 +797,7 @@ def create_job(args): leftovers = [] if live is not None: cleanup_step(leftovers, "close live lock", live.close) - for path in (directory / "job.json.tmp", directory / "job.json", directory / "live.lock", directory): - cleanup_step(leftovers, f"remove {path}", - lambda path=path: path.rmdir() if path == directory else path.unlink(missing_ok=True)) + leftovers += remove_job_files(directory) detail = f"; {'; '.join(leftovers)}" if leftovers else "" raise JobSetupError(f"job {args.job_id}: cannot create the job record: {error}{detail}") from error try: @@ -823,11 +822,28 @@ def fail_unstarted_job(directory, live, job, message): job.update(status="error", exitCode=125, error=message, finishedAt=timestamp()) try: write_job(directory, job) + except OSError as error: + # The queued record would otherwise outlive this process with no runner. This + # invocation created everything in the directory, so removing it keeps the id reusable. + leftovers = remove_job_files(directory) + detail = f"; {'; '.join(leftovers)}" if leftovers else "" + return JobSetupError(f"job {job['jobId']}: {message}; cannot record that error ({error}), " + f"so the job directory was removed{detail}") finally: live.close() return JobSetupError(f"job {job['jobId']}: {message}") +def remove_job_files(directory): + """Remove what job setup creates, by name, then the directory. Returns failures.""" + leftovers = [] + for name in ("job.json.tmp", "job.json", "live.lock", "output.log", "runner.log", CANCEL_FIFO): + cleanup_step(leftovers, f"remove {directory / name}", + lambda name=name: (directory / name).unlink(missing_ok=True)) + cleanup_step(leftovers, f"remove {directory}", directory.rmdir) + return leftovers + + class CancelListener: """Turn a byte on the job's FIFO into SIGTERM for this runner's main thread. @@ -932,12 +948,14 @@ def run_job(directory, live, job, lock_path, echo=True, owned_ticket=None, deadl ticket_path, ticket = owned_ticket if owned_ticket else (None, None) if deadline is None: deadline = time.monotonic() + job["admissionDeadline"] - log = (directory / "runner.log").open("a", buffering=1) + # Opened inside the try below, so a failure still gets a terminal record and cleanup. + log = None def update(**fields): job.update(fields) write_job(directory, job) line = f"{timestamp()} {job['status']} " + json.dumps(fields, sort_keys=True) - log.write(line + "\n") + if log is not None: + log.write(line + "\n") if echo: print(line, flush=True) def report_escaped(identity, outcome): @@ -961,7 +979,8 @@ def interrupted(signum, frame): old_sigchld = signal.signal(signal.SIGCHLD, signal.SIG_DFL) listener = None try: - # submit starts the runner with these blocked; a cancel that arrived meanwhile is + log = (directory / "runner.log").open("a", buffering=1) + # main and submit start this with these blocked; a cancel that arrived meanwhile is # raised here, inside the try, so it still gets a terminal result and cleanup. signal.pthread_sigmask(signal.SIG_UNBLOCK, RUNNER_SIGNALS) if cancel_fd is not None or (directory / CANCEL_FIFO).exists(): @@ -1135,7 +1154,8 @@ def mark_cleanup_errors(): cleanup_step(cleanup_errors, "close shared QA lock", global_lock.close) cleanup_step(cleanup_errors, "release queue ticket", lambda: cleanup_errors.extend(release_ticket(ticket_path, ticket))) - cleanup_step(cleanup_errors, "close runner log", log.close) + if log is not None: + cleanup_step(cleanup_errors, "close runner log", log.close) mark_cleanup_errors() cleanup_step(cleanup_errors, "write release result", lambda: write_job(directory, job)) recorded_errors = len(cleanup_errors) @@ -1218,9 +1238,11 @@ def record(signum, frame): printing = False # From here an interruption cancels the job instead of abandoning it, and a # submitter blocked on a full stdout pipe can still be stopped. - for sig in RUNNER_SIGNALS: - signal.signal(sig, interrupt) try: + # Inside the try: a signal landing between these installs still cancels + # the runner, instead of escaping to the outer handler and abandoning it. + for sig in RUNNER_SIGNALS: + signal.signal(sig, interrupt) if pending: raise Cancelled(pending[0]) printing = True diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py index d6b0d8e8c..c58258b34 100644 --- a/tools/qa-resource/test_qa_resource.py +++ b/tools/qa-resource/test_qa_resource.py @@ -797,9 +797,16 @@ def test_blocked_submit_receipt_does_not_hold_finished_jobs_liveness(self): def test_run_inheriting_ignored_sigchld_still_records_payload_failure(self): # Under SIG_IGN, Darwin reaps the payload at exit and Popen reports ECHILD as status 0. - ignoring = "import signal; signal.signal(signal.SIGCHLD, signal.SIG_IGN); " + BOOTSTRAP + ignoring = ("import signal; signal.signal(signal.SIGCHLD, signal.SIG_IGN); " + + BOOTSTRAP.replace("sys.exit(q.main(sys.argv[2:]))", + "code = q.main(sys.argv[2:]); " + "print('caller SIGCHLD restored', signal.getsignal(signal.SIGCHLD) == signal.SIG_IGN); " + "sys.exit(code)")) runner = self.launch(self.job_args("ignored-sigchld", "import sys; sys.exit(3)"), bootstrap=ignoring) - result = self.finish(runner, "ignored-sigchld", 3) + output, error = runner.communicate(timeout=8) + self.assertEqual(runner.returncode, 3, (output, error)) + self.assertIn("caller SIGCHLD restored True", output) + result = self.job("ignored-sigchld") self.assertEqual((result["status"], result["exitCode"]), ("failed", 3)) self.assert_lock_free() @@ -1154,6 +1161,23 @@ def kernel(changes=None): self.assertTrue(all(p["proof"] == "ancestry" for p in found)) kill.assert_not_called() + def test_pid_reused_between_pin_and_final_read_is_not_owned(self): + job = dict(jobId="job", ownerToken="a" * 32) + uid = os.getuid() + rows = {123: dict(pid=123, ppid=1, pgid=123, uid=uid, stat="S"), + 124: dict(pid=124, ppid=123, pgid=123, uid=uid, stat="S")} + reads = {} + def kernel(pid): + reads[pid] = reads.get(pid, 0) + 1 + # 124 is a true descendant when pinned, then exits and its PID is reused. + start = 900_000_000 if pid == 124 and reads[pid] > 1 else pid * 1_000_000 + return dict(pid=pid, start_us=start, uid=uid, ppid=rows[pid]["ppid"], zombie=False) + with patch.object(qa, "process_snapshot", return_value=rows), \ + patch.object(qa, "process_env", return_value=[]), \ + patch.object(qa, "kernel_identity", side_effect=kernel), \ + patch.object(qa, "process_identity", side_effect=lambda pid, **unused: dict(pid=pid, start="date", comm="python")): + self.assertEqual([p["pid"] for p in qa.owned_processes(job, 123)], [123]) + def test_proven_identity_survives_lost_token_and_reparenting(self): identity = dict(pid=124, uid=os.getuid(), start_us=124_000_001, start="child-start", comm="python", proof="ancestry") @@ -1519,6 +1543,89 @@ def full_disk_lock(path, *rest, **keywords): live.close() self.assertEqual(qa.read_job(directory)["status"], "queued") + def test_setup_failure_without_an_error_record_removes_the_directory(self): + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + args = Mock(state_dir=root / "state", job_id="no-record", kind="package", argv=["unused"], + cwd=root, device=None, admission_deadline=3, timeout=1) + with patch.object(qa.secrets, "token_hex", side_effect=OSError(5, "entropy unavailable")): + with self.assertRaises(OSError): + qa.create_job(args) + self.assertFalse((root / "state/jobs/no-record").exists()) + real_write = qa.write_job + writes = [] + def second_write_fails(directory, job): + writes.append(job["status"]) + if len(writes) > 1: + raise OSError(28, "No space left on device") + real_write(directory, job) + original_touch = Path.touch + def no_log(path, *rest, **keywords): + if path.name == "output.log": + raise OSError(28, "No space left on device") + return original_touch(path, *rest, **keywords) + with patch.object(qa, "write_job", side_effect=second_write_fails), \ + patch.object(Path, "touch", autospec=True, side_effect=no_log): + with self.assertRaisesRegex(qa.JobSetupError, "cannot set up job files: .*cannot record that error"): + qa.create_job(args) + self.assertEqual(writes, ["queued", "error"]) + self.assertFalse((root / "state/jobs/no-record").exists()) + directory, live, job = qa.create_job(args) + live.close() + + def test_runner_log_failure_is_recorded_and_releases_handles(self): + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + args = Mock(state_dir=root / "state", job_id="no-log", kind="package", argv=["unused"], + cwd=root, device=None, admission_deadline=3, timeout=1) + directory, live, job = qa.create_job(args) + cancel_fd = qa.open_cancel_reader(directory) + original_open = Path.open + def no_descriptors(path, *rest, **keywords): + if path.name == "runner.log": + raise OSError(24, "Too many open files") + return original_open(path, *rest, **keywords) + with patch.object(Path, "open", autospec=True, side_effect=no_descriptors), \ + patch.object(qa.subprocess, "Popen") as spawn: + self.assertEqual(qa.run_job(directory, live, job, root / "lock", echo=False, cancel_fd=cancel_fd), 125) + spawn.assert_not_called() + result = qa.read_job(directory) + self.assertEqual(result["status"], "error") + self.assertIn("Too many open files", result["error"]) + self.assertTrue(live.closed) + self.assertFalse(qa.live_report(directory)["live"]) + with self.assertRaises(OSError): + os.fstat(cancel_fd) + + def test_signal_while_submit_reinstalls_handlers_after_spawn_cancels_the_runner(self): + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + args = Mock(state_dir=root / "state", job_id="after-spawn", kind="package", argv=["unused"], + cwd=root, device=None, admission_deadline=3, timeout=1) + directory, live, job = qa.create_job(args) + real_kill, real_signal = os.kill, signal.signal + spawned, kills, receipts = [], [], [] + def popen(*unused, **keywords): + spawned.append(True) + return Mock(pid=987654) # Never a real process: os.kill below only records it. + def install(sig, handler): + previous = real_signal(sig, handler) + if spawned and getattr(handler, "__name__", "") == "interrupt" and sig == qa.RUNNER_SIGNALS[0]: + real_kill(os.getpid(), signal.SIGTERM) # Lands before the other installs. + return previous + def kill(pid, signum): + self.assertEqual(pid, 987654) + kills.append(signum) + with patch.object(qa.subprocess, "Popen", side_effect=popen), \ + patch.object(qa.signal, "signal", side_effect=install), \ + patch.object(qa.os, "kill", side_effect=kill), \ + patch.object(qa, "write_receipt", side_effect=receipts.append): + code = qa.submit(directory, live, job, root / "lock", qa.open_cancel_reader(directory)) + self.assertEqual(code, 130) + self.assertEqual(kills, [signal.SIGTERM]) + self.assertEqual(len(receipts), 1) + self.assertTrue(receipts[0]["cancelRequested"]) + def test_submit_interrupted_while_waiting_for_its_ticket_records_cancelled(self): for sig in qa.RUNNER_SIGNALS: with self.subTest(signal=sig), tempfile.TemporaryDirectory() as temp: From 22638a70aeebc8dab1082537058948650a94b61a Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 05:28:50 -0500 Subject: [PATCH 14/15] tools: report escaped descendants instead of signalling them A PID recheck followed by kill cannot rule out the PID being reused in between, and macOS has no identity-bound signal. Cleanup now signals only the pinned child group, whose unreaped leader keeps its id from being reused. A live escapee is recorded as retained, the job ends cleanup_failed with it listed, the shared lock is held through the cleanup hold, and later admissions wait while that exact execution is alive, including escapees proven only by ancestry, which carry no token. Co-Authored-By: Claude Opus 5.5 (1M context) --- tools/qa-resource/README.md | 16 +-- tools/qa-resource/qa_resource.py | 84 ++++++++------- tools/qa-resource/test_qa_resource.py | 143 +++++++++++++++----------- 3 files changed, 134 insertions(+), 109 deletions(-) diff --git a/tools/qa-resource/README.md b/tools/qa-resource/README.md index e143c2470..352c3a714 100644 --- a/tools/qa-resource/README.md +++ b/tools/qa-resource/README.md @@ -42,23 +42,25 @@ Some machines also make every heavy command take an outer lock, such as `/usr/bi Each payload gets a random `QA_RESOURCE_OWNER_=` environment entry. At admission and cleanup, the runner reads process memory with Darwin `KERN_PROCARGS2`. An exact entry or live-parent ancestry proves ownership at discovery. A `ps` snapshot only nominates descendants, because its PIDs may have been reused since. Ancestry counts only when the kernel's current parent chain, read from the pinned process up to the payload, holds at every link, and each parent started no later than its child. Otherwise the process is not treated as owned. The runner resets an inherited `SIGCHLD` disposition to the default while it owns the payload, so the payload stays an unreaped zombie that keeps its exit status and pins its PID and process-group id. The parser checks every NUL-separated string after the executable because empty `argv[0]` is indistinguishable from padding. Admission accepts only owner headers with a 32-character lowercase hexadecimal token. -On cancellation, timeout, and normal exit, the runner signals discovered escapes before its pinned child group. Discovery proof stays attached to PID, microsecond start time, and uid even after reparenting or token removal. Each per-PID signal rechecks that identity with `KERN_PROC_PID` and excludes zombies and other uids. The runner never signals an escaped process's new group. The leader stays unreaped until checks finish. +On cancellation, timeout, and normal exit, the runner signals only its pinned child group: SIGTERM, then SIGKILL after the grace period. The group leader stays unreaped until every check finishes, so the group id cannot be reused while the runner signals it. Escaped processes are discovered before that SIGTERM, so their proof is recorded before their parents die, and they are reported but never signalled. A PID recheck followed by `kill` cannot rule out the PID being reused in between, and macOS has no signal bound to a process identity, so the runner does not try. -`escapedProcesses` in `job.json` and `runner.log` records `pid`, `start_us`, `uid`, human-readable `start`, executable at discovery, proof, and outcome. `terminated` and `killed` mean the runner sent that signal and then confirmed the identity was no longer live. `exited` means no signal was sent; `retained` means cleanup could not confirm exit. +An escaped process that is still alive makes cleanup incomplete. The job ends `cleanup_failed` (exit 125) with the process listed in `cleanupError`, and the runner keeps the shared lock through the cleanup hold before releasing it. A person decides whether to stop the process. + +`escapedProcesses` in `job.json` and `runner.log` records `pid`, `start_us`, `uid`, human-readable `start`, executable at discovery, proof, and outcome. `exited` means the process ended on its own before cleanup finished; `retained` means it was still alive. Confirmation requires two consecutive complete empty passes at least 0.1 seconds apart, after the last observed owned process. Incomplete reads or expired inspection budgets never count as empty. Discovery errors do not skip signaling the pinned group, but the job reports `cleanup_failed` and enters the cleanup hold. -Admission waits for live same-user owner entries, including orphans from dead runners or expired holds. It never signals these blockers. The runner and its ancestors are excluded as a defense. The payload does not inherit the global lock descriptor. `run` and `submit` reject an inherited owner entry before creating a job, returning 2 with `nested qa-resource jobs are not supported: this process belongs to job X`. +Admission waits for live same-user owner entries, including orphans from dead runners or expired holds, and for any `retained` process recorded by an earlier job while that exact execution (PID, start time and uid) is still alive. The second check covers escapees proven only by ancestry, which carry no token. Admission never signals these blockers. The runner and its ancestors are excluded as a defense. The payload does not inherit the global lock descriptor. `run` and `submit` reject an inherited owner entry before creating a job, returning 2 with `nested qa-resource jobs are not supported: this process belongs to job X`. ## What the tool cannot guarantee -The token is a cooperative marker in mutable process memory, not an authenticated exec record. A process can overwrite its environment strings, exec without the token, or copy the token. Copying it onto an unrelated same-uid process makes cleanup treat that process as owned and signal it. +The token is a cooperative marker in mutable process memory, not an authenticated exec record. A process can overwrite its environment strings, exec without the token, or copy the token. Copying it onto an unrelated same-uid process makes cleanup report that process as retained and admission wait for it; the runner never signals it. -Invisible descendants are not stopped. Darwin omits the environment of `cs_restricted` processes without an error, other-uid environments are unreadable, and already-reparented tokenless helpers have no ancestry after natural exit. An unreadable environment without ancestry is not ownership proof. A descendant under another uid cannot be signaled; if ancestry finds it, cleanup fails. `succeeded` means no observable owned process remained, not that every descendant was stopped. +Invisible descendants are not reported. Darwin omits the environment of `cs_restricted` processes without an error, other-uid environments are unreadable, and already-reparented tokenless helpers have no ancestry after natural exit. An unreadable environment without ancestry is not ownership proof. If ancestry finds a descendant under another uid, it is reported like any other escapee. `succeeded` means no observable owned process remained, not that every descendant has exited. -A chain whose processes each live shorter than one scan interval can evade two-pass confirmation. Microsecond start times avoid coarse `lstart` identity checks for escaped PIDs, but the check-to-signal PID reuse window remains. +A chain whose processes each live shorter than one scan interval can evade two-pass confirmation. -The queue cannot identify a foreign shared-lock holder or attribute a heavy process to a project. FIFO ordering applies only to this tool's tickets; `qa-slot.py` and `package-run.py` still race for the shared flock. Cancellation stops only the job's own child group and proven escaped descendants, and shuts down only the device the job booted. The runner handles SIGTERM, SIGINT, and SIGHUP; later signals cannot interrupt cleanup. `run` and `submit` hold those signals from job creation until their handlers are installed, so an interruption at any point after the job exists is recorded as `cancelled`. If the job directory or its first record cannot be created, or a later setup failure cannot be recorded, the directory is removed and the id stays reusable. A stale job's recorded group and booted-device ownership are reported, never cleaned. Inspect those manually before reusing a device. +The queue cannot identify a foreign shared-lock holder or attribute a heavy process to a project. FIFO ordering applies only to this tool's tickets; `qa-slot.py` and `package-run.py` still race for the shared flock. Cancellation stops only the job's own child group, reports proven escaped descendants without signalling them, and shuts down only the device the job booted. The runner handles SIGTERM, SIGINT, and SIGHUP; later signals cannot interrupt cleanup. `run` and `submit` hold those signals from job creation until their handlers are installed, so an interruption at any point after the job exists is recorded as `cancelled`. If the job directory or its first record cannot be created, or a later setup failure cannot be recorded, the directory is removed and the id stays reusable. A stale job's recorded group and booted-device ownership are reported, never cleaned. Inspect those manually before reusing a device. Boot ownership does not identify a simulator session; if another lane shuts down and re-boots the same UDID mid-job, final shutdown affects their session. diff --git a/tools/qa-resource/qa_resource.py b/tools/qa-resource/qa_resource.py index 232e2e6d8..8c2f2f16b 100644 --- a/tools/qa-resource/qa_resource.py +++ b/tools/qa-resource/qa_resource.py @@ -374,19 +374,32 @@ def owner_blockers(): return pids, reasons -def signal_owned_process(identity, signum): - """Ownership proof survives reparenting; recheck the exact proven identity.""" - current = kernel_identity(identity["pid"]) - if (current is None or current["start_us"] != identity["start_us"] - or current["uid"] != identity["uid"] or current["uid"] != os.getuid() - or current["zombie"]): - return False - try: - # Microsecond identity prevents coarse lstart reuse, not this final race. - os.kill(current["pid"], signum) - return True - except (ProcessLookupError, PermissionError): - return False +def retained_blockers(jobs_dir, job_id): + """Escapees an earlier job could not see exit, while that exact execution still lives. + + Escaped processes are reported, never signalled, so admission waits for them instead. + A token-bearing escapee is also caught by owner_blockers; this covers one proven only by + ancestry, which carries no token. A record whose identity no longer matches is gone. + """ + pids, reasons = [], [] + for record_path in sorted(jobs_dir.glob("*/job.json")): + if record_path.parent.name == job_id: + continue + try: + records = json.loads(record_path.read_text()).get("escapedProcesses") or [] + except (OSError, ValueError) as error: + reasons.append(f"cannot read retained processes in {record_path}: {error}") + continue + for record in records: + if record.get("outcome") != "retained": + continue + current = kernel_identity(record["pid"]) + if (current is not None and not current["zombie"] + and current["start_us"] == record["start_us"] and current["uid"] == record["uid"]): + pids.append(record["pid"]) + reasons.append(f"retained process {record['pid']} ({record.get('comm', '?')}) " + f"from job {record_path.parent.name} is still alive") + return pids, reasons def list_processes(): @@ -539,7 +552,7 @@ class OwnedProcesses: """Keep discovery proof attached to an exact identity until that execution ends.""" def __init__(self, job, child_pid, report=None): self.job, self.child_pid, self.report = job, child_pid, report - self.identities, self.sent, self.escaped = {}, {}, {} + self.identities, self.escaped = {}, {} self.errors = [] self.last_incomplete = None self.saw_owned = False @@ -594,10 +607,7 @@ def snapshot(self, deadline): current.pop(key) self.identities.pop(key) if key in self.escaped: - outcome = {signal.SIGTERM: "terminated", signal.SIGKILL: "killed"}.get( - self.sent.get(key), "exited") - self.record(previous, outcome) - self.sent.pop(key, None) + self.record(previous, "exited") for key, identity in current.items(): self.identities[key] = identity if identity["pgid"] != self.child_pid and key not in self.escaped: @@ -608,17 +618,6 @@ def snapshot(self, deadline): self.saw_owned = bool(observed) or bool(current) return list(current.values()), complete - def signal(self, identities, signum): - for identity in identities: - key = identity_key(identity) - if identity["pgid"] == self.child_pid or self.sent.get(key) == signum: - continue - try: - if signal_owned_process(identity, signum): - self.sent[key] = signum - except Exception as error: - self.error(f"signal escaped PID {identity['pid']}: {error}") - def retained(self, identities): for identity in identities: if identity_key(identity) in self.escaped: @@ -643,7 +642,7 @@ def owned_state(child, owned, deadline): return remaining, group_live, complete -def confirm_owned_exit(child, owned, deadline, escalate=True, poll_seconds=.1): +def confirm_owned_exit(child, owned, deadline, poll_seconds=.1): empty_at = None while True: remaining, group_live, complete = owned_state(child, owned, deadline) @@ -654,8 +653,6 @@ def confirm_owned_exit(child, owned, deadline, escalate=True, poll_seconds=.1): empty_at = now if empty_at is None else empty_at else: empty_at = None - if escalate: - owned.signal(remaining, signal.SIGKILL) if now >= deadline: retained = owned.retained(remaining) raise RuntimeError("completeness uncertain; " @@ -676,10 +673,11 @@ def stop_owned_command(child, owned=None): remaining, complete = owned.snapshot(time.monotonic() + SENSING_TIMEOUT) if not complete: owned.error("initial discovery incomplete: " + (owned.last_incomplete or "unknown reason")) - # Signal discovered escapes first. Their identity stays proven even if their - # parents die on the group SIGTERM and launchd adopts them immediately. - owned.signal(remaining, signal.SIGTERM) - # Discovery failures must never skip either signal to the pinned group. + # Discover escapes before the group SIGTERM, so their proof is recorded before their + # parents die and launchd adopts them. They are reported, never signalled: a PID + # recheck followed by kill cannot rule out reuse in between, and macOS has no + # identity-bound signal. The group is safe to signal because its unreaped leader pins + # the group id. Discovery failures must never skip either signal to the group. try: signal_owned_group(child.pid, signal.SIGTERM) except Exception as error: @@ -687,11 +685,9 @@ def stop_owned_command(child, owned=None): deadline = time.monotonic() + CLEANUP_GRACE_SECONDS while True: remaining, group_live, complete = owned_state(child, owned, deadline) - if complete and not group_live and not remaining: - break - owned.signal(remaining, signal.SIGTERM) + if complete and not group_live: + break # Escapees are left to confirm_owned_exit, which reports them. if time.monotonic() >= deadline: - owned.signal(remaining, signal.SIGKILL) try: signal_owned_group(child.pid, signal.SIGKILL) except Exception as error: @@ -705,7 +701,7 @@ def stop_owned_command(child, owned=None): raise RuntimeError(f"{error}; " + "; ".join(owned.errors)) from error raise if owned.errors: - raise RuntimeError("cleanup discovery or signaling failed: " + "; ".join(owned.errors)) + raise RuntimeError("cleanup discovery failed: " + "; ".join(owned.errors)) # Never poll or reap before the final complete pass: the leader pins its pgid. return child.wait(timeout=CLEANUP_CONFIRM_SECONDS) @@ -713,8 +709,7 @@ def stop_owned_command(child, owned=None): def hold_owned_group(child, owned=None): owned = owned if owned is not None else OwnedProcesses({}, child.pid) try: - confirm_owned_exit(child, owned, time.monotonic() + CLEANUP_HOLD_SECONDS, - escalate=False, poll_seconds=1) + confirm_owned_exit(child, owned, time.monotonic() + CLEANUP_HOLD_SECONDS, poll_seconds=1) return "no live members remained when the shared QA lock was released" except Exception as error: return (f"{error} when the shared QA lock was released " @@ -1019,6 +1014,9 @@ def interrupted(signum, frame): reasons = resource_blockers(pressure, memory, sense_disk()) heavy, owner_reasons = owner_blockers() reasons.extend(owner_reasons) + retained, retained_reasons = retained_blockers(directory.parent, job["jobId"]) + heavy.extend(retained) # Only waited on for exit, never signalled. + reasons.extend(retained_reasons) ignored = [] for process in list_processes(): classification = classify_process(process, job["kind"], job["device"]) diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py index c58258b34..43e74596e 100644 --- a/tools/qa-resource/test_qa_resource.py +++ b/tools/qa-resource/test_qa_resource.py @@ -77,13 +77,13 @@ def __init__(self, args, *positional, **keywords): and args[2].startswith('import sys; sys.path.insert(0, sys.argv[1]); import qa_resource as q; sys.exit(q.resume(')): args = list(args) settings = ('q.OWNER_ENV_PREFIX=' + repr(q.OWNER_ENV_PREFIX) + '; ' - 'q.CLEANUP_GRACE_SECONDS=.2; q.RESAMPLE_SECONDS=.1; ') + 'q.CLEANUP_GRACE_SECONDS=.2; q.RESAMPLE_SECONDS=.1; q.CLEANUP_HOLD_SECONDS=1; ') args[2] = args[2].replace('import qa_resource as q; ', 'import qa_resource as q; ' + settings) super().__init__(args, *positional, **keywords) q.subprocess.Popen = TestPopen ''' BOOTSTRAP = ("import sys; sys.path.insert(0, sys.argv[1]); import qa_resource as q; " - "q.CLEANUP_GRACE_SECONDS=.2; q.RESAMPLE_SECONDS=.1; " + "q.CLEANUP_GRACE_SECONDS=.2; q.RESAMPLE_SECONDS=.1; q.CLEANUP_HOLD_SECONDS=1; " "sys.exit(q.main(sys.argv[2:]))") @@ -282,19 +282,33 @@ def escape_program(self, files, ignores, parent_live=True, ancestry=False, paren parent += "time.sleep(30)\n" return parent + def stop_test_escapee(self, pid): + start = dict(self.escaped_targets)[pid] + identity = qa.process_identity(pid, details=True) + if identity and identity["start"] == start and not identity["stat"].startswith("Z"): + os.kill(pid, signal.SIGKILL) # Only a PID this test spawned and recorded. + deadline = time.monotonic() + 3 + while not self.escaped_gone(pid): + if time.monotonic() >= deadline: + self.fail(f"test escapee {pid} did not exit") + time.sleep(.02) + def escaped_gone(self, pid): identity = qa.process_identity(pid, details=True) return identity is None or identity["stat"].startswith("Z") def assert_escaped_result(self, job, expected): + """Escapees are reported, never signalled, and their job never claims clean cleanup.""" + self.assertEqual((job["status"], job["exitCode"]), ("cleanup_failed", 125)) records = {p["pid"]: p for p in job["escapedProcesses"]} self.assertEqual(set(records), set(expected)) - for pid, (proof, outcome) in expected.items(): + for pid, proof in expected.items(): self.assertEqual(records[pid]["proof"], proof) - self.assertEqual(records[pid]["outcome"], outcome) + self.assertEqual(records[pid]["outcome"], "retained") self.assertTrue(records[pid]["start"]) self.assertTrue(records[pid]["comm"]) - self.assertTrue(self.escaped_gone(pid), records[pid]) + self.assertFalse(self.escaped_gone(pid), records[pid]) + self.assertIn(f"PID {pid} ", job["cleanupError"]) log = Path(job["logs"]["runner"]).read_text() self.assertIn('"escapedProcesses"', log) self.assert_lock_free() @@ -310,21 +324,20 @@ def test_cancel_reparented_session_descendants_and_preserve_sentinel(self): self.assertEqual(identity["ppid"], 1) self.assertEqual(identity["pgid"], pid) self.assertEqual(self.invoke(self.cli("cancel", "escape-cancel")).returncode, 0) - result = self.finish(runner, "escape-cancel", 130) - self.assert_escaped_result(result, {pids[0]: ("token", "terminated"), pids[1]: ("token", "killed")}) + result = self.finish(runner, "escape-cancel", 125) + self.assert_escaped_result(result, {pids[0]: "token", pids[1]: "token"}) self.assertIsNone(sentinel.poll()) - def test_timeout_kills_escaped_ignoring_descendant_not_sentinel(self): + def test_timeout_retains_escaped_descendant_and_spares_sentinel(self): sentinel = self.sleeper(30) path = self.root / "escaped" runner = self.launch(self.job_args("escape-timeout", self.escape_program([path], [True]), timeout=.8)) pid = self.escaped_pid(path) - result = self.finish(runner, "escape-timeout", 124) - self.assertEqual(result["status"], "timed_out") - self.assert_escaped_result(result, {pid: ("token", "killed")}) + result = self.finish(runner, "escape-timeout", 125) + self.assert_escaped_result(result, {pid: "token"}) self.assertIsNone(sentinel.poll()) - def test_natural_exit_cleans_detached_helper_before_lock_release(self): + def test_natural_exit_holds_lock_and_reports_live_detached_helper(self): path = self.root / "escaped" runner = self.launch(self.job_args("escape-natural", self.escape_program([path], [True], parent_live=False))) pid = self.escaped_pid(path) @@ -333,15 +346,15 @@ def test_natural_exit_cleans_detached_helper_before_lock_release(self): while True: try: fcntl.flock(probe, fcntl.LOCK_EX | fcntl.LOCK_NB) - self.assertTrue(self.escaped_gone(pid), "escaped helper lived past lock release") + # Released only after the hold; the helper is still running and reported. + self.assertFalse(self.escaped_gone(pid), "the queue must not stop an escaped helper") break except BlockingIOError: if time.monotonic() >= deadline: self.fail("natural-exit cleanup did not release the test lock") time.sleep(.01) - result = self.finish(runner, "escape-natural", 0) - self.assertEqual(result["status"], "succeeded") - self.assert_escaped_result(result, {pid: ("token", "killed")}) + result = self.finish(runner, "escape-natural", 125) + self.assert_escaped_result(result, {pid: "token"}) def test_tokenless_session_descendant_is_owned_by_live_ancestry(self): path = self.root / "ancestry" @@ -351,21 +364,19 @@ def test_tokenless_session_descendant_is_owned_by_live_ancestry(self): self.assertFalse(any(entry.startswith(TEST_OWNER_PREFIX) for entry in qa.process_env(pid))) self.assertNotEqual(qa.process_identity(pid, details=True)["ppid"], 1) self.assertEqual(self.invoke(self.cli("cancel", "escape-ancestry")).returncode, 0) - result = self.finish(runner, "escape-ancestry", 130) - self.assert_escaped_result(result, {pid: ("ancestry", "killed")}) + result = self.finish(runner, "escape-ancestry", 125) + self.assert_escaped_result(result, {pid: "ancestry"}) - def test_ineffective_escaped_kill_records_retained_and_holds_lock(self): + def test_escaped_descendant_is_never_signalled_and_holds_lock(self): path = self.root / "retained" args = Mock(state_dir=self.state, job_id="retained", kind="package", argv=[sys.executable, "-c", self.escape_program([path], [True], parent_live=False)], cwd=self.root, device=None, admission_deadline=5, timeout=3) directory, live, job = qa.create_job(args) - real_kill, real_sleep = os.kill, time.sleep - held = [] - def ineffective(pid, sig): - if path.exists() and pid == int(path.read_text()): - return None - return real_kill(pid, sig) + real_sleep = time.sleep + held, kills = [], [] + def no_kill(pid, sig): + kills.append((pid, sig)) # Recorded, never delivered: no PID is signalled here. def observe_hold(seconds): if qa.read_job(directory)["status"] == "cleanup_failed": with self.lock.open("a+") as probe: @@ -380,11 +391,12 @@ def observe_hold(seconds): patch.object(qa, "CLEANUP_GRACE_SECONDS", .1), \ patch.object(qa, "CLEANUP_CONFIRM_SECONDS", .1), \ patch.object(qa, "CLEANUP_HOLD_SECONDS", .2), \ - patch.object(qa.os, "kill", side_effect=ineffective), \ + patch.object(qa.os, "kill", side_effect=no_kill), \ patch.object(qa.time, "sleep", side_effect=observe_hold): self.assertEqual(qa.run_job(directory, live, job, self.lock, echo=False), 125) pid = self.escaped_pid(path) result = qa.read_job(directory) + self.assertEqual(kills, []) self.assertEqual(result["status"], "cleanup_failed") self.assertTrue(held) self.assertIn(str(pid), result["cleanupError"]) @@ -410,6 +422,25 @@ def test_token_orphan_admission_deadline_never_signals_orphan(self): self.assertIsNone(result["childPgid"]) self.assert_lock_free() + def test_live_retained_escapee_blocks_admission_and_is_never_signalled(self): + escapee = self.sleeper(30) + identity = qa.kernel_identity(escapee.pid) + earlier = self.state / "jobs" / "earlier" + earlier.mkdir(parents=True) + record = dict(pid=escapee.pid, start_us=identity["start_us"], uid=identity["uid"], + start="test", comm="sleeper", proof="ancestry", outcome="retained") + (earlier / "job.json").write_text(json.dumps(dict(jobId="earlier", childPgid=None, escapedProcesses=[record]))) + runner = self.launch(self.job_args("after-retained", deadline=1)) + self.await_job("after-retained", lambda j: any(f"retained process {escapee.pid}" in r for r in j["reasons"])) + result = self.finish(runner, "after-retained", 75) + self.assertEqual(result["status"], "admission_timeout") + self.assertIsNone(escapee.poll()) + # A record whose execution is gone, here a different start time, no longer blocks. + record["start_us"] += 1 + (earlier / "job.json").write_text(json.dumps(dict(jobId="earlier", childPgid=None, escapedProcesses=[record]))) + self.assertEqual(self.finish(self.launch(self.job_args("after-gone")), "after-gone", 0)["status"], "succeeded") + self.assertIsNone(escapee.poll()) + def test_token_bearing_ancestor_is_excluded_when_runner_has_no_token(self): outer_name = TEST_OWNER_PREFIX + secrets.token_hex(16) self.env[outer_name] = "ancestor-job" @@ -455,8 +486,8 @@ def test_tokenless_escape_survives_parent_term_but_not_durable_cleanup(self): pid = self.escaped_pid(path) self.assertFalse(any(entry.startswith(TEST_OWNER_PREFIX) for entry in qa.process_env(pid))) self.assertEqual(self.invoke(self.cli("cancel", "durable-ancestry")).returncode, 0) - result = self.finish(runner, "durable-ancestry", 130) - self.assert_escaped_result(result, {pid: ("ancestry", "killed")}) + result = self.finish(runner, "durable-ancestry", 125) + self.assert_escaped_result(result, {pid: "ancestry"}) self.assertGreater(result["escapedProcesses"][0]["start_us"], 0) self.assertIsNone(sentinel.poll()) @@ -476,8 +507,11 @@ def test_empty_argv_sleep_is_found_on_cancel_and_natural_exit(self): entry = qa.owner_entry(self.job(name)) self.assertIn(entry, qa.process_env(pid)) self.assertEqual(self.invoke(self.cli("cancel", name)).returncode, 0) - result = self.finish(runner, name, 0 if natural else 130) - self.assert_escaped_result(result, {pid: ("token", "terminated")}) + result = self.finish(runner, name, 125) + self.assert_escaped_result(result, {pid: "token"}) + # A live retained escapee blocks the next admission by design; stop this + # test-spawned one before the next subtest. + self.stop_test_escapee(pid) def test_failed_discovery_still_term_and_kills_pinned_group_and_holds_lock(self): code = "import signal,time; signal.signal(signal.SIGTERM,signal.SIG_IGN); time.sleep(30)" @@ -1080,17 +1114,24 @@ def test_identity_parses_command_with_spaces_and_rejects_bad_dates(self): with self.assertRaisesRegex(RuntimeError, "malformed lstart/comm"): qa.process_identity(123) - def test_changed_microsecond_start_uid_and_zombie_never_receive_signal(self): - identity = dict(pid=123, start_us=123_000_001, uid=os.getuid(), - start="old", comm="python", proof="token") - current = dict(pid=123, start_us=identity["start_us"], uid=os.getuid(), zombie=False) - for changes in (dict(start_us=123_000_002), dict(uid=os.getuid() + 1), dict(zombie=True)): - with self.subTest(changes=changes), \ - patch.object(qa, "kernel_identity", return_value=dict(current, **changes)), \ - patch.object(qa.os, "kill") as kill, patch.object(qa.os, "killpg") as killpg: - self.assertFalse(qa.signal_owned_process(identity, signal.SIGKILL)) - kill.assert_not_called() - killpg.assert_not_called() + def test_cleanup_signals_only_the_pinned_group_and_reports_escapees(self): + escaped = dict(pid=124, pgid=124, ppid=1, stat="S", uid=os.getuid(), start_us=124_000_001, + start="escapee-start", comm="helper", proof="token") + member = dict(escaped, pid=125, pgid=123, start_us=125_000_001, comm="member") + child = Mock(pid=123) + with patch.object(qa, "owned_processes", return_value=[escaped, member]), \ + patch.object(qa, "kernel_identity", side_effect=lambda pid: dict(pid=pid, start_us=pid * 1_000_000 + 1, + uid=os.getuid(), ppid=1, zombie=False)), \ + patch.object(qa, "group_has_members", return_value=False), \ + patch.object(qa, "CLEANUP_GRACE_SECONDS", .05), \ + patch.object(qa, "CLEANUP_CONFIRM_SECONDS", .05), \ + patch.object(qa.os, "kill", side_effect=AssertionError("an escaped PID was signalled")), \ + patch.object(qa.os, "killpg") as killpg: + with self.assertRaisesRegex(RuntimeError, "live members remained; retained: PID 124 \\(helper\\)"): + qa.stop_owned_command(child) + self.assertEqual(killpg.call_args_list[0].args, (123, signal.SIGTERM)) + self.assertTrue(all(call.args[0] == 123 for call in killpg.call_args_list)) + child.wait.assert_not_called() # The leader stays unreaped, pinning the group id. def test_token_match_is_exact_and_unreadable_nonancestry_is_not_owned(self): job = dict(jobId="job", ownerToken="a" * 32) @@ -1117,11 +1158,6 @@ def test_foreign_uid_descendant_is_retained_without_environment_read_or_signal(s found = qa.owned_processes(job, 123) self.assertEqual([(p["pid"], p["proof"]) for p in found], [(123, "ancestry"), (124, "ancestry")]) env.assert_called_once_with(123) - with patch.object(qa, "kernel_identity", return_value=dict(found[1], zombie=False)), \ - patch.object(qa, "process_env") as env, patch.object(qa.os, "kill") as kill: - self.assertFalse(qa.signal_owned_process(found[1], signal.SIGTERM)) - env.assert_not_called() - kill.assert_not_called() def test_kernel_identity_reports_the_real_parent(self): self.assertEqual(qa.kernel_identity(os.getpid())["ppid"], os.getppid()) @@ -1178,17 +1214,6 @@ def kernel(pid): patch.object(qa, "process_identity", side_effect=lambda pid, **unused: dict(pid=pid, start="date", comm="python")): self.assertEqual([p["pid"] for p in qa.owned_processes(job, 123)], [123]) - def test_proven_identity_survives_lost_token_and_reparenting(self): - identity = dict(pid=124, uid=os.getuid(), start_us=124_000_001, - start="child-start", comm="python", proof="ancestry") - with patch.object(qa, "kernel_identity", return_value=dict(identity, zombie=False)), \ - patch.object(qa, "process_identity") as ps, \ - patch.object(qa, "process_env", return_value=[]) as env, patch.object(qa.os, "kill") as kill: - self.assertTrue(qa.signal_owned_process(identity, signal.SIGTERM)) - env.assert_not_called() - ps.assert_not_called() - kill.assert_called_once_with(124, signal.SIGTERM) - def test_kernel_start_microseconds_agree_with_live_ps_lstart(self): identity = qa.kernel_identity(os.getpid()) human = qa.runner_start(os.getpid()) @@ -1282,14 +1307,14 @@ def test_confirmation_needs_two_complete_empty_passes_after_every_reset(self): def sleep(seconds): clock[0] += seconds + .001 with patch.object(owned, "snapshot", side_effect=sequence) as snapshots, \ - patch.object(owned, "signal") as signal_owned, \ patch.object(qa, "group_has_members", return_value=False), \ + patch.object(qa.os, "kill", side_effect=AssertionError("confirmation signalled a PID")), \ + patch.object(qa.os, "killpg", side_effect=AssertionError("confirmation signalled a group")), \ patch.object(qa.time, "monotonic", side_effect=lambda: clock[0]), \ patch.object(qa.time, "sleep", side_effect=sleep): qa.confirm_owned_exit(child, owned, 1) self.assertEqual(snapshots.call_count, 7) self.assertGreaterEqual(clock[0], .6) - self.assertEqual(signal_owned.call_args_list[0].args, ([previous], signal.SIGKILL)) clock[0] = 0 with patch.object(owned, "snapshot", return_value=([], False)), \ patch.object(qa, "group_has_members", return_value=False), \ From fca2172773d1009b5d4e818a728377aeb675af9e Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 3 Oct 2026 05:39:15 -0500 Subject: [PATCH 15/15] tools: persist every live identity as retained and validate the records - At the cleanup deadline, record every proven identity still alive, not only those that left the payload group, so a later admission can wait for a member that dropped its token while process scans were failing. - retained_blockers validates each job record and retained entry. A non-object record, a non-list escapedProcesses, or an entry without integer pid, start_us and uid is a path-specific waiting reason, not a crash or a bypass. Co-Authored-By: Claude Opus 5.5 (1M context) --- tools/qa-resource/qa_resource.py | 20 ++++++++--- tools/qa-resource/test_qa_resource.py | 48 +++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/tools/qa-resource/qa_resource.py b/tools/qa-resource/qa_resource.py index 8c2f2f16b..f973a86e9 100644 --- a/tools/qa-resource/qa_resource.py +++ b/tools/qa-resource/qa_resource.py @@ -386,12 +386,23 @@ def retained_blockers(jobs_dir, job_id): if record_path.parent.name == job_id: continue try: - records = json.loads(record_path.read_text()).get("escapedProcesses") or [] + document = json.loads(record_path.read_text()) + if not isinstance(document, dict): + raise ValueError("the record is not a JSON object") + records = document.get("escapedProcesses") or [] + if not isinstance(records, list): + raise ValueError("escapedProcesses is not a list") except (OSError, ValueError) as error: + # Unreadable is unsure, not clear: wait and name the file to fix. reasons.append(f"cannot read retained processes in {record_path}: {error}") continue for record in records: - if record.get("outcome") != "retained": + if isinstance(record, dict) and record.get("outcome") not in ("retained", None): + continue + fields = [record.get(key) for key in ("pid", "start_us", "uid")] if isinstance(record, dict) else [] + if (not isinstance(record, dict) or record.get("outcome") != "retained" + or not all(type(value) is int for value in fields) or fields[0] <= 0): + reasons.append(f"malformed retained process record in {record_path}: {record!r}") continue current = kernel_identity(record["pid"]) if (current is not None and not current["zombie"] @@ -619,9 +630,10 @@ def snapshot(self, deadline): return list(current.values()), complete def retained(self, identities): + # Every proven identity still alive is recorded, including one whose cached group is + # still the payload's: without a record, a later admission could not wait for it. for identity in identities: - if identity_key(identity) in self.escaped: - self.record(identity, "retained") + self.record(identity, "retained") return "; ".join(f"PID {p['pid']} ({p['comm']}) start {p['start']} " f"start_us {p['start_us']} uid {p['uid']}" for p in identities) diff --git a/tools/qa-resource/test_qa_resource.py b/tools/qa-resource/test_qa_resource.py index 43e74596e..07d2342c7 100644 --- a/tools/qa-resource/test_qa_resource.py +++ b/tools/qa-resource/test_qa_resource.py @@ -1298,6 +1298,54 @@ def test_candidate_disappearance_is_settled_or_incomplete(self): patch.object(qa, "process_env", side_effect=OSError(qa.errno.ESRCH, "gone")): self.assertEqual(qa.owned_processes(job, 99), []) + def test_retained_records_block_only_a_matching_live_execution(self): + uid = os.getuid() + with tempfile.TemporaryDirectory() as temp: + jobs = Path(temp) + def job(name, document): + (jobs / name).mkdir() + (jobs / name / "job.json").write_text(document if isinstance(document, str) else json.dumps(document)) + retained = lambda pid, **changes: dict(dict(pid=pid, start_us=pid * 10, uid=uid, comm="helper", + outcome="retained"), **changes) + job("live", dict(escapedProcesses=[retained(201), dict(retained(202), outcome="exited")])) + job("gone", dict(escapedProcesses=[retained(203), retained(204), retained(205)])) + job("self", dict(escapedProcesses=[retained(206)])) + job("array", "[1, 2]") + job("partial", '{"escapedProcesses": [') + job("not-list", dict(escapedProcesses={"pid": 207})) + job("bad-fields", dict(escapedProcesses=[dict(retained(208), start_us="2080"), {"outcome": "retained"}, "x"])) + kernel = {201: dict(start_us=2010, uid=uid, zombie=False), + 203: dict(start_us=2031, uid=uid, zombie=False), # PID reused. + 204: dict(start_us=2040, uid=uid + 1, zombie=False), # Another uid. + 205: dict(start_us=2050, uid=uid, zombie=True), + 206: dict(start_us=2060, uid=uid, zombie=False), + 208: dict(start_us=2080, uid=uid, zombie=False)} + with patch.object(qa, "kernel_identity", side_effect=lambda pid: dict(kernel[pid], pid=pid) if pid in kernel else None), \ + patch.object(qa.os, "kill", side_effect=AssertionError("admission signalled a PID")): + pids, reasons = qa.retained_blockers(jobs, "self") + self.assertEqual(pids, [201]) + text = "\n".join(reasons) + self.assertIn("retained process 201 (helper) from job live is still alive", text) + for name in ("array", "partial", "not-list"): + self.assertIn(f"cannot read retained processes in {jobs / name / 'job.json'}", text) + self.assertEqual(text.count(f"malformed retained process record in {jobs / 'bad-fields' / 'job.json'}"), 3) + self.assertEqual(len(reasons), 7) + + def test_live_identity_in_the_payload_group_is_recorded_retained(self): + member = dict(pid=125, pgid=99, ppid=1, stat="S", uid=os.getuid(), start_us=125, start="date", + comm="member", proof="token") + reports = [] + owned, clock = qa.OwnedProcesses({}, 99, report=lambda identity, outcome: reports.append((identity["pid"], outcome))), [0.0] + def sleep(seconds): + clock[0] += seconds + .001 + with patch.object(owned, "snapshot", return_value=([member], True)), \ + patch.object(qa, "group_has_members", return_value=False), \ + patch.object(qa.time, "monotonic", side_effect=lambda: clock[0]), \ + patch.object(qa.time, "sleep", side_effect=sleep): + with self.assertRaisesRegex(RuntimeError, "retained: PID 125 \\(member\\)"): + qa.confirm_owned_exit(Mock(pid=99), owned, .5) + self.assertEqual(reports, [(125, "retained")]) + def test_confirmation_needs_two_complete_empty_passes_after_every_reset(self): previous = dict(pid=123, start_us=123, uid=os.getuid(), pgid=123, start="date", comm="python", proof="token")