Skip to content

fix(glob): bound slow-path gitignore matching memory#1380

Open
Simon (simonhj) wants to merge 3 commits into
v1.xfrom
simon/glob-content-dedup-v1x
Open

fix(glob): bound slow-path gitignore matching memory#1380
Simon (simonhj) wants to merge 3 commits into
v1.xfrom
simon/glob-content-dedup-v1x

Conversation

@simonhj

@simonhj Simon (simonhj) commented Jun 26, 2026

Copy link
Copy Markdown

socket fix / socket scan could abort with a heap-OOM SIGABRT on large monorepos. On the slow path (taken when any gitignore or projectIgnore pattern is negated), globWithGitIgnore built one ignore instance holding every nested .gitignore's cwd-anchored patterns and tested each streamed path against it; the first match call JIT-compiled all of them into V8 code-space, which is capped near 250-300MB regardless of --max-old-space-size.

This reworks only that slow path: one matcher per .gitignore, built from its raw lines and deduped by content, applied along each candidate's ancestor chain. A repo where N packages share one boilerplate .gitignore now compiles a single matcher instead of N, so compiled-regex memory is bounded by the number of distinct .gitignore contents rather than file count. The no-negation fast path is unchanged.

Because the slow path now applies each .gitignore relative to its own directory, its nested-gitignore matching is more git-faithful than the prior cwd-anchored translation: a bare filename matches at any depth, and a file under an excluded directory is not re-included by a deeper negation. Both verified against git check-ignore. Built-in ignored directories and discovered virtualenvs are still pruned on the slow path, and scan targets outside cwd no longer throw.

Tested: the existing glob suite plus regression coverage for the OOM (a 300-package tree), the two nested-gitignore behaviors, and slow-path exclusion of venvs and built-in ignored dirs.


Note

Medium Risk
Changes core file-discovery ignore logic on the negated-gitignore path, which can alter which files are scanned versus the prior global matcher, though behavior is tightened toward git and covered by new tests.

Overview
Fixes heap OOM / SIGABRT on large monorepos during socket fix / socket scan when any gitignore (or project ignore) line is negated (!), which forces the streaming slow path.

On that path, globWithGitIgnore no longer builds one global ignore instance from every nested .gitignore flattened into cwd-anchored patterns (which could JIT-compile hundreds of thousands of regexes into V8 code-space). It now compiles one matcher per distinct .gitignore body, caches matchers by content, and walks each candidate’s ancestor directories so shared boilerplate across hundreds of packages stays bounded. The no-negation fast path is unchanged.

Slow-path matching is also more git-like: patterns apply relative to each .gitignore’s directory, parent directory excludes block deeper ! re-includes, and built-in / venv pruning still goes through fast-glob’s ignore list. Adds regression tests for code-space growth, nested ignore semantics, and venv/coverage exclusion on the slow path.

Reviewed by Cursor Bugbot for commit d7463cb. Configure here.

Scanning a large monorepo could abort with a heap-OOM SIGABRT. On the slow path (any `\!` negation present), globWithGitIgnore built one `ignore` instance holding every nested .gitignore's cwd-anchored patterns; its first match call JIT-compiled all of them into V8 code-space (capped near 250-300MB regardless of --max-old-space-size) and crashed before the scan ran.

Rework only that slow path: build one matcher per .gitignore from its raw lines, dedup identical files by content so a repo of N packages sharing one boilerplate .gitignore compiles a single matcher instead of N, and apply them along each candidate's ancestor chain. A 300-package tree now grows code-space ~1MB instead of crossing the cliff. The no-negation fast path is left exactly as before.

Because the slow path now applies each .gitignore relative to its own directory, its nested-gitignore matching is also more git-faithful than the old cwd-anchored translation (a bare filename matches at any depth; a file under an excluded directory is not re-included). This is intentional and verified against `git check-ignore`; the fast path keeps the prior semantics. Also guards against scan targets outside cwd, which the old slow path threw on.
@simonhj

Copy link
Copy Markdown
Author

Claude (@claude) review once

@jdalton

Copy link
Copy Markdown
Collaborator

Dug into the slow-path rework with adversarial verification on each concern (including trying hard to refute my own findings). The mechanism is sound and one big worry turned out to be unfounded — details in the fold-outs, most severe first.

🟠 1. The memory bound only holds for byte-identical .gitignore files — the distinct-content monorepo can still OOM

The new code dedups matchers by exact file content: igByContent keys on the raw .gitignore body, so 300 packages sharing one boilerplate body compile a single matcher. That's a real win for the boilerplate case.

