mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 11:49:28 +00:00
fix(cpg): a timed-out query must kill the JVM, not just the script
`joern` and `joern-parse` are shell scripts that launch a JVM as a separate process. `subprocess.run(timeout=)` kills the script and leaves the JVM resident, holding its heap, indefinitely. Every `query_failed: timeout` leaked about a gigabyte. Found by running out of memory. After a day of 300s and 2,400s taint timeouts, four orphaned JVMs -- three hours old, ~2GB between them -- were still resident on a 7GB machine, and background tasks started being killed. On a CI runner doing several scans in sequence this is an OOM rather than a degradation, and it would present as "the scan needs more memory", which is the wrong lesson entirely: the scan's own peak is a flat ~1.5GB whether its query runs for five minutes or forty. The command now starts in its own session, and a timeout SIGTERMs the process group and SIGKILLs what survives. An injected test runner is left alone, because those spawn nothing. Tested with a real shell script that backgrounds a child, since the bug is specifically about grandchildren -- and the test reaps that child itself if the assertion fails, rather than leaking the process it is complaining about. 855 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
This commit is contained in:
co-authored by
Claude Opus 5
parent
a895bceefc
commit
5abc6cd6f6
@@ -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'
|
||||
|
||||
Reference in New Issue
Block a user