Skip to content

fix(extraction): give parse workers a native stack the kernel walkers can't overflow - #1582

Open
apollo600 wants to merge 1 commit into
colbymchenry:mainfrom
apollo600:fix/parse-worker-stack-size
Open

fix(extraction): give parse workers a native stack the kernel walkers can't overflow#1582
apollo600 wants to merge 1 commit into
colbymchenry:mainfrom
apollo600:fix/parse-worker-stack-size

Conversation

@apollo600

Copy link
Copy Markdown

Fixes #1581.

What was happening

codegraph index on a repo containing a deeply nested C/C++ file died with Segmentation fault (core dumped) — the whole CLI process, no error message, no index. Reproduced on llvm/llvm-project, where the trigger is clang/test/Parser/parser_overflow.c (16,384 nested braces).

ccpp::visit_node recurses once per parse-tree level, ~450 bytes of native stack per level. Parse workers were created as new Worker(scriptPath), i.e. on Node's default 4MB worker stack (the main thread gets 8MB), so ~9k levels exhausted it. The overflow is a SIGSEGV inside the addon and never becomes a JS exception, so:

  • parse-worker.ts's catch (which handles memory access out of bounds / out of memory by exiting so the pool respawns) never runs;
  • tryKernelExtractRaw's per-file defer: fallback to the wasm extractor never runs;
  • and because worker threads share the process, the whole codegraph process dies.

Measured, with the worker stack as the only variable:

worker stack 4MB 6MB 8MB 16MB 64MB
parser_overflow.c SIGSEGV SIGSEGV ok ok ok

Nesting depth at 4MB: 8,000 levels ok, 10,000 levels SIGSEGV.

Same file on the wasm path parses fine — CODEGRAPH_KERNEL=0 indexed all 31,608 files of llvm-project in 1m48s. With the kernel on and only that one file excluded: 1m24s, and the node/edge counts match the wasm run except for that file's own 2 nodes.

What this changes

ParseWorkerPool pins resourceLimits: { stackSizeMb: 16 }. A thread stack is reserved lazily, page by page, so this costs nothing on files of ordinary depth.

I want to be explicit that this raises the cliff rather than removing it. The complete fix is a depth cap in the kernel walkers that raises defer:, which routes the file to the wasm extractor that already handles it.

A third thing worth considering separately: when the pool sees a worker exit with a parse in flight, naming that file in the error would turn this class of bug from a bisect into a one-line diagnosis.

Tests

New __tests__/parse-worker-stack.test.ts:

  • the stack constant covers the deepest nesting seen in the wild, with headroom;
  • resourceLimits actually reaches the worker thread, and the partial resourceLimits doesn't silently cap the V8 heap (parse workers legitimately reach ~1.4GB RSS on a large index);
  • the kernel parses a 16,384-deep C file without dying — skipped when no .node is staged, matching kernel-scaffold.test.ts.

Both worker arms run in a child process deliberately: on a regression the parse segfaults, and a segfault in a worker thread would take the test runner down with it instead of reporting a failure. Verified by setting the constant back to 4 — the suite fails cleanly with expected 'SIGSEGV' to be null and the runner survives.

npx tsc --noEmit clean; __tests__/parse-pool.test.ts (17 tests) still passes.

… can't overflow

A C/C++ file with thousands of nested braces segfaulted the whole
`codegraph index` process. The kernel walkers recurse once per parse-tree
level (~450 bytes of native stack per level for c/cpp), and parse workers ran
on Node's default 4MB worker stack, so deep nesting overflowed it. The
overflow is a SIGSEGV inside the addon rather than a JS exception, so neither
parse-worker's catch nor the kernel's per-file `defer:` fallback could see it
— and since worker threads share the process, it took the CLI down with no
diagnostic and no index.

Repro'd on llvm/llvm-project, where `clang/test/Parser/parser_overflow.c`
nests 16,384 braces: SIGSEGV at 4MB and 6MB of worker stack, parses cleanly at
8MB. The pool now pins `resourceLimits.stackSizeMb` to 16MB, which is reserved
lazily and so costs nothing on files of ordinary depth.

This raises the cliff rather than removing it. The complete fix is a depth cap
in the kernel walkers that raises `defer:`, landing such a file on the wasm
extractor — which parses this input correctly today, and is why
`CODEGRAPH_KERNEL=0` was a working workaround.

The regression test runs the parse in a child process on purpose: if this
regresses, the parse segfaults, and a segfault in a worker thread would take
the test runner down instead of reporting a failure.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Native kernel stack-overflows on deeply nested C/C++ files, killing the whole index process (SIGSEGV)

1 participant