The catch: nothing else bounds memory. igByContent and matchersByDir are unbounded Maps with no cap or eviction, and the ignore package (v7.0.5) compiles each rule's regex lazily on first test but memoizes it forever — so over a full walk, every rule in every distinct matcher compiles and stays resident. With N packages whose .gitignore files differ even by a comment or one path (the common real-world shape), the aggregate compiled-regex count is the same as the old single flattened matcher, and the ~250-300MB V8 code-space cliff is still reachable.

The regression test constructs exactly (and only) the favorable case: it writes one gitignoreBody byte-identical to all 300 packages — its own comments say "300k nominal patterns collapse to one compiled matcher."

Fix idea 💡: add a distinct-content variant of the memory test (append a unique line per package) to make the residual exposure visible, and either document the bound honestly ("bounded by distinct contents") or add a real cap — e.g. compile per-scope matchers lazily on first ancestor hit and hold them in a bounded LRU, letting cold scopes recompile rather than accumulate.

🟠 2. A deliberate gitignore-semantics change ships inside this memory fix, and the fast path disagrees with the new slow path

The old slow path matched against one global matcher built from cwd-anchored translated patterns. The new slow path applies each .gitignore relative to its own directory with raw lines — which the PR's own test file calls "more git-faithful than origin's cwd-anchored translation." It is! But it's a behavior change, and two consequences ride along:

  1. No old-vs-new equivalence coverage. The new tests verify the new behavior against git check-ignore; nothing pins where outcomes changed, so scan-scope drift ships silently under a memory-fix title. Concrete change: packages/a/.gitignore containing bare secret.json previously did NOT exclude packages/a/sub/secret.json (anchored translation); now it does (git-correct match-anywhere-below).
  2. Fast/slow divergence. The fast path (no negations anywhere) still uses the old anchored translation. So the same repo state yields different exclusion sets depending on whether any ! line exists anywhere: with a stray !keep in some .gitignore, packages/a/sub/secret.json is excluded; remove it and the file is scanned. Verified with the exact translation code (matchEverywherePrefix is '' for nested bare names on the fast path).

Fix idea 💡: state the semantics change in the PR description/changelog explicitly, and file the fast-path alignment (applying the same per-scope git-faithful matching when translating for fast-glob, or at least a test documenting the known divergence) so the two engines converge deliberately rather than drift.

🟡 Smaller items
  • 🟡 The memory regression test gates on expect(grew).toBeLessThan(80) over v8.getHeapSpaceStatistics() code-space growth without forcing GC first — that's Node-build and CI-machine dependent and a flake risk. A generous margin plus an explicit global.gc() (with --expose-gc) or a relative before/after ratio would be sturdier.
  • 🟡 pathIgnoredByChain rebuilds the ancestor-dirs array from scratch for every prefix of every streamed path (O(depth²) on surviving paths, plus per-call array/string allocations). Not a regression (see the verified note below) but a cheap win: memoize per-directory ignored status in a Map keyed by prefix — most streamed paths share ancestors.
  • 🟡 Trailing-slash projectIgnorePaths entries (like build/) mean directory-only in git semantics on the slow path but get stripped to **/build on the fast path, where they'd also match a file named build — a small real divergence between the two channels (the bare-name case, for the record, agrees on both paths).
  • 🟡 Matchers are built for .gitignore files found inside directories that are themselves gitignored (discovery only prunes built-in ignores) — git never reads those. Mostly harmless due to ancestor short-circuiting, but it inflates the matcher maps against the memory goal.
  • 🟡 The file now has two POSIX-ification idioms: the new code imports normalizePath while ignoreFileLinesToGlobPatterns keeps a hand-rolled .replace(/\\/g, '/'). One helper for both keeps the edge cases (dot-segment collapsing) consistent.
🟢 Verified: the CPU-regression worry does not hold — per-path cost actually drops

I chased the theory that the per-scope chain walk turns the old OOM into a hang, and it refutes cleanly: files under an ignored subtree short-circuit at the shallowest matching prefix (~O(1), not O(depth²)); the non-pruning of gitignored subtrees on the negation path predates this PR (and this PR actually prunes more by adding IGNORED_DIR_PATTERNS and venv globs to fast-glob); and the replaced single ig.ignores() call was O(all patterns) per path — with ~300k flattened patterns, that dwarfs the new per-scope walks. Per-path CPU and memory both improve in the targeted scenario.

Item 1 (distinct-content memory bound) and the equivalence-test half of item 2 (semantics change) are the ones I'd settle before merge — the first because it bounds how much of the original incident this actually fixes, the second because scan-scope changes deserve their own line in the changelog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants