From c6445096eb3fdfa70bc1d13995918e0cbd783a38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sat, 11 Jul 2026 18:07:08 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20stop=20Napi::Error=20SIGABRT=20on=20anal?= =?UTF-8?q?yze=20=E2=80=94=20index=20C++=20type=20lookups,=20terminate=20w?= =?UTF-8?q?orkers=20only=20at=20JS-safe=20points=20(#2432)=20(#2436)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 2 + ...plan-fix-2432-napi-abort-cpp-extraction.md | 228 ++++++++++++++++ gitnexus/README.md | 2 + gitnexus/src/cli/i18n/en.ts | 2 +- gitnexus/src/cli/i18n/zh-CN.ts | 2 +- .../core/ingestion/languages/cpp/captures.ts | 247 +++++++++++++----- .../src/core/ingestion/workers/worker-pool.ts | 117 ++++++++- .../cpp-captures-typeclass-benchmark.test.ts | 105 ++++++++ .../test/unit/cpp-captures-budget.test.ts | 62 +++++ .../worker-pool-cumulative-timeout.test.ts | 4 + .../unit/worker-pool-timeout-retire.test.ts | 110 +++++++- 11 files changed, 801 insertions(+), 80 deletions(-) create mode 100644 docs/plans/2026-07-11-gitnexus-plan-fix-2432-napi-abort-cpp-extraction.md create mode 100644 gitnexus/test/integration/cpp-captures-typeclass-benchmark.test.ts create mode 100644 gitnexus/test/unit/cpp-captures-budget.test.ts diff --git a/README.md b/README.md index 4c3e66652..b4896ce36 100644 --- a/README.md +++ b/README.md @@ -445,6 +445,8 @@ Most `analyze` knobs are also CLI flags (`--workers`, `--worker-timeout`, `--max | `GITNEXUS_WORKER_MAX_RESPAWNS_PER_SLOT` | `3` | Max replacement spawns per worker slot before the slot is dropped from the active rotation. Bounds respawn loops on a chronically-crashing slot. | Hosts where a flaky worker should retry more (raise) or fail-fast (lower) before the slot is dropped. | | `GITNEXUS_WORKER_MAX_CUMULATIVE_TIMEOUT_MS` | `5 × subBatchTimeoutMs` | Total retry wall-time budget per job before quarantining. Combined with `timeoutBackoffFactor`, prevents exponentially-growing retries from stalling for hours. | Slow files that legitimately need long total retry windows; lower to fail-fast on stalls. | | `GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD`| `max(3, poolSize)` | Per-slot consecutive deaths before the pool's circuit breaker trips. After tripping, every subsequent dispatch rejects until a fresh pool is created. | Hosts where a SIGSEGV-prone native grammar should trip the breaker sooner; CI runners that should fail loudly. | +| `GITNEXUS_WORKER_SHUTDOWN_DRAIN_MS` | `30000` | Max wait at pool shutdown for a retired worker still inside native code. The worker is terminated at its next JS-safe point instead of mid-native-call (which aborts the whole process with `Napi::Error`, #2432); on expiry it is left running, unref'd, and terminated when it surfaces. | Shutdown latency matters more than draining a wedged worker (lower), or a legitimately-slow native grammar needs longer to surface (raise). | +| `GITNEXUS_CPP_CAPTURE_BUDGET_MS` | `20000` | Per-file wall-clock budget for C++ capture extraction. On breach the file keeps the captures accumulated so far and logs a warning — the worker returns to JS instead of stalling in native-heavy loops (#2432). `0` expires immediately. | Pathological generated C++ that still exceeds the budget after the indexed lookups; raise for completeness, lower to fail-fast. | | `GITNEXUS_CHUNK_BYTE_BUDGET` | `2097152` (2 MB) | Chunk boundary used for cache-key composition and dispatch. Smaller = finer-grained cache hits but more dispatch overhead. | Tuning incremental-analyze cache behavior on monorepos. | | `GITNEXUS_NO_GITIGNORE` | unset | When set, skips `.gitignore` parsing. `.gitnexusignore` is still honored. | Indexing a repo whose `.gitignore` excludes files you actually want indexed (e.g., generated code committed for cross-repo lookup). | | `GITNEXUS_SKIP_OPTIONAL_GRAMMARS` | unset | When `=1` strictly, skips the vendored grammar materialize for `tree-sitter-dart`, `tree-sitter-proto`, `tree-sitter-swift`, and `tree-sitter-kotlin` at install time (and the Dart/Proto source builds). Those four won't be parsed; the install still succeeds. | Installing on a host without a C++ toolchain or where the vendored prebuilds don't match; willing to skip Dart/Proto/Swift/Kotlin parsing. | diff --git a/docs/plans/2026-07-11-gitnexus-plan-fix-2432-napi-abort-cpp-extraction.md b/docs/plans/2026-07-11-gitnexus-plan-fix-2432-napi-abort-cpp-extraction.md new file mode 100644 index 000000000..702d4c3d3 --- /dev/null +++ b/docs/plans/2026-07-11-gitnexus-plan-fix-2432-napi-abort-cpp-extraction.md @@ -0,0 +1,228 @@ +# GitNexus Engineering Plan + +> Task: Fix #2432 — `analyze` aborts with `Napi::Error` SIGABRT on triton-lang/triton: pathological C++ capture extraction triggers worker timeouts, then worker termination lands mid-native-call. +> Evidence verified at commit 737a8cdb; GitNexus index refreshed this session (`node .gitnexus/run.cjs analyze --index-only --pdg`, 230,253 nodes / 487,362 edges). Deepened same session: assumption A1 empirically refuted (see §5a/§12); §6-C redesigned accordingly. (Note: the MCP context resource still displays a stale banner after refresh — tools serve the refreshed data; PDG queries succeed. Cosmetic cache issue, recorded in §12.) + +## 1. Objective + +`gitnexus analyze` on triton (repro: `GITNEXUS_MAX_FILE_SIZE=5120 … analyze --worker-timeout 60`) must complete without SIGABRT. Two stacked defects, both fixed: + +1. **Perf (trigger):** C++ scope-capture extraction is O(calls × args × treeSize) per file — `lib/Dialect/TritonInstrument/IR/FunctionBuilder.cpp` (194 KB, parses in 46 ms) burns **151 s** in the worker; `hip_prof_str.h` (`.h` → cpp provider) burns 116 s. Measured via `--cpu-prof` on the real dist worker `[verified]`. +2. **Crash (abort):** terminating a worker thread that is inside an N-API call — whether via pool shutdown, breaker trip, **or plain process exit** — makes the pending `Napi::Error` escape as an uncaught C++ exception → `std::terminate` → SIGABRT kills the whole CLI (workers are `worker_threads`, shared process). Reproduced 2/2 on origin/main, exit 134 `[verified]`; process-exit variant reproduced directly `[verified]` (§5a). + +Acceptance criteria: triton repro completes (exit 0); `FunctionBuilder.cpp` extraction drops from ~151 s to sub-second; no worker-pool or cpp-resolver test regressions. + +## 2. Current Behaviour + +Per-file worker flow (`parse-worker.ts` `processFileGroup`): parse → `query.matches` → `extractParsedFile` → provider `emitScopeCaptures` (`emitCppScopeCaptures` for `.cpp`/`.h` — `c-cpp.ts:435` maps both) `[verified]`. + +For every call-expression capture, `inferCppCallArgTypeClasses` (`cpp/captures.ts:929`, driven from `:325`) classifies each identifier argument via: + +- `lookupDeclaredTypeClassForIdentifier` (`:1133`) — linear scan of the enclosing scope's children per identifier `[verified]`; +- `lookupFunctionParameterTypeClass` (`:1182`) + `findEnclosingFunctionParameter` (`:1200`) — walk up + param scan per identifier `[verified]`; +- **`isKnownEnumName` (`:1248`) — walks to the AST root and full-tree DFS for `enum_specifier`, per identifier argument** `[verified]`. CPU profile: 87.7 s of 149.9 s inside it; self-time dominated by tree-sitter N-API accessors (`child`/`childCount`/`type`/`unmarshalNode`) `[verified]`. + +Crash path: idle-timeout/give-up paths all pass `'retire'` → `retireWorkerAfterTimeout` (`worker-pool.ts:1188`) defers terminate until the worker posts `sub-batch-done`/`result`/`error` (the #1848 fix) `[verified]`. But: + +- `parse-impl.ts:1123-1125` `finally { await workerPool?.terminate() }` → pool `terminate` (`worker-pool.ts:2025` awaits) → `terminateTrackedWorkers` (`:971-980`) — terminates every live AND retired worker unconditionally `[verified]`. +- `tripBreaker` (`:1303`) fire-and-forgets the same (`void terminateTrackedWorkers`, `:1312`) on breaker trip — including live workers that may be mid-native-parse `[verified]`. +- These are the ONLY two callers `[verified]` (grep; matches the graph's d=1). +- **Existing test `worker-pool-timeout-retire.test.ts:97` asserts the crash-causing contract**: a never-safe retired mock worker gets `terminateCalls === 1` after `pool.terminate()` (`:114-118`); `:155` asserts the same for breaker trips `[verified]`. Both flip intentionally under this fix. +- Run evidence: process died 12 s after the last retire log with no error-path output; the 2-file mini corpus (retired workers finish before shutdown) exits 0 `[verified]`. + +## 3. Relevant Architecture + +- Shared ingestion pipeline is language-agnostic (AGENTS.md): the fix stays inside the cpp language module (`languages/cpp/captures.ts`) and the generic worker pool; no `LanguageProvider` interface change. +- Precedent for exactly this bug class: Go scope-capture re-walk fix (#1848/#1915), Python (#1918), C++ ADL once-built index (#1990) — `languages/c/captures.ts:39-42` documents the pattern `[verified]`. +- The C provider has its own thin `emitCScopeCaptures` (164 lines, no arg-type-class inference) — not affected `[verified]`. +- Workers cluster (76 symbols, 65% cohesion) is self-contained; depth-3 upstream impact of the shutdown change stays entirely inside it `[graph]`. +- `parse-worker.ts:1362-1369` documents the group-catch trap: a throw escaping per-file processing makes the language-group catch drop every remaining file — any new bail path must be caught per-file, never thrown outward `[verified]`. + +## 4. GitNexus Findings + +- `impact {target: emitCppScopeCaptures, direction: upstream, maxDepth: 2}` → 0 dependents, LOW `[graph]`. **Graph/source discrepancy:** the real consumer is the provider-hook indirection (`c-cpp.ts:495 emitScopeCaptures: emitCppScopeCaptures` → `scope-extractor-bridge.ts:41 extractParsedFile`) which the call graph doesn't model. Source wins; internal signature changes are still safe (all hot functions are file-private). +- `impact {target: terminateTrackedWorkers, direction: upstream}` at depth 2 → d=1: `tripBreaker`, `terminate`; deepened at `maxDepth: 3, summaryOnly` → 13 symbols total (d1:2, d2:7, d3:4), risk LOW, all in the Workers module. Key output: `"direct": 2` — both d=1 dependents modified deliberately in §6-C and source-confirmed `[verified]`. +- Related tests located and read: `test/unit/worker-pool-timeout-retire.test.ts` (mock `TimeoutThenHealthyWorker` harness with `terminateCalls`/`unrefCalls` counters and a `delayed-safe-return` mode — supports the new scenarios without factory changes `[verified]`), `test/integration/resolvers/cpp.test.ts` + `c.test.ts` (golden equivalence gate), `test/integration/cpp-adl-benchmark.test.ts` (GITNEXUS_BENCH-gated; template for the new benchmark) `[verified]`. + +## 5. Statement-Level PDG Findings + +- `pdg_query {mode: controls, target: isKnownEnumName}` (28 edges): the DFS body (`:1253-1262`) is control-dependent only on the trivial `typeName === ''` guard (`:1249`, guard:true) and its own loop conditions — **no memoization or early-exit gate exists**; the full-tree walk runs unconditionally on every call `[graph]`, consistent with source `[verified]`. +- `pdg_query {mode: controls, target: terminateTrackedWorkers}` → 0 edges: straight-line, unconditional termination of both worker lists `[graph]`. The crash fix is precisely "add the missing control dependency" (safe-point gate). +- Performance-mode scan: the hot loop's N-API fan-out (`cur.child(i)` per node per DFS per identifier) is the marshalling hotspot (`unmarshalNode` 15.5 s incl.) `[verified via profile]`. +- Ordering constraint: `lookupDeclaredTypeClassForIdentifier` returns the **first** matching `declaration` in scope-child order, with no position filtering relative to the identifier — the replacement index must preserve first-declaration-wins and must NOT introduce use-before-decl filtering `[verified]`. + +## 5a. Deepen finding — A1 refuted empirically + +Driver test (`exit-with-busy-worker.mjs`, kept in scratchpad): main thread `process.exit(0)` five seconds into the real dist worker's extraction of `FunctionBuilder.cpp`, worker `unref()`d → **process aborts: `terminate called after throwing an instance of 'Napi::Error'`, exit 134** `[verified]`. Node tears down worker environments on process exit through the same terminate path. Consequence: "skip terminating unsafe workers and let the process exit" merely relocates the abort. The shutdown design must instead guarantee workers reach a JS safe point in bounded time before the process exits — this promotes the previously-deferred cooperative extraction deadline into scope (§6-D). + +## 6. Proposed Changes + +**A. Perf root-cause — per-file lookup index in `gitnexus/src/core/ingestion/languages/cpp/captures.ts`.** +Introduce a lazily-built, per-invocation index object created at the top of `emitCppScopeCaptures` and threaded through `inferCppCallArgTypes` / `inferCppCallArgTypeClasses` → the lookup helpers (all file-private; no exported API change): + +- `enumNames: Set` — built by ONE root DFS on first `isKnownEnumName` query (lazy: files with no identifier args pay nothing). `isKnownEnumName` becomes a Set lookup. Behavior-identical: current code matches any `enum_specifier` name anywhere in the translation unit. +- `scopeDecls: Map>` — per-scope declaration map built on first lookup in that scope by one pass over `scope` children, first-declaration-wins (skip existing keys). Replaces the per-identifier linear scans in `lookupDeclaredTypeClassForIdentifier` / `lookupDeclaredTypeForIdentifier` (`:1090-1131`). +- `fnParams: Map>` — same treatment for `findEnclosingFunctionParameter`. + +Complexity: O(treeSize + identifiers) per file. Expected: 151 s → sub-second (parse itself is 46 ms). `classifyCppParameterType` / `normalizeCppTypeText` stay per-hit (cheap; memoizing them changes nothing observable). + +**B. New benchmark test — `gitnexus/test/integration/cpp-captures-typeclass-benchmark.test.ts`** modeled exactly on `cpp-adl-benchmark.test.ts` (`describe.skipIf(!GITNEXUS_BENCH)`): synthetic C++ file scaling call-sites × enums, asserts sub-quadratic scaling of the capture-emit phase. + +**C. Crash fix — safe-point-gated shutdown with bounded drain, `gitnexus/src/core/ingestion/workers/worker-pool.ts`.** (Redesigned after §5a.) + +- **C1 (gate):** extend `RetiredWorkerRecord` with `safeToTerminate`, set exactly where `terminateWhenBackInJs` fires today (`onRetiredMessage` for `sub-batch-done`/`result`/`error`, and `messageerror`). `terminateTrackedWorkers` terminates retired records only when safe; unsafe records keep their armed at-safe-point terminate listener. +- **C2 (bounded drain):** pool `terminate()` awaits unsafe retired records' safe-point terminate up to a cap (`GITNEXUS_WORKER_SHUTDOWN_DRAIN_MS`, default ≈ 30 s — comfortably above D's per-file deadline so the drain converges for the known class). On cap expiry: log a clear diagnostic naming the wedged worker + in-flight file and proceed (residual abort risk at process exit remains for truly-wedged native code, now rare and diagnosed). The breaker path (`tripBreaker:1312`) stays fire-and-forget — it must never block the dispatch rejection; its unsafe records drain when the pipeline's `finally` runs pool `terminate()`. +- **C3 (breaker-path live workers):** on breaker trip, live workers in `busySlots` (`:1154` `[verified]`) are routed through `retireWorkerAfterTimeout` instead of direct `terminate()` — same mid-native abort risk, same cure. Idle live workers terminate directly (parked in the JS event loop; safe). The normal post-parse `terminate()` still direct-terminates live workers — all idle by construction (jobs drained). + +**D. Cooperative extraction deadline (promoted from deferred Q2 by §5a) — `cpp/captures.ts`.** +Bound per-file wall time inside `emitCppScopeCaptures`'s match loop: check `Date.now()` against a soft budget (`GITNEXUS_CPP_CAPTURE_BUDGET_MS`, default ≈ 20 s; post-A one iteration is microseconds, so check granularity of every N=64 matches is ample). On breach: **return** partial captures accumulated so far + `reportWarning` naming the file — never throw (the group-catch trap, §3). This guarantees cpp extraction returns to JS in bounded time, which is what makes C2's drain converge and process exit safe. Generic all-language budget remains a deferred follow-up (§12). + +## 7. Implementation Sequence + +1. **cpp captures index (A).** Build the index type + lazy constructors; convert `isKnownEnumName`, `lookupDeclaredType{Class}ForIdentifier`, `lookupFunctionParameterType{Class}`, `findEnclosingFunctionParameter`; thread from `emitCppScopeCaptures`. Gate: `npx vitest run test/integration/resolvers/cpp.test.ts test/integration/resolvers/c.test.ts` passes unchanged. +2. **Benchmark (B).** Add the GITNEXUS_BENCH-gated benchmark; record before/after in the PR body (before: 151 s / 116 s from this plan). +3. **Extraction deadline (D).** Budget check + partial-return + warning; unit test with a tiny budget forcing the bail (assert warning emitted, remaining files in group still processed). +4. **Worker-pool shutdown safety (C1–C3).** Gate + drain + breaker routing. Update `worker-pool-timeout-retire.test.ts:97` and `:155` (both currently assert the buggy contract) and add: (i) `pool.terminate()` with a never-safe retired worker + tiny drain cap → resolves after cap, `terminateCalls === 0`, diagnostic logged; (ii) retired worker signals safe during drain → terminated, `terminate()` resolves promptly; (iii) breaker trip with busy live worker → retired, not direct-terminated. +5. **End-to-end validation.** Rebuild (`npm run build`); re-run the triton repro → exits 0, `FunctionBuilder.cpp` indexed (not quarantined); mini 2-file corpus completes in seconds; re-run the §5a exit-with-busy-worker driver against the built worker with D's budget lowered → clean exit. + +Steps 1–2 alone de-trigger #2432; 3–4 close the abort class. Each step leaves the tree green. + +## 8. Test Strategy + +- **Update:** `worker-pool-timeout-retire.test.ts:97` + `:155` — expectations flip to the new contract (unsafe ⇒ not terminated at shutdown; terminated at safe point). The mock harness supports this as-is `[verified]`. +- **Add:** benchmark (§6-B); three shutdown cases (§7-4); deadline-bail unit test (§7-3). +- **Regression:** resolver goldens `cpp.test.ts`/`c.test.ts` unchanged (equivalence gate); full `npm run test:unit`; `npm run test:integration` (carries its build via `pretest:integration`). +- **Edge cases:** file with enums but no calls (lazy index never built); duplicate declaration in one scope (first-wins preserved); use-before-decl in scope (still resolved — no position filter); anonymous enums (name-less `enum_specifier` excluded, same as today); breaker trip with mixed busy/idle live workers; drain cap = 0 (immediate proceed); deadline breach mid-file (partial captures kept, group continues). +- **Failure paths:** shutdown never hangs (drain is capped); deadline bail is a warning, never a group-dropping throw (§3 trap). +- **Verification commands (verified to exist):** `npm run build`, `npm run test:unit`, `npm run test:integration`, `GITNEXUS_BENCH=1 npx vitest run test/integration/cpp-captures-typeclass-benchmark.test.ts` — all from `gitnexus/`. + +## 9. Risk and Impact Analysis + +- **d=1 dependents of `terminateTrackedWorkers`** — `tripBreaker` (`:1312`), `terminate` (`:2025`): both modified deliberately; no other callers `[verified]`. Depth-3 radius stays pool-internal (13 symbols, Workers module) `[graph]`. +- **Behavioral-equivalence risk (A):** first-declaration-wins + position-free matching must be preserved (§5). Mitigation: resolver goldens + explicit edge cases. +- **Node identity:** key maps by `SyntaxNode.id` (stable within a tree); wrapper object identity is NOT usable (wrappers are recreated per access — a `WeakMap` would silently fail). +- **Drain-cap tuning (C2 vs D):** drain cap must exceed D's budget or the drain can expire while a worker is legitimately finishing its bailed file — defaults 30 s vs 20 s encode that; both env-tunable, relation asserted in a unit test comment. +- **Residual abort window:** a worker wedged in native code longer than the drain cap still aborts at process exit — now requires non-cpp pathological input (D bounds cpp) and is logged with the culprit file before it can happen. Accepted; full elimination needs child-process workers (out of scope, §12). +- **`emitCppScopeCaptures` consumers:** provider hook only; signature unchanged (D's budget read from env inside the module) — zero external surface. +- **Deadline false positives (D):** 20 s default is ~3 orders of magnitude above post-A extraction cost of the worst observed file; breach ⇒ degraded coverage for that file (warning), never a failed run. +- **Coverage change:** triton's `FunctionBuilder.cpp` was previously quarantined; post-fix it indexes — strictly an improvement. +- **Concurrency:** the new index and deadline state are function-scoped per invocation (per file, per worker thread) — no shared state, no `clearCaches()` interaction. + +## 10. Files Expected to Change + +| File | Symbols | Reason | +|---|---|---| +| `gitnexus/src/core/ingestion/languages/cpp/captures.ts` | `emitCppScopeCaptures`, `inferCppCallArgTypes`, `inferCppCallArgTypeClasses`, `lookupDeclaredType{Class}ForIdentifier`, `lookupFunctionParameterType{Class}`, `findEnclosingFunctionParameter`, `isKnownEnumName` (+ index type, + deadline) | A: O(n²)→O(n) index; D: bounded extraction | +| `gitnexus/src/core/ingestion/workers/worker-pool.ts` | `RetiredWorkerRecord`, `retireWorkerAfterTimeout`, `terminateTrackedWorkers`, `terminate`, `tripBreaker` | C1–C3: safe-point gate + bounded drain + breaker routing | +| `gitnexus/test/unit/worker-pool-timeout-retire.test.ts` | `:97`, `:155` + 3 new cases | New shutdown contract | +| `gitnexus/test/integration/cpp-captures-typeclass-benchmark.test.ts` | new | Scaling regression gate | +| `gitnexus/test/unit/` (new file) | cpp capture deadline-bail | D coverage | + +## 11. Reusable Implementation Context + +```yaml +implementation_context: + task_summary: > + Fix #2432 (SIGABRT on triton analyze): (A) replace per-identifier full-tree/ + per-scope AST re-walks in cpp capture extraction with a lazily-built per-file + index; (C) gate worker terminate on a JS-safe-point flag with a bounded + shutdown drain and breaker-path retire routing; (D) bound cpp capture + extraction wall-time per file (partial-return + warning, never throw). + acceptance_criteria: + - "Triton repro (avoid[4] artifacts) exits 0, no Napi::Error abort" + - "FunctionBuilder.cpp capture extraction sub-second (was 151s)" + - "resolvers/cpp.test.ts + worker-pool suites green" + - "exit-with-busy-worker driver (avoid[4]) exits cleanly against built worker" + primary_symbols: + - { symbol: isKnownEnumName, file: gitnexus/src/core/ingestion/languages/cpp/captures.ts, lines: "1248-1265", role: "full-tree DFS per identifier — replace with per-file enum-name Set" } + - { symbol: lookupDeclaredTypeClassForIdentifier, file: gitnexus/src/core/ingestion/languages/cpp/captures.ts, lines: "1133-1172", role: "per-identifier scope scan — replace with per-scope decl map; preserve first-wins, position-free" } + - { symbol: lookupFunctionParameterTypeClass, file: gitnexus/src/core/ingestion/languages/cpp/captures.ts, lines: "1182-1224", role: "per-identifier param walk — memoize per function node.id" } + - { symbol: inferCppCallArgTypeClasses, file: gitnexus/src/core/ingestion/languages/cpp/captures.ts, lines: "929-1010", role: "per-call driver — threads the index down; call sites at 311/325" } + - { symbol: emitCppScopeCaptures, file: gitnexus/src/core/ingestion/languages/cpp/captures.ts, lines: "15-", role: "per-file entry — owns index lifetime + D deadline checks in its match loop" } + - { symbol: terminateTrackedWorkers, file: gitnexus/src/core/ingestion/workers/worker-pool.ts, lines: "971-980", role: "add safeToTerminate gate (C1); callers: tripBreaker :1312 (void), terminate :2025 (await) — the only two" } + - { symbol: retireWorkerAfterTimeout, file: gitnexus/src/core/ingestion/workers/worker-pool.ts, lines: "1188-1245", role: "set safeToTerminate where terminateWhenBackInJs fires; unref already at :1240" } + - { symbol: tripBreaker, file: gitnexus/src/core/ingestion/workers/worker-pool.ts, lines: "1303-1315", role: "C3: retire busySlots members instead of direct terminate; stays fire-and-forget" } + - { symbol: "pool terminate", file: gitnexus/src/core/ingestion/workers/worker-pool.ts, lines: "~2010-2027", role: "C2: bounded drain of unsafe records before/instead of force terminate" } + related_symbols: + - { symbol: extractParsedFile, relationship: "CALLS emitScopeCaptures via provider hook", relevance: "graph-invisible consumer; signature unchanged" } + - { symbol: "parse-impl.ts:1124 finally", relationship: CALLS, relevance: "the shutdown trigger; C2 drain runs under this await" } + - { symbol: "c-cpp.ts:435 extensions", relationship: config, relevance: ".h routes to cpp provider — hip_prof_str.h covered by A+D" } + - { symbol: busySlots, relationship: "state read by C3", relevance: "worker-pool.ts:1154; add/delete sites verified at :1631/:1646/:1676/:1687/:1720/:1814" } + execution_path: + - "worker: parse file → query.matches → extractParsedFile → emitCppScopeCaptures" + - "per call capture: inferCppCallArgTypeClasses → per identifier: scope scan + full-tree enum DFS (hot)" + - "worker exceeds idle timeout → retire (no terminate) → parse ends → parse-impl finally → pool.terminate → terminateTrackedWorkers → terminate mid-N-API → SIGABRT" + - "ALSO: process exit with native-busy unref'd worker → same abort (verified) — why C2+D exist" + pdg_constraints: + - description: "isKnownEnumName full-tree DFS gated only by typeName!=='' (guard, line 1249); no memo gate exists" + affected_statements: ["gitnexus/src/core/ingestion/languages/cpp/captures.ts:1253-1262"] + implementation_consequence: "Set lookup is behavior-identical; keep the empty/'unknown' early-out" + - description: "terminateTrackedWorkers is straight-line (0 CDG edges) — terminates unconditionally" + affected_statements: ["gitnexus/src/core/ingestion/workers/worker-pool.ts:975-977"] + implementation_consequence: "add safeToTerminate control dependency; drain bounded, never unbounded await" + - description: "lookupDeclaredTypeClassForIdentifier: first-declaration-wins, position-free scope match" + affected_statements: ["gitnexus/src/core/ingestion/languages/cpp/captures.ts:1148-1170"] + implementation_consequence: "build per-scope map in child order, skip existing keys, no use-before-decl filtering" + architectural_patterns: + - { pattern: "once-built per-file index over repeated AST walks", example_location: "gitnexus/src/core/ingestion/languages/c/captures.ts:39-42 (comment citing go #1848 / python #1918); ADL index #1990", usage_guidance: "thread an index object; key node maps by SyntaxNode.id, never object identity" } + - { pattern: "GITNEXUS_BENCH-gated scaling benchmark", example_location: "gitnexus/test/integration/cpp-adl-benchmark.test.ts", usage_guidance: "copy harness shape incl. skipIf + table output" } + - { pattern: "mock-Worker retire harness", example_location: "gitnexus/test/unit/worker-pool-timeout-retire.test.ts:12-64", usage_guidance: "TimeoutThenHealthyWorker: terminateCalls/unrefCalls counters + 'delayed-safe-return' mode cover all new cases; no factory change needed" } + - { pattern: "per-file bail must not throw", example_location: "gitnexus/src/core/ingestion/workers/parse-worker.ts:1362-1369 (CFG isolation comment)", usage_guidance: "D returns partial captures + reportWarning; a throw drops the whole language group" } + files_to_modify: + - { file: gitnexus/src/core/ingestion/languages/cpp/captures.ts, symbols: [see primary], intended_change: "A index + D deadline" } + - { file: gitnexus/src/core/ingestion/workers/worker-pool.ts, symbols: [see primary], intended_change: "C1 gate, C2 drain, C3 breaker routing" } + - { file: gitnexus/test/unit/worker-pool-timeout-retire.test.ts, symbols: [], intended_change: "flip :97/:155 + 3 new cases" } + - { file: gitnexus/test/integration/cpp-captures-typeclass-benchmark.test.ts, symbols: [], intended_change: "new benchmark" } + tests: + - file: gitnexus/test/unit/worker-pool-timeout-retire.test.ts + scenarios: + - "never-safe retired worker + tiny drain cap → pool.terminate() resolves after cap, terminateCalls === 0, diagnostic logged" + - "retired worker signals safe during drain → terminated, terminate() resolves promptly" + - "breaker trip with busy live worker → routed through retire, not direct terminate" + - "UPDATED :97/:155 — unsafe workers not terminated at shutdown (was: terminated)" + - file: gitnexus/test/integration/cpp-captures-typeclass-benchmark.test.ts + scenarios: ["N call-sites × M enums synthetic file → capture emit scales sub-quadratically"] + - file: "gitnexus/test/unit/ (new: cpp capture deadline test)" + scenarios: ["GITNEXUS_CPP_CAPTURE_BUDGET_MS=1 on a many-call file → partial captures returned, warning emitted, no throw"] + - file: gitnexus/test/integration/resolvers/cpp.test.ts + scenarios: ["existing golden behavior unchanged (equivalence gate — run, don't modify)"] + verification_commands: + - "cd gitnexus && npm run build" + - "cd gitnexus && npm run test:unit" + - "cd gitnexus && npm run test:integration" + - "cd gitnexus && GITNEXUS_BENCH=1 npx vitest run test/integration/cpp-captures-typeclass-benchmark.test.ts" + risks: + - "equivalence break in decl ordering (first-wins) → resolver goldens catch" + - "drain cap must exceed D budget (30s > 20s) or drains expire on legitimately-bailing workers" + - "SyntaxNode object identity is NOT stable — key by node.id" + - "residual abort: non-cpp native wedge longer than drain cap still aborts at exit — logged, accepted (child-process workers out of scope)" + assumptions: + - "D's env-read (GITNEXUS_CPP_CAPTURE_BUDGET_MS) is visible in worker threads — CHECK: workers inherit process.env by default; confirm no env filtering in spawnWorker (worker-pool.ts:909-925 sets only workerData/resourceLimits — none seen)" + open_questions: + - "Q2 (narrowed): generic all-language extraction budget — deferred follow-up issue after cpp-only D lands" + avoid: + - "Do not repeat full repository discovery — symbols and line ranges verified at 737a8cdb" + - "Do not change LanguageProvider or emitScopeCaptures signatures — provider hook consumers are graph-invisible" + - "Do not add position/use-before-decl filtering to scope lookups — changes resolution behavior" + - "Repro artifacts in scratchpad: repro-2432-wt (worktree), triton/, mini-2432/, repro-run{1,2}.log, profiles/CPU.*.cpuprofile, profile-worker.mjs, prof-top.mjs, exit-with-busy-worker.mjs — reuse for §7-5, do not re-derive" + - "Do not let a D bail throw out of emitCppScopeCaptures — the language-group catch drops all remaining files (parse-worker.ts:1362-1369)" + - "Do not edit CHANGELOG (release-time owned)" +``` + +## 12. Assumptions and Open Questions + +- **A1 — RESOLVED (refuted):** process exit with a native-busy unref'd worker DOES abort (§5a, empirical). Design consequence absorbed into §6-C2/§6-D. +- **A2 — RESOLVED:** mock harness supports all new shutdown cases without factory changes (test file read in full). +- **A3 (new, minor):** worker threads see `process.env` for D's budget knob — spawn options set only `workerData`/`resourceLimits`, so default env inheritance applies; executor re-verifies in one line. +- **Q2 (narrowed):** generic per-language extraction budget — file as follow-up issue once cpp-only D proves the shape. +- **Deferred:** `lookupDeclaredTypeForIdentifier` (`:1090`) gets the same index for consistency (cheap, in A) though not hot (0.4 s incl.). +- **Graph/source discrepancies recorded:** (i) provider-hook edges invisible to `impact`; (ii) MCP `context` resource staleness banner not refreshed after `--index-only --pdg` while tools serve fresh data — both worth separate GitNexus issues, not this fix. + +## 13. Definition of Done + +1. `GITNEXUS_HOME= GITNEXUS_LBUG_EXTENSION_INSTALL=never GITNEXUS_MAX_FILE_SIZE=5120 node gitnexus/dist/cli/index.js analyze --worker-timeout 60` on triton-lang/triton exits 0 with no `Napi::Error`/SIGABRT, and `lib/Dialect/TritonInstrument/IR/FunctionBuilder.cpp` appears in the index (not quarantined). +2. Mini 2-file corpus (FunctionBuilder.cpp + hip_prof_str.h) analyzes in seconds (was 318.9 s). +3. The §5a exit-with-busy-worker driver, run against the rebuilt worker, exits cleanly. +4. Updated + new worker-pool unit tests green (including flipped `:97`/`:155` contract); resolver goldens (`cpp.test.ts`, `c.test.ts`) green unchanged; `npm run test:unit` and `npm run test:integration` green in `gitnexus/`. +5. Benchmark demonstrates sub-quadratic capture-emit scaling behind `GITNEXUS_BENCH=1`; deadline-bail test proves partial-return-not-throw. +6. No `LanguageProvider`/public API signature changes; no CHANGELOG edits. diff --git a/gitnexus/README.md b/gitnexus/README.md index 9f812045a..2dd0d8180 100644 --- a/gitnexus/README.md +++ b/gitnexus/README.md @@ -535,6 +535,8 @@ Three env vars expose the pool's resilience layers (respawn budget, cumulative-t | `GITNEXUS_WORKER_MAX_RESPAWNS_PER_SLOT` | `3` | Max replacement spawns per slot before the slot is dropped from the active rotation. | | `GITNEXUS_WORKER_MAX_CUMULATIVE_TIMEOUT_MS` | `5 × subBatchTimeoutMs` | Total retry wall-time budget per job before quarantining. Bounds exponentially-growing retry waits. | | `GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD` | `max(3, poolSize)` | Per-slot consecutive deaths before the pool's circuit breaker trips. After tripping, dispatches require a fresh pool. | +| `GITNEXUS_WORKER_SHUTDOWN_DRAIN_MS` | `30000` | Max wait at pool shutdown for a retired worker still inside native code — terminated at its next JS-safe point instead of mid-native-call, which would abort the process (`Napi::Error`, #2432). | +| `GITNEXUS_CPP_CAPTURE_BUDGET_MS` | `20000` | Per-file wall-clock budget for C++ capture extraction; on breach the file keeps partial captures with a warning (#2432). `0` expires immediately. | ### Graph cleanup tuning diff --git a/gitnexus/src/cli/i18n/en.ts b/gitnexus/src/cli/i18n/en.ts index 74a4e397f..c4705701b 100644 --- a/gitnexus/src/cli/i18n/en.ts +++ b/gitnexus/src/cli/i18n/en.ts @@ -300,5 +300,5 @@ export const en = { 'help.option.group.contracts.repo': 'Filter by repo', 'help.option.group.contracts.unmatched': 'Show only unmatched contracts', 'help.analyze.environment': - '\nEnvironment variables:\n GITNEXUS_NO_GITIGNORE=1 Skip .gitignore parsing (still reads .gitnexusignore)\n GITNEXUS_MAX_FILE_SIZE=N Override large-file skip threshold (KB). Default 512, max 32768.\n GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS=N Worker idle timeout in milliseconds. Default 30000.\n GITNEXUS_WAL_CHECKPOINT_THRESHOLD=N LadybugDB WAL auto-checkpoint threshold in bytes (default 67108864 = 64 MiB; -1 keeps Ladybug stock ~16 MiB).\n GITNEXUS_WORKER_SUB_BATCH_MAX_BYTES=N Worker job byte budget. Default 8388608.\n GITNEXUS_WORKER_POOL_SIZE=N Parse worker count override. Default cores-1 capped at 16.\n GITNEXUS_PARSE_CHUNK_CONCURRENCY=N Concurrent in-flight parse chunks. Default 2.\n GITNEXUS_WORKER_MAX_RESPAWNS_PER_SLOT=N Max replacement spawns per slot before drop. Default 3.\n GITNEXUS_WORKER_MAX_CUMULATIVE_TIMEOUT_MS=N Total retry wall-time per job. Default 5x sub-batch timeout.\n GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD=N Per-slot deaths to trip circuit breaker. Default max(3, poolSize).\n GITNEXUS_EMBEDDING_THREADS=N Limit local ONNX CPU threads for --embeddings.\n GITNEXUS_SEMANTIC_EXACT_SCAN_LIMIT=N Max embedding chunks for exact-scan fallback. Default 10000.\n GITNEXUS_VECTOR_MAX_DISTANCE=N Max accepted semantic/vector cosine distance (0 < N <= 2; higher values clamp to 2). Default 0.6 for MCP, 0.5 elsewhere.\n\nFlags override the corresponding env vars when both are provided.\n\nTip: `.gitnexusignore` supports `.gitignore`-style negation. Add e.g.\n `!__tests__/` to index a directory that is auto-filtered by default (#771).', + '\nEnvironment variables:\n GITNEXUS_NO_GITIGNORE=1 Skip .gitignore parsing (still reads .gitnexusignore)\n GITNEXUS_MAX_FILE_SIZE=N Override large-file skip threshold (KB). Default 512, max 32768.\n GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS=N Worker idle timeout in milliseconds. Default 30000.\n GITNEXUS_WAL_CHECKPOINT_THRESHOLD=N LadybugDB WAL auto-checkpoint threshold in bytes (default 67108864 = 64 MiB; -1 keeps Ladybug stock ~16 MiB).\n GITNEXUS_WORKER_SUB_BATCH_MAX_BYTES=N Worker job byte budget. Default 8388608.\n GITNEXUS_WORKER_POOL_SIZE=N Parse worker count override. Default cores-1 capped at 16.\n GITNEXUS_PARSE_CHUNK_CONCURRENCY=N Concurrent in-flight parse chunks. Default 2.\n GITNEXUS_WORKER_MAX_RESPAWNS_PER_SLOT=N Max replacement spawns per slot before drop. Default 3.\n GITNEXUS_WORKER_MAX_CUMULATIVE_TIMEOUT_MS=N Total retry wall-time per job. Default 5x sub-batch timeout.\n GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD=N Per-slot deaths to trip circuit breaker. Default max(3, poolSize).\n GITNEXUS_WORKER_SHUTDOWN_DRAIN_MS=N Max wait at pool shutdown for a retired worker still inside native code (terminated at its next safe point instead of aborting the process). Default 30000.\n GITNEXUS_CPP_CAPTURE_BUDGET_MS=N Per-file wall-clock budget for C++ capture extraction; on breach the file keeps partial captures with a warning. Default 20000.\n GITNEXUS_EMBEDDING_THREADS=N Limit local ONNX CPU threads for --embeddings.\n GITNEXUS_SEMANTIC_EXACT_SCAN_LIMIT=N Max embedding chunks for exact-scan fallback. Default 10000.\n GITNEXUS_VECTOR_MAX_DISTANCE=N Max accepted semantic/vector cosine distance (0 < N <= 2; higher values clamp to 2). Default 0.6 for MCP, 0.5 elsewhere.\n\nFlags override the corresponding env vars when both are provided.\n\nTip: `.gitnexusignore` supports `.gitignore`-style negation. Add e.g.\n `!__tests__/` to index a directory that is auto-filtered by default (#771).', } as const; diff --git a/gitnexus/src/cli/i18n/zh-CN.ts b/gitnexus/src/cli/i18n/zh-CN.ts index 6f3ded476..c5ee954bb 100644 --- a/gitnexus/src/cli/i18n/zh-CN.ts +++ b/gitnexus/src/cli/i18n/zh-CN.ts @@ -279,5 +279,5 @@ export const zhCN = { 'help.option.group.contracts.repo': '按仓库过滤', 'help.option.group.contracts.unmatched': '仅显示未匹配契约', 'help.analyze.environment': - '\n环境变量：\n GITNEXUS_NO_GITIGNORE=1 跳过 .gitignore 解析（仍读取 .gitnexusignore）\n GITNEXUS_MAX_FILE_SIZE=N 覆盖大文件跳过阈值（KB）。默认 512，最大 32768。\n GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS=N Worker 空闲超时（毫秒）。默认 30000。\n GITNEXUS_WAL_CHECKPOINT_THRESHOLD=N LadybugDB WAL 自动 checkpoint 阈值（字节，默认 67108864 = 64 MiB；-1 保持 Ladybug 默认约 16 MiB）。\n GITNEXUS_WORKER_SUB_BATCH_MAX_BYTES=N Worker 作业字节预算。默认 8388608。\n GITNEXUS_WORKER_POOL_SIZE=N 解析 worker 数量覆盖值。默认 cores-1，最多 16。\n GITNEXUS_PARSE_CHUNK_CONCURRENCY=N 并发进行中的解析分块数。默认 2。\n GITNEXUS_WORKER_MAX_RESPAWNS_PER_SLOT=N 每个 slot 丢弃前允许的最大替换进程数。默认 3。\n GITNEXUS_WORKER_MAX_CUMULATIVE_TIMEOUT_MS=N 每个作业的总重试墙钟时间。默认 5 倍子批次超时。\n GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD=N 每个 slot 触发熔断的死亡次数。默认 max(3, poolSize)。\n GITNEXUS_EMBEDDING_THREADS=N 限制 --embeddings 的本地 ONNX CPU 线程数。\n GITNEXUS_SEMANTIC_EXACT_SCAN_LIMIT=N exact-scan 回退的最大嵌入分块数。默认 10000。\n GITNEXUS_VECTOR_MAX_DISTANCE=N 语义/向量搜索接受的最大余弦距离（0 < N <= 2；超出则钳制为 2）。MCP 默认 0.6，其他路径默认 0.5。\n\n当参数和对应环境变量同时提供时，参数优先。\n\n提示：`.gitnexusignore` 支持 `.gitignore` 风格的取反。比如添加\n `!__tests__/` 可以索引默认自动过滤的目录（#771）。', + '\n环境变量：\n GITNEXUS_NO_GITIGNORE=1 跳过 .gitignore 解析（仍读取 .gitnexusignore）\n GITNEXUS_MAX_FILE_SIZE=N 覆盖大文件跳过阈值（KB）。默认 512，最大 32768。\n GITNEXUS_WORKER_SUB_BATCH_TIMEOUT_MS=N Worker 空闲超时（毫秒）。默认 30000。\n GITNEXUS_WAL_CHECKPOINT_THRESHOLD=N LadybugDB WAL 自动 checkpoint 阈值（字节，默认 67108864 = 64 MiB；-1 保持 Ladybug 默认约 16 MiB）。\n GITNEXUS_WORKER_SUB_BATCH_MAX_BYTES=N Worker 作业字节预算。默认 8388608。\n GITNEXUS_WORKER_POOL_SIZE=N 解析 worker 数量覆盖值。默认 cores-1，最多 16。\n GITNEXUS_PARSE_CHUNK_CONCURRENCY=N 并发进行中的解析分块数。默认 2。\n GITNEXUS_WORKER_MAX_RESPAWNS_PER_SLOT=N 每个 slot 丢弃前允许的最大替换进程数。默认 3。\n GITNEXUS_WORKER_MAX_CUMULATIVE_TIMEOUT_MS=N 每个作业的总重试墙钟时间。默认 5 倍子批次超时。\n GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD=N 每个 slot 触发熔断的死亡次数。默认 max(3, poolSize)。\n GITNEXUS_WORKER_SHUTDOWN_DRAIN_MS=N 线程池关闭时等待仍在原生代码中的已退役 worker 的最长时间（到达安全点后再终止，避免进程级 abort）。默认 30000。\n GITNEXUS_CPP_CAPTURE_BUDGET_MS=N C++ 捕获提取的每文件墙钟预算；超出后该文件保留部分捕获并输出警告。默认 20000。\n GITNEXUS_EMBEDDING_THREADS=N 限制 --embeddings 的本地 ONNX CPU 线程数。\n GITNEXUS_SEMANTIC_EXACT_SCAN_LIMIT=N exact-scan 回退的最大嵌入分块数。默认 10000。\n GITNEXUS_VECTOR_MAX_DISTANCE=N 语义/向量搜索接受的最大余弦距离（0 < N <= 2；超出则钳制为 2）。MCP 默认 0.6，其他路径默认 0.5。\n\n当参数和对应环境变量同时提供时，参数优先。\n\n提示：`.gitnexusignore` 支持 `.gitignore` 风格的取反。比如添加\n `!__tests__/` 可以索引默认自动过滤的目录（#771）。', } satisfies EnglishMessages; diff --git a/gitnexus/src/core/ingestion/languages/cpp/captures.ts b/gitnexus/src/core/ingestion/languages/cpp/captures.ts index d34b5c73c..f55efb48c 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/captures.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/captures.ts @@ -22,6 +22,28 @@ import { markCppInlineNamespaceRange } from './inline-namespaces.js'; import { extractCppTemplateConstraints } from './constraint-extractor.js'; import { captureCppMemberLookupFacts } from './member-lookup.js'; import { CPP_BRACED_INIT_TYPE_PREFIX } from './conversion-rank.js'; +import { logger } from '../../../logger.js'; + +/** + * Per-file wall-clock budget for the capture-emit loop (#2432). A worker + * thread stuck in this loop cannot be terminated safely (terminating a + * thread mid-N-API call aborts the whole process with Napi::Error), so the + * loop must bound itself: on breach we return the captures accumulated so + * far with a warning — degraded coverage for one file, never a crash or a + * thrown error (a throw here would make the language-group catch drop every + * remaining file in the batch). + * + * `GITNEXUS_CPP_CAPTURE_BUDGET_MS`: unset/invalid/negative → 20000; explicit + * 0 → expires immediately (deterministic test hook). + */ +const CPP_CAPTURE_BUDGET_DEFAULT_MS = 20_000; + +function cppCaptureBudgetMs(): number { + const raw = process.env.GITNEXUS_CPP_CAPTURE_BUDGET_MS; + if (raw === undefined || raw === '') return CPP_CAPTURE_BUDGET_DEFAULT_MS; + const parsed = Number(raw); + return Number.isFinite(parsed) && parsed >= 0 ? parsed : CPP_CAPTURE_BUDGET_DEFAULT_MS; +} export function emitCppScopeCaptures( sourceText: string, @@ -38,11 +60,33 @@ export function emitCppScopeCaptures( const rawMatches = getCppScopeQuery().matches(tree.rootNode); const out: CaptureMatch[] = []; + // #2432: reset the per-file lookup index. The identifier-argument type + // lookups below used to re-walk the AST per identifier (full-tree DFS in + // isKnownEnumName, per-scope declaration scans) — O(calls × args × treeSize) + // per file, 151s on a 194KB file that parses in 46ms. The index makes each + // lookup O(1) after a single lazily-built pass. + resetCppFileLookupIndex(); + // Track ranges where typedef-struct/enum was captured as its concrete type // so we can suppress the duplicate @declaration.typedef match. const concreteTypedefRanges = new Set(); + // #2432: per-file deadline for the loop below (see cppCaptureBudgetMs). + // Checked every 64 matches — post-index a single iteration is microseconds, + // so the check granularity costs nothing and bounds the drift past the + // deadline to well under a second. + const budgetMs = cppCaptureBudgetMs(); + const deadline = Date.now() + budgetMs; + let matchIndex = 0; + for (const m of rawMatches) { + if ((matchIndex++ & 63) === 0 && Date.now() >= deadline) { + logger.warn( + { filePath, budgetMs, processedMatches: matchIndex - 1, totalMatches: rawMatches.length }, + `C++ capture extraction exceeded its ${budgetMs}ms budget for ${filePath}; returning partial captures for this file (#2432).`, + ); + break; + } const grouped: Record = {}; // Parallel tag -> captured SyntaxNode map. The tree-sitter query already // hands us each matched node as `c.node`, so anchors resolve via a @@ -1076,6 +1120,66 @@ function inferCppBracedInitType(node: SyntaxNode): string { : `${CPP_BRACED_INIT_TYPE_PREFIX}unknown:${elementTypes.length}`; } +/** + * Per-file lookup index (#2432). Reset at the top of `emitCppScopeCaptures` + * (the single per-file entry) and populated lazily by the lookup helpers + * below. Everything is keyed by `SyntaxNode.id` — node WRAPPER objects are + * recreated per access by the tree-sitter binding, so object identity (and + * therefore WeakMap keys) would silently never hit. + * + * - `enumNames`: every named `enum_specifier` in the translation unit, + * collected by ONE root DFS on first `isKnownEnumName` query (was: one + * full-tree DFS per identifier argument — the #2432 hotspot). + * - `scopeDecls`: per enclosing scope, first-declaration-wins map of + * variable name → `declaration` statement (position-free, matching the + * scan it replaces). + * - `fnParams`: per `function_definition`/`function_declarator`, map of + * parameter name → `parameter_declaration` (null when the function has + * no parameter list, preserving the scan's early-return semantics). + */ +interface CppFileLookupIndex { + enumNames: Set | null; + scopeDecls: Map>; + fnParams: Map | null>; +} + +let fileLookupIndex: CppFileLookupIndex = { + enumNames: null, + scopeDecls: new Map(), + fnParams: new Map(), +}; + +function resetCppFileLookupIndex(): void { + fileLookupIndex = { enumNames: null, scopeDecls: new Map(), fnParams: new Map() }; +} + +/** + * First-declaration-wins map of the scope's `declaration` children that + * carry a concrete (non-placeholder) type and a nameable declarator — + * exactly the entries the replaced per-identifier scans could match. + */ +function scopeDeclarationsFor(scope: SyntaxNode): Map { + const cached = fileLookupIndex.scopeDecls.get(scope.id); + if (cached !== undefined) return cached; + const decls = new Map(); + for (let i = 0; i < scope.childCount; i++) { + const stmt = scope.child(i); + if (stmt === null || stmt.type !== 'declaration') continue; + const typeNode = stmt.childForFieldName('type'); + if (typeNode === null) continue; + if (typeNode.type === 'placeholder_type_specifier') continue; + const declarator = stmt.childForFieldName('declarator'); + if (declarator === null) continue; + const nameChild = declaredNameNode(declarator); + if (nameChild === null) continue; + const name = extractDeclaratorLeafName(nameChild); + if (name === '' || decls.has(name)) continue; + decls.set(name, stmt); + } + fileLookupIndex.scopeDecls.set(scope.id, decls); + return decls; +} + /** * Look up the declared type of a variable by scanning sibling declarations * in the enclosing compound_statement (function body). Handles: @@ -1109,25 +1213,12 @@ function lookupDeclaredTypeForIdentifier(identNode: SyntaxNode): string { const paramType = lookupFunctionParameterType(scope, varName); if (paramType !== '') return paramType; - // Scan declarations in the scope for a matching variable name - for (let i = 0; i < scope.childCount; i++) { - const stmt = scope.child(i); - if (stmt === null || stmt.type !== 'declaration') continue; - - const typeNode = stmt.childForFieldName('type'); - if (typeNode === null) continue; - // Skip auto/placeholder types — those need chain-follow, not literal - if (typeNode.type === 'placeholder_type_specifier') continue; - - // Check init_declarator children for the variable name - const declarator = stmt.childForFieldName('declarator'); - if (declarator === null) continue; - const nameChild = declaredNameNode(declarator); - if (nameChild !== null && extractDeclaratorLeafName(nameChild) === varName) { - return normalizeCppTypeText(typeNode.text); - } - } - return ''; + // Indexed scope-declaration lookup (#2432; was a per-identifier scan). + const stmt = scopeDeclarationsFor(scope).get(varName); + if (stmt === undefined) return ''; + const typeNode = stmt.childForFieldName('type'); + if (typeNode === null) return ''; + return normalizeCppTypeText(typeNode.text); } function lookupDeclaredTypeClassForIdentifier(identNode: SyntaxNode): ParameterTypeClass { @@ -1145,30 +1236,23 @@ function lookupDeclaredTypeClassForIdentifier(identNode: SyntaxNode): ParameterT const paramTypeClass = lookupFunctionParameterTypeClass(scope, varName, identNode); if (paramTypeClass !== undefined) return paramTypeClass; - for (let i = 0; i < scope.childCount; i++) { - const stmt = scope.child(i); - if (stmt === null || stmt.type !== 'declaration') continue; + // Indexed scope-declaration lookup (#2432; was a per-identifier scan). + const stmt = scopeDeclarationsFor(scope).get(varName); + if (stmt === undefined) return unknownTypeClass('unknown'); + const typeNode = stmt.childForFieldName('type'); + const declarator = stmt.childForFieldName('declarator'); + const nameChild = declarator !== null ? declaredNameNode(declarator) : null; + if (typeNode === null || nameChild === null) return unknownTypeClass('unknown'); - const typeNode = stmt.childForFieldName('type'); - if (typeNode === null) continue; - if (typeNode.type === 'placeholder_type_specifier') continue; - - const declarator = stmt.childForFieldName('declarator'); - if (declarator === null) continue; - const nameChild = declaredNameNode(declarator); - if (nameChild === null || extractDeclaratorLeafName(nameChild) !== varName) continue; - - const typeClass = classifyCppParameterType( - typeNode.text, - nameChild.text, - stmt.text.replace(/;\s*$/, ''), - ); - if (isKnownEnumName(identNode, typeClass.base)) { - return { ...typeClass, base: `enum:${typeClass.base}` }; - } - return typeClass; + const typeClass = classifyCppParameterType( + typeNode.text, + nameChild.text, + stmt.text.replace(/;\s*$/, ''), + ); + if (isKnownEnumName(identNode, typeClass.base)) { + return { ...typeClass, base: `enum:${typeClass.base}` }; } - return unknownTypeClass('unknown'); + return typeClass; } function lookupFunctionParameterType(scope: SyntaxNode, varName: string): string { @@ -1201,28 +1285,44 @@ function findEnclosingFunctionParameter(scope: SyntaxNode, varName: string): Syn let node: SyntaxNode | null = scope.parent; while (node !== null) { if (node.type === 'function_definition' || node.type === 'function_declarator') { - const fnDecl = - node.type === 'function_declarator' - ? node - : findFirstDescendantOfType(node, 'function_declarator'); - const params = fnDecl?.childForFieldName('parameters') ?? null; - if (params !== null) { - for (let i = 0; i < params.namedChildCount; i++) { - const param = params.namedChild(i); - if (param === null || param.type !== 'parameter_declaration') continue; - const declarator = param.childForFieldName('declarator'); - if (declarator !== null && extractDeclaratorLeafName(declarator) === varName) { - return param; - } - } - } - return null; + return enclosingFunctionParametersFor(node)?.get(varName) ?? null; } node = node.parent; } return null; } +/** + * First-wins map of a function's `parameter_declaration`s by declarator + * leaf name (#2432; was a per-identifier scan). `null` when the function + * has no parameter list — the caller returns null without walking further + * up, preserving the replaced scan's early-return. + */ +function enclosingFunctionParametersFor(fnNode: SyntaxNode): Map | null { + const cached = fileLookupIndex.fnParams.get(fnNode.id); + if (cached !== undefined) return cached; + const fnDecl = + fnNode.type === 'function_declarator' + ? fnNode + : findFirstDescendantOfType(fnNode, 'function_declarator'); + const params = fnDecl?.childForFieldName('parameters') ?? null; + let index: Map | null = null; + if (params !== null) { + index = new Map(); + for (let i = 0; i < params.namedChildCount; i++) { + const param = params.namedChild(i); + if (param === null || param.type !== 'parameter_declaration') continue; + const declarator = param.childForFieldName('declarator'); + if (declarator === null) continue; + const name = extractDeclaratorLeafName(declarator); + if (name === '' || index.has(name)) continue; + index.set(name, param); + } + } + fileLookupIndex.fnParams.set(fnNode.id, index); + return index; +} + function declaredNameNode(declarator: SyntaxNode): SyntaxNode | null { if (declarator.type !== 'init_declarator') return declarator; for (let i = 0; i < declarator.namedChildCount; i++) { @@ -1247,21 +1347,28 @@ function normalizeCppTypeText(text: string): string { function isKnownEnumName(node: SyntaxNode, typeName: string): boolean { if (typeName === '' || typeName === 'unknown') return false; - let root: SyntaxNode = node; - while (root.parent !== null) root = root.parent; - const stack: SyntaxNode[] = [root]; - while (stack.length > 0) { - const cur = stack.pop()!; - if (cur.type === 'enum_specifier') { - const name = cur.childForFieldName('name'); - if (name?.text === typeName) return true; - } - for (let i = 0; i < cur.childCount; i++) { - const child = cur.child(i); - if (child !== null) stack.push(child); + // One full-tree DFS per FILE (lazy), not per identifier argument — the + // per-identifier walk here was the dominant cost of #2432 (87s of a 151s + // extraction on a file that parses in 46ms). + if (fileLookupIndex.enumNames === null) { + let root: SyntaxNode = node; + while (root.parent !== null) root = root.parent; + const names = new Set(); + const stack: SyntaxNode[] = [root]; + while (stack.length > 0) { + const cur = stack.pop()!; + if (cur.type === 'enum_specifier') { + const name = cur.childForFieldName('name'); + if (name !== null) names.add(name.text); + } + for (let i = 0; i < cur.childCount; i++) { + const child = cur.child(i); + if (child !== null) stack.push(child); + } } + fileLookupIndex.enumNames = names; } - return false; + return fileLookupIndex.enumNames.has(typeName); } /** diff --git a/gitnexus/src/core/ingestion/workers/worker-pool.ts b/gitnexus/src/core/ingestion/workers/worker-pool.ts index acdc0339d..28ec14589 100644 --- a/gitnexus/src/core/ingestion/workers/worker-pool.ts +++ b/gitnexus/src/core/ingestion/workers/worker-pool.ts @@ -241,6 +241,19 @@ export interface WorkerPoolOptions { pdg?: boolean; /** Per-function source-line cap for worker-side CFG construction (0 ⇒ no cap). */ pdgMaxFunctionLines?: number; + /** + * Max wall time `terminate()` waits for a retired worker that has NOT yet + * reached a JS-visible safe point before giving up on terminating it + * (#2432). Terminating a worker thread that is inside an N-API call aborts + * the whole process (`Napi::Error` → `std::terminate` → SIGABRT) — and the + * same abort fires at plain process exit, so the drain is what makes + * shutdown safe. On expiry the worker is left running (unref'd, with its + * at-safe-point terminate listener still armed) and a diagnostic is logged. + * Default 30000ms — above the C++ capture budget + * (`GITNEXUS_CPP_CAPTURE_BUDGET_MS`, 20000ms) so the drain converges for + * the known pathological class. 0 ⇒ no wait (test hook). + */ + shutdownDrainMs?: number; } export class WorkerPoolDispatchError extends Error { @@ -521,6 +534,9 @@ function nonNegativeInteger(value: unknown): number | undefined { : undefined; } +/** See {@link WorkerPoolOptions.shutdownDrainMs}. */ +const DEFAULT_SHUTDOWN_DRAIN_MS = 30_000; + interface ResolvedWorkerPoolOptions { subBatchSize: number; subBatchMaxBytes: number; @@ -530,6 +546,7 @@ interface ResolvedWorkerPoolOptions { maxRespawnsPerSlot: number; maxCumulativeTimeoutMs: number; consecutiveFailureThreshold: number; + shutdownDrainMs: number; } export function resolveWorkerPoolOptions( @@ -562,6 +579,10 @@ export function resolveWorkerPoolOptions( positiveInteger(options.consecutiveFailureThreshold) ?? positiveInteger(process.env.GITNEXUS_WORKER_CONSECUTIVE_FAILURE_THRESHOLD) ?? Math.max(DEFAULT_CONSECUTIVE_FAILURE_THRESHOLD_FLOOR, poolSize ?? 0), + shutdownDrainMs: + nonNegativeInteger(options.shutdownDrainMs) ?? + nonNegativeInteger(process.env.GITNEXUS_WORKER_SHUTDOWN_DRAIN_MS) ?? + DEFAULT_SHUTDOWN_DRAIN_MS, }; } @@ -936,6 +957,15 @@ export const createWorkerPool = ( reason: string; cleanup: () => void; terminate: () => Promise; + /** + * True once the worker has been observed at a JS-visible safe point + * (posted a message / messageerror, or died). Until then the worker may + * be inside an N-API call, and `worker.terminate()` would abort the + * whole process (`Napi::Error` → SIGABRT, #2432). + */ + safeToTerminate: boolean; + /** Resolves when `safeToTerminate` flips (or the worker exits/errors). */ + safePoint: Promise; }; const retiredWorkers = new Set(); const respawnCount: number[] = new Array(size).fill(0); @@ -968,15 +998,55 @@ export const createWorkerPool = ( // a terminate during startup aborts pending backoff/retries (#1741). let terminated = false; + /** Resolves `true` when `promise` settles within `ms`, else `false`. The + * timer is unref'd so an expiring drain never holds the process open. */ + const settledWithin = (promise: Promise, ms: number): Promise => { + if (ms <= 0) return Promise.resolve(false); + return new Promise((resolve) => { + const timer = setTimeout(() => resolve(false), ms); + timer.unref?.(); + void promise.then(() => { + clearTimeout(timer); + resolve(true); + }); + }); + }; + const terminateTrackedWorkers = async ( liveWorkers: readonly (Worker | undefined)[], ): Promise => { const retired = Array.from(retiredWorkers); await Promise.all([ ...liveWorkers.map((worker) => worker?.terminate().catch(() => undefined)), - ...retired.map((record) => record.terminate()), + ...retired.map(async (record) => { + // #2432: a retired worker that has not reached a JS-visible safe + // point may be inside an N-API call — terminating it aborts the + // WHOLE process (`Napi::Error` → std::terminate → SIGABRT). Drain: + // wait (bounded) for its safe point; on expiry leave it running — + // it is unref'd and its at-safe-point terminate listener stays + // armed — and log which file wedged it. + if (!record.safeToTerminate) { + const drained = await settledWithin(record.safePoint, poolOptions.shutdownDrainMs); + if (!drained) { + logger.warn( + { + workerIndex: record.workerIndex, + reason: record.reason, + drainMs: poolOptions.shutdownDrainMs, + }, + `Worker ${record.workerIndex} is still inside native code after the ` + + `${poolOptions.shutdownDrainMs}ms shutdown drain; leaving it un-terminated ` + + `to avoid a native abort (#2432). It will be terminated at its next safe point.`, + ); + return; + } + } + await record.terminate(); + }), ]); - retiredWorkers.clear(); + // Undrained records stay tracked so a repeated shutdown call can retry + // their (now possibly safe) terminate; record.terminate() removes each + // drained record via its cleanup. }; for (let i = 0; i < size; i++) { @@ -1192,6 +1262,19 @@ export const createWorkerPool = ( ): void => { let cleaned = false; let terminateStarted = false; + let resolveSafePoint!: () => void; + const safePoint = new Promise((resolve) => { + resolveSafePoint = resolve; + }); + + // A message/messageerror proves the worker is executing JS again; an + // exit/error means the thread is gone. Either way `worker.terminate()` + // can no longer land mid-N-API call (#2432), so shutdown's drain may + // stop waiting. + function markSafeToTerminate() { + record.safeToTerminate = true; + resolveSafePoint(); + } function cleanupRetired() { if (cleaned) return; @@ -1211,6 +1294,7 @@ export const createWorkerPool = ( } function terminateWhenBackInJs() { + markSafeToTerminate(); void terminateRetired(); } @@ -1222,8 +1306,14 @@ export const createWorkerPool = ( } } - const onRetiredError = () => cleanupRetired(); - const onRetiredExit = () => cleanupRetired(); + const onRetiredError = () => { + markSafeToTerminate(); + cleanupRetired(); + }; + const onRetiredExit = () => { + markSafeToTerminate(); + cleanupRetired(); + }; const onRetiredMessageError = () => terminateWhenBackInJs(); const record: RetiredWorkerRecord = { worker, @@ -1231,6 +1321,8 @@ export const createWorkerPool = ( reason, cleanup: cleanupRetired, terminate: terminateRetired, + safeToTerminate: false, + safePoint, }; retiredWorkers.add(record); worker.on('message', onRetiredMessage); @@ -1308,8 +1400,23 @@ export const createWorkerPool = ( reject(err); const liveWorkers = workers.slice(); for (let i = 0; i < workers.length; i++) workers[i] = undefined; + // #2432: a live worker with a job in flight may be inside an N-API + // call — direct terminate risks the same native abort as the retired + // case. Route busy workers through the retire path (terminate at + // their next JS-visible safe point); idle workers are parked in the + // JS event loop and terminate safely right away. + const idleWorkers: (Worker | undefined)[] = []; + for (let i = 0; i < liveWorkers.length; i++) { + const worker = liveWorkers[i]; + if (worker === undefined) continue; + if (busySlots.has(i)) { + retireWorkerAfterTimeout(worker, i, 'circuit breaker tripped with job in flight'); + } else { + idleWorkers.push(worker); + } + } activeSlots.clear(); - void terminateTrackedWorkers(liveWorkers); + void terminateTrackedWorkers(idleWorkers); }; const maybeDone = () => { diff --git a/gitnexus/test/integration/cpp-captures-typeclass-benchmark.test.ts b/gitnexus/test/integration/cpp-captures-typeclass-benchmark.test.ts new file mode 100644 index 000000000..ca5a21405 --- /dev/null +++ b/gitnexus/test/integration/cpp-captures-typeclass-benchmark.test.ts @@ -0,0 +1,105 @@ +/** + * C++ capture-emit identifier-type-lookup scaling benchmark. + * + * Guards the #2432 fix: `emitCppScopeCaptures`'s identifier-argument type + * lookups used to re-walk the AST per identifier — `isKnownEnumName` ran a + * full-tree DFS for EVERY identifier argument of every call, and the + * scope/parameter lookups re-scanned their scope per identifier — + * O(calls × args × treeSize) per file (151s on a 194KB triton file that + * tree-sitter parses in 46ms). They now query a lazily-built per-file index + * (enum-name set, per-scope declaration maps, per-function parameter maps), + * making extraction O(treeSize + identifiers). + * + * Run: GITNEXUS_BENCH=1 npx vitest run test/integration/cpp-captures-typeclass-benchmark.test.ts + * + * WHY DIRECT CALLS, NOT THE PIPELINE: `emitCppScopeCaptures` is the exported + * per-file entry that owns the index lifetime; calling it directly isolates + * exactly the regressed cost (parse + capture emit) from workers, chunking, + * and scope resolution. Co-scaling enums, functions, and call sites with N + * makes the OLD cost O(N²) and the NEW cost O(N); the wall ratio then + * separates them cleanly (linear ≈ Nratio, quadratic ≈ Nratio²). The guard + * sits at Nratio^1.5. + */ +import { describe, it, expect } from 'vitest'; +import { emitCppScopeCaptures } from '../../src/core/ingestion/languages/cpp/captures.js'; + +const BENCH_ENABLED = process.env.GITNEXUS_BENCH === '1'; + +interface BenchResult { + n: number; + callSites: number; + elapsedMs: number; + captureCount: number; +} + +/** + * Generate one C++ file with N enums, N functions of 8 identifier-arg call + * sites each. Every call passes locally-declared identifiers whose declared + * type matches an enum name, forcing the full lookup chain per identifier: + * scope-declaration lookup → classify → enum-name check (the old full-tree + * DFS). Tree size and identifier count both scale with N. + */ +function generateFixture(n: number): string { + const enums = Array.from( + { length: n }, + (_, k) => `enum class Color${k} { Red${k}, Green${k}, Blue${k} };`, + ).join('\n'); + const fns = Array.from({ length: n }, (_, k) => { + const calls = Array.from( + { length: 8 }, + (_, j) => ` sink(c${k}, x${k}, ${j});\n other(x${k}, c${k});`, + ).join('\n'); + return `void fn${k}(int p${k}) {\n Color${k} c${k} = Color${k}::Red${k};\n int x${k} = ${k};\n${calls}\n}`; + }).join('\n'); + return `${enums}\n${fns}\n`; +} + +function runBenchmark(n: number): BenchResult { + const source = generateFixture(n); + const start = Date.now(); + const captures = emitCppScopeCaptures(source, `bench_${n}.cpp`); + return { + n, + callSites: n * 16, + elapsedMs: Date.now() - start, + captureCount: captures.length, + }; +} + +describe.skipIf(!BENCH_ENABLED)('C++ capture identifier-type-lookup benchmark', () => { + it('capture emit scales sub-quadratically with co-scaled enums and call sites', () => { + // Warm-up: parser + query compilation are lazy singletons; exclude their + // one-time cost from the measured runs. + runBenchmark(4); + + const scales = [50, 100, 200]; + const results = scales.map(runBenchmark); + + console.log('\nC++ capture identifier-type-lookup benchmark'); + for (const r of results) { + console.log( + ` n=${String(r.n).padStart(4)} callSites=${String(r.callSites).padStart(5)} ` + + `wall=${String(r.elapsedMs).padStart(6)}ms captures=${r.captureCount}`, + ); + } + + const first = results[0]; + const last = results[results.length - 1]; + const nRatio = last.n / first.n; + + // Linear ≈ nRatio, quadratic ≈ nRatio². nRatio^1.5 sits between them with + // margin for timer/GC noise. Guard the ratio only when the base run is + // measurable (>=20ms) — below that, timer noise dominates and the run is + // itself proof the pathological cost is gone (old code: seconds at n=50). + if (first.elapsedMs >= 20) { + const wallRatio = last.elapsedMs / first.elapsedMs; + expect(wallRatio).toBeLessThan(Math.pow(nRatio, 1.5)); + } else { + expect(last.elapsedMs).toBeLessThan(5_000); + } + + // Sanity: the fixture actually produced call captures at every scale. + expect(first.captureCount).toBeGreaterThan(first.callSites); + expect(last.captureCount).toBeGreaterThan(last.callSites); + }, 300_000); +}); diff --git a/gitnexus/test/unit/cpp-captures-budget.test.ts b/gitnexus/test/unit/cpp-captures-budget.test.ts new file mode 100644 index 000000000..d65f51437 --- /dev/null +++ b/gitnexus/test/unit/cpp-captures-budget.test.ts @@ -0,0 +1,62 @@ +/** + * #2432 — the C++ capture-emit loop must bound its own wall time. + * + * A worker thread stuck in capture extraction cannot be terminated safely + * (terminating a thread mid-N-API call aborts the process with Napi::Error), + * so `emitCppScopeCaptures` checks a per-file deadline and RETURNS partial + * captures with a warning on breach — it must never throw (a throw would + * make parse-worker's language-group catch drop every remaining file). + */ +import { describe, it, expect, afterEach } from 'vitest'; +import { emitCppScopeCaptures } from '../../src/core/ingestion/languages/cpp/captures.js'; +import { _captureLogger } from '../../src/core/logger.js'; + +const MANY_CALLS = [ + 'enum class Color { Red, Green };', + ...Array.from({ length: 200 }, (_, k) => { + return `void fn${k}(int p${k}) {\n Color c${k} = Color::Red;\n sink(c${k}, p${k});\n}`; + }), +].join('\n'); + +const prevBudget = process.env.GITNEXUS_CPP_CAPTURE_BUDGET_MS; + +afterEach(() => { + if (prevBudget === undefined) delete process.env.GITNEXUS_CPP_CAPTURE_BUDGET_MS; + else process.env.GITNEXUS_CPP_CAPTURE_BUDGET_MS = prevBudget; +}); + +describe('C++ capture extraction budget (#2432)', () => { + it('returns partial captures with a warning on budget breach, never throws', () => { + const full = emitCppScopeCaptures(MANY_CALLS, 'budget-full.cpp'); + expect(full.length).toBeGreaterThan(200); + + process.env.GITNEXUS_CPP_CAPTURE_BUDGET_MS = '0'; // expires immediately + const cap = _captureLogger(); + try { + const partial = emitCppScopeCaptures(MANY_CALLS, 'budget-breach.cpp'); + expect(partial.length).toBeLessThan(full.length); + + const warning = cap + .records() + .find((r: { msg?: string }) => (r.msg ?? '').includes('exceeded its 0ms budget')); + expect(warning).toMatchObject({ filePath: 'budget-breach.cpp', budgetMs: 0 }); + } finally { + cap.restore(); + } + }); + + it('invalid budget values fall back to the default and do not fire on normal files', () => { + process.env.GITNEXUS_CPP_CAPTURE_BUDGET_MS = 'not-a-number'; + const cap = _captureLogger(); + try { + const captures = emitCppScopeCaptures(MANY_CALLS, 'budget-default.cpp'); + expect(captures.length).toBeGreaterThan(200); + const warning = cap + .records() + .find((r: { msg?: string }) => (r.msg ?? '').includes('capture extraction exceeded')); + expect(warning).toBeUndefined(); + } finally { + cap.restore(); + } + }); +}); diff --git a/gitnexus/test/unit/worker-pool-cumulative-timeout.test.ts b/gitnexus/test/unit/worker-pool-cumulative-timeout.test.ts index e662263f9..370f5f322 100644 --- a/gitnexus/test/unit/worker-pool-cumulative-timeout.test.ts +++ b/gitnexus/test/unit/worker-pool-cumulative-timeout.test.ts @@ -91,6 +91,10 @@ describe('worker pool cumulative-timeout exhaustion (U10 M6)', () => { // cumulative-timeout branch, not the consecutive-failure trip. consecutiveFailureThreshold: 100, maxRespawnsPerSlot: 100, + // #2432: the hanging worker never reaches a JS-safe point, so the + // finally-block terminate() would otherwise wait the full default + // shutdown drain (30s) before giving up on it. + shutdownDrainMs: 25, workerFactory: () => new HangingWorker() as unknown as import('node:worker_threads').Worker, }); diff --git a/gitnexus/test/unit/worker-pool-timeout-retire.test.ts b/gitnexus/test/unit/worker-pool-timeout-retire.test.ts index 0579ae6d6..29da7f18f 100644 --- a/gitnexus/test/unit/worker-pool-timeout-retire.test.ts +++ b/gitnexus/test/unit/worker-pool-timeout-retire.test.ts @@ -99,6 +99,7 @@ describe('worker pool timeout retirement', () => { subBatchIdleTimeoutMs: 20, maxTimeoutRetries: 1, timeoutBackoffFactor: 2, + shutdownDrainMs: 25, workerFactory: () => new TimeoutThenHealthyWorker() as unknown as import('node:worker_threads').Worker, }); @@ -113,9 +114,19 @@ describe('worker pool timeout retirement', () => { expect(TimeoutThenHealthyWorker.instances[0].unrefCalls).toBe(1); expect(TimeoutThenHealthyWorker.instances[0].terminateCalls).toBe(0); + // #2432: the retired worker never reached a JS-visible safe point, so + // shutdown must NOT terminate it (terminating a thread mid-N-API call + // aborts the whole process). The bounded drain expires and terminate() + // resolves with the worker left running. await pool.terminate(); + expect(TimeoutThenHealthyWorker.instances[0].terminateCalls).toBe(0); - expect(TimeoutThenHealthyWorker.instances[0].terminateCalls).toBe(1); + // Once the worker reaches a safe point, the armed listener terminates it. + TimeoutThenHealthyWorker.instances[0].emit('message', { type: 'sub-batch-done' }); + await waitFor( + () => TimeoutThenHealthyWorker.instances[0]?.terminateCalls === 1, + 'Timed out waiting for post-shutdown safe-point terminate', + ); } finally { await pool.terminate(); } @@ -152,12 +163,13 @@ describe('worker pool timeout retirement', () => { } }); - it('terminates retired workers when the circuit breaker shuts the pool down', async () => { + it('leaves an unsafe retired worker running on breaker trip, terminating it at its safe point', async () => { const pool = createWorkerPool(workerUrl, 1, { subBatchIdleTimeoutMs: 10, maxTimeoutRetries: 1, timeoutBackoffFactor: 2, consecutiveFailureThreshold: 1, + shutdownDrainMs: 25, workerFactory: () => new TimeoutThenHealthyWorker() as unknown as import('node:worker_threads').Worker, }); @@ -169,12 +181,104 @@ describe('worker pool timeout retirement', () => { ]), ).rejects.toThrow(/circuit breaker/i); + // #2432: the stalled worker never signalled a safe point — the breaker's + // background drain must expire WITHOUT terminating it. + await new Promise((resolve) => setTimeout(resolve, 80)); + expect(TimeoutThenHealthyWorker.instances[0].terminateCalls).toBe(0); + expect(TimeoutThenHealthyWorker.instances[0].unrefCalls).toBeGreaterThanOrEqual(1); + + TimeoutThenHealthyWorker.instances[0].emit('message', { type: 'sub-batch-done' }); await waitFor( () => TimeoutThenHealthyWorker.instances[0]?.terminateCalls === 1, - 'Timed out waiting for circuit breaker cleanup to terminate retired worker', + 'Timed out waiting for safe-point terminate after breaker trip', ); } finally { await pool.terminate(); } }); + + it('retires (not terminates) a busy live worker when the breaker trips from another slot', async () => { + // Slot 0 stalls mid-job (native-busy); slot 1 dies, tripping the breaker + // (threshold 1). The breaker must route the BUSY live worker through the + // retire path — direct terminate would abort the process mid-N-API call. + class BusyAndDyingWorker extends TimeoutThenHealthyWorker { + override postMessage(msg: unknown): void { + if (msg !== null && typeof msg === 'object') { + const type = (msg as { type?: unknown }).type; + if (type === 'sub-batch') { + if (this.id === 0) return; // busy forever, never messages back + queueMicrotask(() => this.emit('error', new Error('worker crashed'))); + return; + } + } + super.postMessage(msg); + } + } + + const pool = createWorkerPool(workerUrl, 2, { + subBatchSize: 1, + subBatchIdleTimeoutMs: 5_000, + consecutiveFailureThreshold: 1, + shutdownDrainMs: 25, + workerFactory: () => + new BusyAndDyingWorker() as unknown as import('node:worker_threads').Worker, + }); + + try { + await expect( + pool.dispatch<{ path: string; content: string }, { paths: string[] }>([ + { path: 'src/busy.ts', content: 'const a = 1;' }, + { path: 'src/dies.ts', content: 'const b = 2;' }, + ]), + ).rejects.toThrow(/circuit breaker/i); + + const busy = TimeoutThenHealthyWorker.instances[0]; + // Retired, not terminated: unref'd with the safe-point listener armed. + await waitFor(() => busy.unrefCalls >= 1, 'Timed out waiting for busy worker to be retired'); + await new Promise((resolve) => setTimeout(resolve, 80)); + expect(busy.terminateCalls).toBe(0); + + busy.emit('message', { type: 'sub-batch-done' }); + await waitFor( + () => busy.terminateCalls === 1, + 'Timed out waiting for retired busy worker to terminate at its safe point', + ); + } finally { + await pool.terminate(); + } + }); + + it('terminate() drains a retired worker that reaches its safe point mid-drain', async () => { + TimeoutThenHealthyWorker.firstWorkerBehavior = 'delayed-safe-return'; + TimeoutThenHealthyWorker.safeReturnDelayMs = 5_000; // safe point arrives only via manual emit + const pool = createWorkerPool(workerUrl, 1, { + subBatchIdleTimeoutMs: 10, + maxTimeoutRetries: 1, + timeoutBackoffFactor: 2, + shutdownDrainMs: 2_000, + workerFactory: () => + new TimeoutThenHealthyWorker() as unknown as import('node:worker_threads').Worker, + }); + + try { + const results = await pool.dispatch<{ path: string; content: string }, { paths: string[] }>([ + { path: 'src/native-stall.ts', content: 'const x = 1;' }, + ]); + expect(results).toEqual([{ paths: ['src/native-stall.ts'] }]); + expect(TimeoutThenHealthyWorker.instances[0].terminateCalls).toBe(0); + + // Signal the safe point shortly after shutdown starts: the drain must + // pick it up and terminate promptly instead of waiting out the cap. + const terminatePromise = pool.terminate(); + setTimeout(() => { + TimeoutThenHealthyWorker.instances[0].emit('message', { type: 'sub-batch-done' }); + }, 20); + const start = Date.now(); + await terminatePromise; + expect(Date.now() - start).toBeLessThan(1_500); + expect(TimeoutThenHealthyWorker.instances[0].terminateCalls).toBe(1); + } finally { + await pool.terminate(); + } + }); });