From c9c06d6f2e182221279ca73ff01a0d754b4f335d Mon Sep 17 00:00:00 2001 From: Moritz Angermann Date: Sun, 18 Oct 2026 07:47:01 +0900 Subject: [PATCH] nix: bound Intel diagnostics without a GNU timeout dependency --- scripts/nix/README.md | 4 +++ scripts/nix/test-trace-intel-ghc.py | 43 ++++++++++++++++++++++++++--- scripts/nix/trace-intel-ghc.py | 42 ++++++++++++++++++++-------- 3 files changed, 74 insertions(+), 15 deletions(-) diff --git a/scripts/nix/README.md b/scripts/nix/README.md index 8f58e97faa..7ca21aa308 100644 --- a/scripts/nix/README.md +++ b/scripts/nix/README.md @@ -385,6 +385,10 @@ sandbox stays enabled. The replay has a 300-second wall-clock/per-derivation limit and a 360-second overall preparation-and-replay budget, plus a five-second termination grace. It adds no compiler flags and preserves the actual compiler arguments and response-file bytes. Compiler queries pass through unchanged. +Each launcher command runs in its own process group. Python enforces the +deadline and kills only that group on expiry, with at most five seconds to reap +the command; GNU `timeout` and Homebrew aliases are not required. Missing Apple +or Nix tools are named explicitly. The recorder loads only into the selected normal SimpleX iOS compiler process. It reports its main executable's UUID, image base and slide, then records SIGBUS diff --git a/scripts/nix/test-trace-intel-ghc.py b/scripts/nix/test-trace-intel-ghc.py index c36a2dfb7e..6b87e2d8c1 100644 --- a/scripts/nix/test-trace-intel-ghc.py +++ b/scripts/nix/test-trace-intel-ghc.py @@ -6,7 +6,7 @@ import importlib.util import io import os from pathlib import Path -import shutil +import signal import sys import tempfile import time @@ -49,7 +49,8 @@ class LauncherTests(unittest.TestCase): return None argv = ["trace-intel-ghc.py", "--repo", folder, "--attribute", target or TRACE.ATTRIBUTE, "--log", str(log)] with patch.dict(os.environ, {"GITHUB_ACTIONS": actions}), patch.object(TRACE.platform, "system", return_value="Darwin"), \ - patch.object(TRACE.platform, "machine", return_value=machine), patch.object(TRACE.shutil, "which", return_value="/existing/tool"), \ + patch.object(TRACE.platform, "machine", return_value=machine), \ + patch.object(TRACE.shutil, "which", side_effect=lambda tool: None if tool in ("timeout", "gtimeout") else "/existing/tool"), \ patch.object(TRACE, "run_step", side_effect=step), patch("sys.argv", argv), \ contextlib.redirect_stdout(io.StringIO()), contextlib.redirect_stderr(io.StringIO()): result = TRACE.main() @@ -99,15 +100,49 @@ class LauncherTests(unittest.TestCase): self.assertEqual(sum("--expr" in call[0] for call in calls), 1) self.assertFalse(any("--file" in call[0] for call in calls)) - @unittest.skipUnless(shutil.which("timeout"), "GNU timeout is required") def test_actual_metadata_output_is_bounded(self): with self.assertRaises(ValueError): TRACE.run_step([sys.executable, "-c", "print('x' * 65536)"], time.monotonic() + 10, 5, True) self.assertEqual(TRACE.run_step([sys.executable, "-c", "print('revision')"], time.monotonic() + 10, 5, True), "revision") + def test_actual_timeout_kills_owned_command_and_its_child(self): + with tempfile.TemporaryDirectory() as folder: + pidfile = Path(folder) / "owned-pids" + grandchild = "import signal,time; signal.signal(signal.SIGTERM,signal.SIG_IGN); time.sleep(60)" + program = ("import os,signal,subprocess,sys,time; " + "signal.signal(signal.SIGTERM,signal.SIG_IGN); " + "p=subprocess.Popen([sys.executable,'-c',sys.argv[2]]); " + "open(sys.argv[1],'w').write(str(os.getpid())+' '+str(p.pid)); time.sleep(60)") + before = time.monotonic() + with self.assertRaisesRegex(TimeoutError, "wall-clock limit"): + TRACE.command_status([sys.executable, "-c", program, str(pidfile), grandchild], 1) + self.assertLess(time.monotonic() - before, 7) + parent, child = map(int, pidfile.read_text().split()) + # The direct child is reaped. An orphaned grandchild can briefly + # remain a zombie on Linux; it must not be running in either case. + for pid in (parent, child): + stopped = False + until = time.monotonic() + 3 + while time.monotonic() < until: + try: + os.kill(pid, 0) + except ProcessLookupError: + stopped = True + break + stat = Path("/proc") / str(pid) / "stat" + if stat.exists() and stat.read_text().split(")", 1)[1].split()[0] == "Z": + stopped = True + break + time.sleep(0.02) + self.assertTrue(stopped, "Owned fixture PID still running: " + str(pid)) + + def test_nonzero_actual_command_remains_a_failure(self): + with self.assertRaisesRegex(RuntimeError, "returned 7"): + TRACE.run_step([sys.executable, "-c", "raise SystemExit(7)"], time.monotonic() + 10, 5) + def test_deadline_and_nix_literal(self): - with patch.object(TRACE.time, "monotonic", return_value=2), patch.object(TRACE.subprocess, "run") as run: + with patch.object(TRACE.time, "monotonic", return_value=2), patch.object(TRACE.subprocess, "Popen") as run: with self.assertRaises(TimeoutError): TRACE.run_step(["nix", "build"], 1, 300) run.assert_not_called() diff --git a/scripts/nix/trace-intel-ghc.py b/scripts/nix/trace-intel-ghc.py index a3309ce3b3..750dd8b5f2 100644 --- a/scripts/nix/trace-intel-ghc.py +++ b/scripts/nix/trace-intel-ghc.py @@ -8,6 +8,7 @@ from pathlib import Path import platform import re import shutil +import signal import subprocess import sys import tempfile @@ -34,32 +35,50 @@ def nix_string(value): return '"' + str(value).replace("\\", "\\\\").replace('"', '\\"').replace("${", "\\${") + '"' +def command_status(arguments, seconds, **streams): + # A fresh session gives this command and its descendants a group owned by + # this launcher. macOS need not provide GNU timeout (or a Homebrew alias). + child = subprocess.Popen(list(map(str, arguments)), start_new_session=True, **streams) + try: + return child.wait(timeout=seconds) + except subprocess.TimeoutExpired: + # wait() has not reaped this PID, so its group cannot have been reused. + # Kill the whole owned group, including children that ignore SIGTERM. + try: + os.killpg(child.pid, signal.SIGKILL) + except ProcessLookupError: + pass + try: + child.wait(timeout=5) + except subprocess.TimeoutExpired as error: + raise TimeoutError("Owned diagnostic PID " + str(child.pid) + + " was not reaped within five seconds after SIGKILL") from error + raise TimeoutError("Diagnostic command reached its wall-clock limit") from None + + def run_step(arguments, deadline, seconds, capture=False): remaining = min(seconds, int(deadline - time.monotonic())) if remaining < 1: raise TimeoutError("Diagnostic wall-clock limit reached") - # GNU timeout owns the process group it creates. Each operation has a - # bounded wall clock and a five-second kill grace, including compilation. - command = ["timeout", "--kill-after=5s", str(remaining), *map(str, arguments)] if capture: # Keep subprocess output out of memory until its size is known. These # three metadata queries have tiny successful outputs; errors remain # visible through a bounded stderr suffix. with tempfile.TemporaryFile() as output, tempfile.TemporaryFile() as errors: - result = subprocess.run(command, stdout=output, stderr=errors, check=False) - if result.returncode: + status = command_status(arguments, remaining, stdout=output, stderr=errors) + if status: errors.seek(0, os.SEEK_END) errors.seek(max(0, errors.tell() - 4096)) print(errors.read(4096).decode(errors="replace"), file=sys.stderr, flush=True) - raise RuntimeError("Diagnostic command returned " + str(result.returncode)) + raise RuntimeError("Diagnostic command returned " + str(status)) output.seek(0) data = output.read(4097) if len(data) > 4096: raise ValueError("Diagnostic command exceeded the metadata output limit") return data.decode().strip() - result = subprocess.run(command, check=False) - if result.returncode: - raise RuntimeError("Diagnostic command returned " + str(result.returncode)) + status = command_status(arguments, remaining) + if status: + raise RuntimeError("Diagnostic command returned " + str(status)) return None @@ -85,8 +104,9 @@ def main(): if expected is None: print("Skipping Intel GHC diagnostic: no unique observed SimpleX SIGBUS component.", flush=True) return 0 - if not all(shutil.which(tool) for tool in ("timeout", "xcrun", "git", "nix")): - raise ValueError("A required existing diagnostic tool is unavailable") + missing = [tool for tool in ("xcrun", "git", "nix") if not shutil.which(tool)] + if missing: + raise ValueError("Required existing diagnostic tools unavailable: " + ", ".join(missing)) repo = args.repo.resolve() revision = run_step(["git", "-C", repo, "rev-parse", "HEAD"], deadline, 10, True) if re.fullmatch(r"[0-9a-f]{40}", revision) is None: