Skip to content

Refuse to spawn an extraction pool that would spawn its own (#1637) - #3620

Open
ayushcodes10 wants to merge 3 commits into
Graphify-Labs:v8from
ayushcodes10:fix-1637-windows-spawn-guard
Open

ayushcodes10 wants to merge 3 commits into
Graphify-Labs:v8from
ayushcodes10:fix-1637-windows-spawn-guard

Conversation

@ayushcodes10

Copy link
Copy Markdown
Contributor

Summary

Fixes #1637. On Windows, a caller script with no if __name__ == "__main__": guard turns extract()'s parallel path into a fork bomb rather than a slow failure: every spawned worker re-executes the top-level module on import, and if that module calls extract() again at module scope (the common shape of a quick inline runner script), the worker opens its own ProcessPoolExecutor, whose own guard-less children do the same. A second reporter measured this directly — ~1,278 python.exe processes and ~28 GB consumed before the machine had to be power-cycled.

Root cause

_extract_parallel already caught BrokenProcessPool and fell back to sequential extraction, but only after the pool tried to run. ProcessPoolExecutor._adjust_process_count() respawns dying workers as they die, and on a guard-less caller every respawned worker dies the same way (re-importing __main__ triggers _check_not_importing_main) and — critically — re-executes the module-level extract() call, opening yet another pool. The pool keeps minting doomed processes faster than any per-future exception can propagate and stop it, so the reactive except BrokenProcessPool handler never gets a chance to run.

Fix

Two checks now run in _extract_parallel before a ProcessPoolExecutor is ever constructed:

  1. Unconditional: refuse to open a pool when already running inside a multiprocessing child (multiprocessing.parent_process() is not None). A legitimate call to _extract_parallel only ever happens in the main process — a worker should only ever run _extract_single_file via the pool machinery — so this is always safe and breaks the specific recursive-spawn mechanism regardless of platform.
  2. Windows only: pre-emptively decline the pool when the caller's own __main__ module's source lacks the __main__ guard text, instead of discovering the failure only after the pool has already started respawning. Scoped to sys.platform == "win32" (matching the existing Windows-specific worker cap already in this function) since macOS/Linux typically default to fork, where this class of guard-less-caller re-execution doesn't occur the same way, and scoping it avoids changing extraction behavior on non-Windows CI.

Both checks fall back to sequential extraction exactly like the existing BrokenProcessPool handler — correct output, just without the pool.

Test plan

  • Added test_extract_parallel_declines_pool_inside_a_spawned_worker, test_extract_parallel_declines_pool_on_windows_when_caller_lacks_guard, and test_extract_parallel_still_spawns_pool_on_windows_when_caller_has_guard to tests/test_extract.py.
  • Confirmed all pre-existing Windows-spawn fallback tests in the same file still pass unmodified.
  • Full suite: python3 -m pytest -q — 5624 passed, 68 skipped, no regressions.
  • python3 -m tools.skillgen --check — OK.

🤖 Generated with Claude Code

ayushcodes10 and others added 3 commits September 17, 2026 01:57
A guard less caller (no if __name__ == "__main__": block) makes every
Windows spawned worker re execute the top level module on import. If
that module calls extract() again at module scope, the worker opens
its own pool, whose own guard less children do the same, faster than
any per future BrokenProcessPool exception can surface and stop it.

Two checks now run before the pool is ever opened: refuse
unconditionally when already inside a multiprocessing child (a
legitimate call only ever happens in the main process), and on
Windows, skip the pool when the caller's own __main__ module has no
guard, rather than only catching the failure after the fact.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers both new guards in _extract_parallel: refusing to open a pool
from inside a spawned worker, and pre emptively declining the pool on
Windows for a caller whose main module lacks the guard, while
confirming a properly guarded caller still takes the pool path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.

Formal verification. No changes could be formally verified in this run.


Graphify review — findings

Guards extract()'s parallel path against a fork bomb on guard-less Windows callers: _extract_parallel now refuses to open a ProcessPoolExecutor whenever multiprocessing.parent_process() shows it's already inside a spawned worker, and on Windows it also declines pre-emptively when the caller's __main__ source lacks an if __name__ == "__main__": guard (detected by _caller_main_lacks_guard, which treats an unreadable source as "can't tell"). Both cases warn and hand the work back for sequential extraction; a properly guarded Windows caller still takes the pool path.

Worth a look

  • Windows fork-bomb guard is bypassed by any __main__ substringgraphify/extract.py:6393 · Escalate · high
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • Guard detection treats any 'main' occurrence as a valid multiprocessing guardgraphify/extract.py · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • Windows guard check is bypassed by any 'main' stringgraphify/extract.py:6388 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • Guard detection matches any 'main' substring, not the guardgraphify/extract.py:6391 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • Guard detection false-negative for any file containing the string 'main'graphify/extract.py:6392 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 2474 functions depend on the 875 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 645 callers, 45 callees
  • new: _rebuild_code() — 129 callers, 54 callees
  • new: extract_js() — 85 callers, 4 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: dispatch_command() — 2 callers, 125 callees
  • new: _get_extractor() — 26 callers, 6 callees
  • new: run_pipeline() — 8 callers, 13 callees
  • new: collect_files() — 17 callers, 6 callees
  • …and 30 more — each is listed as a finding

Verification — 2474 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 2299 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

286 of 286 test file(s) selected (100%) via static blast radius.

Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.

  • tests/test_affected_cli.py — full-run-safety
  • tests/test_affected_member_seed.py — full-run-safety
  • tests/test_agents_platform.py — full-run-safety
  • tests/test_analyze.py — full-run-safety
  • tests/test_anthropic_custom_endpoint.py — full-run-safety
  • tests/test_antigravity_install.py — full-run-safety
  • tests/test_apm_fallback_version.py — full-run-safety
  • tests/test_architecture_doc.py — full-run-safety
  • tests/test_astro_extraction.py — impact, full-run-safety
  • tests/test_astro_import_ids.py — impact, full-run-safety
  • tests/test_atomic_canvas_export.py — full-run-safety
  • tests/test_atomic_version_stamp.py — full-run-safety
  • tests/test_atomic_writes.py — full-run-safety
  • tests/test_backend_env_isolation.py — full-run-safety
  • tests/test_backend_extras.py — full-run-safety
  • tests/test_benchmark.py — full-run-safety
  • tests/test_benchmark_raw_graph.py — full-run-safety
  • tests/test_build.py — impact, full-run-safety
  • tests/test_build_merge_dedup_scope.py — full-run-safety
  • tests/test_build_merge_hyperedges_and_prune.py — full-run-safety
  • tests/test_build_merge_shrink_guard.py — full-run-safety
  • tests/test_builtin_global_type_refs.py — impact, full-run-safety
  • tests/test_cache.py — full-run-safety
  • tests/test_callflow_html.py — full-run-safety
  • tests/test_cargo_introspect.py — full-run-safety
  • tests/test_carried_hyperedge_remap.py — full-run-safety
  • tests/test_case_sensitive_resolution.py — impact, full-run-safety
  • tests/test_charmap_encoding.py — full-run-safety
  • tests/test_chunking.py — full-run-safety
  • tests/test_cjs_module_extension.py — impact, full-run-safety
  • tests/test_claude_cli_backend.py — full-run-safety
  • tests/test_claude_md.py — full-run-safety
  • tests/test_cli_broken_pipe.py — full-run-safety
  • tests/test_cli_export.py — full-run-safety
  • tests/test_cli_help.py — full-run-safety
  • tests/test_cluster.py — full-run-safety
  • tests/test_codebuddy.py — full-run-safety
  • tests/test_community_hub_labels.py — full-run-safety
  • tests/test_community_labels_skill.py — full-run-safety
  • tests/test_confidence.py — full-run-safety
  • tests/test_corrupt_graph_json.py — full-run-safety
  • tests/test_cpp_nested_and_cli.py — impact, full-run-safety
  • tests/test_cpp_objc_cross_file_calls.py — impact, full-run-safety
  • tests/test_cpp_preprocess.py — full-run-safety
  • tests/test_cross_extension_reexport_self_cycle.py — impact, full-run-safety
  • tests/test_cross_language_call_resolution.py — impact, full-run-safety
  • tests/test_cross_repo_external_call_guards.py — impact, full-run-safety
  • tests/test_cross_repo_member_calls.py — impact, full-run-safety
  • tests/test_cross_repo_shared_types.py — full-run-safety
  • tests/test_csharp_call_site_generic_args.py — impact, full-run-safety
  • … and 236 more

non-code file(s) changed (CHANGELOG.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)

changed code file(s) with no mapped test (CHANGELOG.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests

Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.

Formal verification

Could not verify: Could not verify \_extract\_parallel.

The verifier did not have enough to check \_extract\_parallel, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: no capturable inputs from the test suite; property tier: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set

· 38 more finding(s) on lines outside this diff (see the check run).

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

Labels

None yet

Projects

None yet

1 participant