diff --git a/src/openultrasast/cpg/backend.py b/src/openultrasast/cpg/backend.py index c1bf50c..dffead0 100644 --- a/src/openultrasast/cpg/backend.py +++ b/src/openultrasast/cpg/backend.py @@ -24,6 +24,7 @@ import logging import os import re import shutil +import signal import subprocess import tempfile from collections.abc import Callable, Mapping, Sequence @@ -655,13 +656,56 @@ class JoernBackend: return f"-J-Xmx{configured if configured.isdigit() else CPG_HEAP_MB}m" def _run(self, command: list[str], *, timeout: int, cwd: Path | None = None) -> subprocess.CompletedProcess[str] | None: + """Run one Joern command, and on timeout kill the whole PROCESS GROUP rather than the child. + + `joern` and `joern-parse` are shell scripts that launch a JVM as a separate process. Killing the + script -- which is all `subprocess.run(timeout=)` does -- leaves that JVM running, holding its heap, + for as long as the machine lasts. Every `query_failed: timeout` leaked about a gigabyte. + + Measured the hard way: after a day of 300s and 2,400s taint timeouts, four orphaned JVMs three hours + old were still resident and the machine ran out of memory. On a CI runner doing several scans in a + row it would OOM rather than merely degrade -- and it would look like the SCAN needing more memory, + which is the wrong lesson entirely. + """ run = self.runner if self.runner is not None else subprocess.run + if self.runner is not None: # tests inject a fake runner and never spawn anything + try: + return run(command, capture_output=True, text=True, timeout=timeout, check=False, cwd=str(cwd) if cwd else None) + except (OSError, subprocess.SubprocessError): + return None try: - return run(command, capture_output=True, text=True, timeout=timeout, check=False, cwd=str(cwd) if cwd else None) + with subprocess.Popen( # noqa: S603 -- the command is built here, never from user input + command, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + cwd=str(cwd) if cwd else None, + start_new_session=True, # its own process group, so the JVM dies with the script + ) as process: + try: + stdout, stderr = process.communicate(timeout=timeout) + except subprocess.TimeoutExpired: + _terminate_group(process) + return None + return subprocess.CompletedProcess(command, process.returncode, stdout, stderr) except (OSError, subprocess.SubprocessError): return None +def _terminate_group(process: subprocess.Popen[str]) -> None: + """SIGTERM the process group, then SIGKILL what is left. Never raises: this is cleanup, not logic.""" + for signal_number in (signal.SIGTERM, signal.SIGKILL): + try: + os.killpg(os.getpgid(process.pid), signal_number) + except (OSError, ProcessLookupError): + return + try: + process.wait(timeout=10) + return + except subprocess.TimeoutExpired: + continue + + # Every Joern frontend logs a dropped file the same way, at WARN, and then carries on: # # WARN AstCreationPass Failed to process '/abs/path/to/file.php' diff --git a/tests/test_cpg_backend.py b/tests/test_cpg_backend.py index 0ccf22b..3b2c4d5 100644 --- a/tests/test_cpg_backend.py +++ b/tests/test_cpg_backend.py @@ -7,6 +7,7 @@ Nothing here imports a JVM binding, and every test runs with Joern absent. from __future__ import annotations +import contextlib import json import subprocess from pathlib import Path @@ -51,6 +52,7 @@ def test_the_seam_imports_nothing_outside_the_standard_library() -> None: "os", "re", "shutil", + "signal", "subprocess", "tempfile", "dataclasses", @@ -684,3 +686,42 @@ def test_a_file_php2cpg_miscompiles_is_excluded_and_named(tmp_path: Path, monkey build = next(c for c in commands if "--script" not in c) assert "--exclude" in build and any("bad.php" in part for part in build), "and excluded from the build" assert not any("ok.php" in part for part in build if part != str(tmp_path)), "a global at function scope is fine" + + +def test_a_timed_out_query_kills_the_jvm_not_just_the_script(tmp_path: Path) -> None: + """`joern` is a shell script that launches a JVM as a separate process. + + `subprocess.run(timeout=)` kills the script and leaves the JVM resident, holding its heap. Measured the + hard way: after a day of 300s and 2,400s taint timeouts, four orphaned JVMs three hours old were still + using about a gigabyte each and the machine ran out of memory. On CI that is an OOM rather than a + degradation, and it would look like the scan needing more memory -- the wrong lesson entirely. + + Uses a real shell script that backgrounds a child, because the whole bug is about grandchildren. + """ + import os + import time + + from openultrasast.cpg.backend import JoernBackend + + marker = tmp_path / "child-alive" + script = tmp_path / "fake-joern.sh" + script.write_text( + f"#!/bin/sh\n( while true; do echo x > '{marker}'; sleep 0.2; done ) &\necho $! > " + str(tmp_path / "child.pid") + "\nsleep 60\n" + ) + script.chmod(0o755) + + started = time.monotonic() + assert JoernBackend()._run([str(script)], timeout=2) is None, "a timeout reports nothing, never a verdict" + assert time.monotonic() - started < 30, "and it does not wait for the script's own sleep" + + child = int((tmp_path / "child.pid").read_text().strip()) + time.sleep(0.5) + alive = True + try: + os.kill(child, 0) + except ProcessLookupError: + alive = False + if alive: # cleanup, so a failure here does not leak the process it is complaining about + with contextlib.suppress(ProcessLookupError, PermissionError): + os.kill(child, 9) + assert not alive, "the grandchild JVM must die with the script, or every timeout leaks a gigabyte"