Skip to content

fix: subprocess isolate to close timeout-immune tree-sitter hang#1290

Merged
anderdc merged 3 commits into
entrius:testfrom
seroperson:treesitter-security
May 29, 2026
Merged

fix: subprocess isolate to close timeout-immune tree-sitter hang#1290
anderdc merged 3 commits into
entrius:testfrom
seroperson:treesitter-security

Conversation

@seroperson
Copy link
Copy Markdown
Contributor

Closes #1289

Summary

  • Run the per-PR parse + cursor walk in a spawn-context multiprocessing.Pool(processes=1) with a hard SCORING_SUBPROCESS_BUDGET_S = 5.0s wall-clock. On timeout or worker crash the worker is killed and the PR is zero-scored with scoring_method='skipped-isolation-timeout'; the round continues.
  • Register an atexit hook (isolated_scoring.shutdown) so the multiprocessing resource_tracker does not log spurious leaked-semaphore warnings.

Follow-up to #1276.

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Other (describe below)

Testing

  • Tests added/updated
  • Manually tested

Existing tests cover the subprocess isolation, but I've added several more to ensure timeout hanging works correctly.

@xiao-xiao-mao xiao-xiao-mao Bot added the bug Something isn't working label May 15, 2026
@seroperson seroperson force-pushed the treesitter-security branch from fe4f8f4 to 69fbd9e Compare May 15, 2026 16:35
@plind-junior
Copy link
Copy Markdown
Contributor

@seroperson In which case does this edge case happen? I assume its only when the PR file size is over 1MB. wdyt?

@seroperson
Copy link
Copy Markdown
Contributor Author

@seroperson In which case does this edge case happen? I assume its only when the PR file size is over 1MB. wdyt?

It's not exactly about an edge case, but a specifically structured file which makes parser hang regardless of defined timeout. The size doesn't matter. To not disclosure details publicly, they were already sent in DM to the maintainers.

@seroperson seroperson force-pushed the treesitter-security branch from 69fbd9e to 756c26f Compare May 23, 2026 04:52
@anderdc
Copy link
Copy Markdown
Collaborator

anderdc commented May 26, 2026

can you set SCORING_SUBPROCESS_BUDGET_S = 5.0 to match TREE_SITTER_PARSE_TIMEOUT_MICROS = 2_000_000 timing of 2 seconds

@anderdc
Copy link
Copy Markdown
Collaborator

anderdc commented May 29, 2026

Sorry, my earlier ask was off — I read the budget as per-file, but SCORING_SUBPROCESS_BUDGET_S bounds the whole PR (the for file in file_changes loop plus pickling in/out), while the 2s TREE_SITTER_PARSE_TIMEOUT_MICROS is per-file. At 2.0s a multi-file PR, or one slow-but-legit file plus IPC, can blow the budget and zero-score the whole PR. Please set it back to 5s, or whatever value you think gives enough headroom over the 2s per-file parse timeout. Worth noting it should also cover worker cold-start: right after a timeout resets the pool, the next PR pays a fresh spawn + bittensor/tree-sitter import inside the same window.

@seroperson seroperson force-pushed the treesitter-security branch from 96184fb to 470d199 Compare May 29, 2026 18:22
@seroperson seroperson force-pushed the treesitter-security branch from 470d199 to 6861add Compare May 29, 2026 18:29
@anderdc anderdc merged commit add2df8 into entrius:test May 29, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: subprocess-isolate scoring to close timeout-immune tree-sitter hangs

3 participants