mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
fix(cpg): reclaim the scratch tree, and sweep what earlier runs abandoned
Every build allocated a temp directory for the graph, its request files and
joern's own working copy of the CPG. Nothing ever reclaimed it. A day of
scanning left 1,055 directories and 355MB in /tmp on a disk already at 95%,
and the runs that leaked hardest were the ones that FAILED -- which is exactly
backwards, since a failed scan has the least to show for the space it took.
Two mechanisms, because they cover different deaths:
* `CpgResult.cleanup` disposes of the tree, and `scan_repository` calls it
when the graph has answered everything it is going to. Never raises: a scan
that found things must not fail while tidying up.
* A sweep at build time removes trees older than six hours. A process killed
mid-scan never runs its cleanup -- which is how most of the 1,055 died --
so the disposal alone is not enough. Only directories under the system temp
root carrying this module's own prefix are touched, and a scan running in
another process right now is left alone.
Note for the record what was NOT a product leak: 6.3GB of /tmp/joern-cli-*
was three copies of the Joern CLI I made by hand during today's container
experiments, and 63MB of workspace/ in the repo came from running joern from
the repo root instead of a scratch directory. The product already runs joern
with cwd set to the scratch tree, so its workspace is contained and now
disposed with it. Both are cleaned up; neither was the code's fault, and
conflating my litter with its leak would have hidden the real one.
Disk went from 6.1GB free to 19GB.
857 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
37eca758d4
commit
f496f41be1
@@ -27,6 +27,7 @@ import shutil
|
|||||||
import signal
|
import signal
|
||||||
import subprocess
|
import subprocess
|
||||||
import tempfile
|
import tempfile
|
||||||
|
import time
|
||||||
from collections.abc import Callable, Mapping, Sequence
|
from collections.abc import Callable, Mapping, Sequence
|
||||||
from dataclasses import dataclass, field
|
from dataclasses import dataclass, field
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
@@ -188,6 +189,10 @@ class CpgResult:
|
|||||||
# remainder is a scan of LESS CODE THAN IT WAS ASKED ABOUT, and silence about those files is the same
|
# remainder is a scan of LESS CODE THAN IT WAS ASKED ABOUT, and silence about those files is the same
|
||||||
# quiet failure as an empty query result. The names travel with the CPG so the driver can say so.
|
# quiet failure as an empty query result. The names travel with the CPG so the driver can say so.
|
||||||
unparsed: tuple[str, ...] = ()
|
unparsed: tuple[str, ...] = ()
|
||||||
|
# Removes the scratch tree this CPG lives in. A build allocates a temp directory holding the graph, its
|
||||||
|
# request files and joern's own workspace copy; nothing reclaimed it, and a day of scanning left 1,055
|
||||||
|
# directories and 355MB behind on a disk that was already at 95%. The driver disposes in a `finally`.
|
||||||
|
cleanup: Callable[[], None] | None = None
|
||||||
|
|
||||||
|
|
||||||
class CpgBackend(Protocol):
|
class CpgBackend(Protocol):
|
||||||
@@ -236,8 +241,10 @@ class JoernBackend:
|
|||||||
parse = shutil.which("joern-parse")
|
parse = shutil.which("joern-parse")
|
||||||
if parse is None and os.environ.get("OPENULTRASAST_JOERN_PROBE", "").strip().lower() not in {"1", "on", "true", "yes"}:
|
if parse is None and os.environ.get("OPENULTRASAST_JOERN_PROBE", "").strip().lower() not in {"1", "on", "true", "yes"}:
|
||||||
return None
|
return None
|
||||||
scratch = Path(tempfile.mkdtemp(prefix="ousast-cpg-"))
|
_sweep_stale_scratch()
|
||||||
|
scratch = Path(tempfile.mkdtemp(prefix=SCRATCH_PREFIX))
|
||||||
cpg_path = scratch / "cpg.bin"
|
cpg_path = scratch / "cpg.bin"
|
||||||
|
dispose = lambda: shutil.rmtree(scratch, ignore_errors=True) # noqa: E731 -- one expression, named
|
||||||
|
|
||||||
# THE PRECONDITION. Before anything is built, make the toolchain open one file from the tree and say
|
# THE PRECONDITION. Before anything is built, make the toolchain open one file from the tree and say
|
||||||
# how big it is. Every instrument failure this project has had takes the same shape -- the input was
|
# how big it is. Every instrument failure this project has had takes the same shape -- the input was
|
||||||
@@ -269,6 +276,7 @@ class JoernBackend:
|
|||||||
run=lambda query, params: self.query_across(shards, query, params),
|
run=lambda query, params: self.query_across(shards, query, params),
|
||||||
run_batch=lambda query, requests: self.query_batch_across(shards, query, requests),
|
run_batch=lambda query, requests: self.query_batch_across(shards, query, requests),
|
||||||
unparsed=unparsed,
|
unparsed=unparsed,
|
||||||
|
cleanup=dispose,
|
||||||
)
|
)
|
||||||
command = [parse or "joern-parse", self._heap_flag(), str(root), "--output", str(cpg_path)]
|
command = [parse or "joern-parse", self._heap_flag(), str(root), "--output", str(cpg_path)]
|
||||||
completed = self._run(command, timeout=self.build_timeout, cwd=scratch)
|
completed = self._run(command, timeout=self.build_timeout, cwd=scratch)
|
||||||
@@ -287,6 +295,7 @@ class JoernBackend:
|
|||||||
run=lambda query, params: self.query(cpg_path, query, params),
|
run=lambda query, params: self.query(cpg_path, query, params),
|
||||||
run_batch=lambda query, requests: self.query_batch(cpg_path, query, requests),
|
run_batch=lambda query, requests: self.query_batch(cpg_path, query, requests),
|
||||||
unparsed=unparsed,
|
unparsed=unparsed,
|
||||||
|
cleanup=dispose,
|
||||||
)
|
)
|
||||||
|
|
||||||
def _files_with_frontend_defect(self, root: Path, language: str) -> tuple[str, ...]:
|
def _files_with_frontend_defect(self, root: Path, language: str) -> tuple[str, ...]:
|
||||||
@@ -692,6 +701,33 @@ class JoernBackend:
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
SCRATCH_PREFIX = "ousast-cpg-"
|
||||||
|
# How long an abandoned scratch tree may survive. Long enough that a scan running in another process is never
|
||||||
|
# disturbed, short enough that a machine scanning all day does not fill its disk.
|
||||||
|
SCRATCH_MAX_AGE_SECONDS = 6 * 60 * 60
|
||||||
|
|
||||||
|
|
||||||
|
def _sweep_stale_scratch() -> None:
|
||||||
|
"""Remove scratch trees older than `SCRATCH_MAX_AGE_SECONDS`, best effort and never fatal.
|
||||||
|
|
||||||
|
Belt as well as braces: `cleanup` disposes of a scan's own tree, but a process killed mid-scan -- which
|
||||||
|
is how most of them died here -- never runs its `finally`. Only directories under the system temp root
|
||||||
|
carrying this module's own prefix are touched.
|
||||||
|
"""
|
||||||
|
root = Path(tempfile.gettempdir())
|
||||||
|
cutoff = time.time() - SCRATCH_MAX_AGE_SECONDS
|
||||||
|
try:
|
||||||
|
candidates = list(root.glob(f"{SCRATCH_PREFIX}*"))
|
||||||
|
except OSError:
|
||||||
|
return
|
||||||
|
for path in candidates:
|
||||||
|
try:
|
||||||
|
if path.is_dir() and path.stat().st_mtime < cutoff:
|
||||||
|
shutil.rmtree(path, ignore_errors=True)
|
||||||
|
except OSError: # noqa: PERF203 -- one unreadable entry must not stop the sweep
|
||||||
|
continue
|
||||||
|
|
||||||
|
|
||||||
def _terminate_group(process: subprocess.Popen[str]) -> 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."""
|
"""SIGTERM the process group, then SIGKILL what is left. Never raises: this is cleanup, not logic."""
|
||||||
for signal_number in (signal.SIGTERM, signal.SIGKILL):
|
for signal_number in (signal.SIGTERM, signal.SIGKILL):
|
||||||
|
|||||||
@@ -277,6 +277,9 @@ def scan_repository(
|
|||||||
scanned = len(judged)
|
scanned = len(judged)
|
||||||
|
|
||||||
findings = _ordered(_deduplicated(collected))
|
findings = _ordered(_deduplicated(collected))
|
||||||
|
# The graph has answered everything it is going to. Its scratch tree holds the CPG, the request files and
|
||||||
|
# joern's own working copy -- tens of megabytes per scan -- and nothing else reclaims it.
|
||||||
|
_dispose(cpg)
|
||||||
return ModelScanResult(
|
return ModelScanResult(
|
||||||
findings=findings,
|
findings=findings,
|
||||||
by_rung=_tally(findings),
|
by_rung=_tally(findings),
|
||||||
@@ -293,6 +296,16 @@ def scan_repository(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _dispose(cpg: object) -> None:
|
||||||
|
"""Release a built CPG's scratch tree. Never raises: cleanup must not turn a good scan into a failure."""
|
||||||
|
cleanup = getattr(cpg, "cleanup", None)
|
||||||
|
if callable(cleanup):
|
||||||
|
try:
|
||||||
|
cleanup()
|
||||||
|
except Exception as exc: # noqa: BLE001 -- a scan that found things must not fail while tidying up
|
||||||
|
logger.warning("could not remove the cpg scratch directory: %s", exc)
|
||||||
|
|
||||||
|
|
||||||
def _build(backend: Any, root: Path, language: str) -> Any:
|
def _build(backend: Any, root: Path, language: str) -> Any:
|
||||||
"""Build through the backend, passing the language when the backend can use it."""
|
"""Build through the backend, passing the language when the backend can use it."""
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -55,6 +55,7 @@ def test_the_seam_imports_nothing_outside_the_standard_library() -> None:
|
|||||||
"signal",
|
"signal",
|
||||||
"subprocess",
|
"subprocess",
|
||||||
"tempfile",
|
"tempfile",
|
||||||
|
"time",
|
||||||
"dataclasses",
|
"dataclasses",
|
||||||
"pathlib",
|
"pathlib",
|
||||||
"typing",
|
"typing",
|
||||||
@@ -725,3 +726,62 @@ def test_a_timed_out_query_kills_the_jvm_not_just_the_script(tmp_path: Path) ->
|
|||||||
with contextlib.suppress(ProcessLookupError, PermissionError):
|
with contextlib.suppress(ProcessLookupError, PermissionError):
|
||||||
os.kill(child, 9)
|
os.kill(child, 9)
|
||||||
assert not alive, "the grandchild JVM must die with the script, or every timeout leaks a gigabyte"
|
assert not alive, "the grandchild JVM must die with the script, or every timeout leaks a gigabyte"
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_built_cpg_can_dispose_of_its_scratch_tree(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
"""Every build allocates a temp directory for the graph, its request files and joern's own working copy.
|
||||||
|
|
||||||
|
Nothing reclaimed it. A day of scanning left 1,055 directories and 355MB behind, on a disk already at
|
||||||
|
95%, and the scans that leaked hardest were the ones that failed -- which is the wrong way round.
|
||||||
|
"""
|
||||||
|
import subprocess
|
||||||
|
|
||||||
|
from openultrasast.cpg.backend import JoernBackend
|
||||||
|
|
||||||
|
(tmp_path / "a.php").write_text("<?php function a() {}")
|
||||||
|
|
||||||
|
def runner(command, **kwargs): # type: ignore[no-untyped-def]
|
||||||
|
if "-r" in command:
|
||||||
|
return subprocess.CompletedProcess(args=command, returncode=0, stdout="", stderr="")
|
||||||
|
if "--script" in command:
|
||||||
|
body = '---OUSAST-CPG-BEGIN---\n{"files": "1", "methods": "1"}\n---OUSAST-CPG-END---\n'
|
||||||
|
return subprocess.CompletedProcess(args=command, returncode=0, stdout=body, stderr="")
|
||||||
|
Path(command[command.index("-o") + 1]).write_text("cpg")
|
||||||
|
return subprocess.CompletedProcess(args=command, returncode=0, stdout="", stderr="")
|
||||||
|
|
||||||
|
monkeypatch.setenv("OPENULTRASAST_JOERN_PROBE", "on")
|
||||||
|
monkeypatch.setattr("shutil.which", lambda name: f"/opt/bin/{name}")
|
||||||
|
|
||||||
|
result = JoernBackend(runner=runner).build(tmp_path, language="php")
|
||||||
|
assert result is not None and callable(result.cleanup)
|
||||||
|
|
||||||
|
scratch = result.cpg_path.parent
|
||||||
|
assert scratch.is_dir()
|
||||||
|
result.cleanup()
|
||||||
|
assert not scratch.exists(), "the scratch tree is gone once the caller is done with the graph"
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_sweep_removes_only_old_scratch_and_only_ours(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
"""A process killed mid-scan never runs its cleanup, which is how most of the 1,055 died. The sweep is
|
||||||
|
the backstop -- and it must not touch a scan running in another process, nor anything that is not ours."""
|
||||||
|
import os
|
||||||
|
import time
|
||||||
|
|
||||||
|
from openultrasast.cpg.backend import SCRATCH_MAX_AGE_SECONDS, SCRATCH_PREFIX, _sweep_stale_scratch
|
||||||
|
|
||||||
|
monkeypatch.setattr("tempfile.gettempdir", lambda: str(tmp_path))
|
||||||
|
old = tmp_path / f"{SCRATCH_PREFIX}old"
|
||||||
|
fresh = tmp_path / f"{SCRATCH_PREFIX}fresh"
|
||||||
|
foreign = tmp_path / "someone-elses-work"
|
||||||
|
for path in (old, fresh, foreign):
|
||||||
|
path.mkdir()
|
||||||
|
(path / "cpg.bin").write_text("x")
|
||||||
|
stale = time.time() - SCRATCH_MAX_AGE_SECONDS - 60
|
||||||
|
os.utime(old, (stale, stale))
|
||||||
|
os.utime(foreign, (stale, stale))
|
||||||
|
|
||||||
|
_sweep_stale_scratch()
|
||||||
|
|
||||||
|
assert not old.exists(), "an abandoned tree older than the ceiling is reclaimed"
|
||||||
|
assert fresh.is_dir(), "a scan running right now is left alone"
|
||||||
|
assert foreign.is_dir(), "and nothing outside this module's own prefix is ever touched"
|
||||||
|
|||||||
Reference in New Issue
Block a user