fix(scan): prevent tree-sitter segfault on large/deeply-nested files — closes #116#124
Merged
Merged
Conversation
…closes #116 Root cause: tree-sitter 0.26 + tree-sitter-javascript 0.25 + tree-sitter-python 0.25 binding combination has nondeterministic SIGSEGV when walking deeply-nested ASTs. The crash occurs in node.children / node.child_by_field_name access during GC, and even with gc.disable() the binding's internal cleanup can invalidate Node pointers held across function boundaries. Fix applies four layers of defense: 1. base_parser.walk_tree: convert recursive → iterative DFS with explicit (node, depth) stack, and disable cyclic GC for the walk duration. Adds parse_tree() method that returns the Tree object (not just root node) so callers can hold an explicit reference. 2. base_parser.parse: keep self._last_tree reference so the Tree is not collected while callers hold Node references from the most recent parse. 3. js_backend_parser.extract_references: rewrite from two-pass (which held body_node Node references across passes → dangling pointers) to single-pass recursive walk that processes each declaration's body immediately. Body Node references never leave the callback frame. _find_calls_in_scope also converted from iterative walk_tree to local recursive walk for the same reason. Files > 100 lines are skipped with a warning (pragmatic workaround — tree-sitter-javascript 0.25 still crashes on large files even with all the above mitigations). 4. python_parser.extract_references: disable GC during parse+walk, add max_depth=200 guard to _walk. Files > 200 lines skipped with warning. 5. outline_engine._outline_python / _outline_javascript: route to regex fallback for files > 200 / 100 lines respectively. Verified: 'codelens scan .' on the CodeLens repo itself now completes without crash (was SIGSEGV exit 139). Test suite: 1183 passed, 1 pre- existing failure (#118, unrelated).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



What
Fixes the CRITICAL RED TEAM issue #116:
codelens scan .on the CodeLens repo itself crashed with SIGSEGV (exit 139) injs_backend_parser._parse_variable_declaratorandpython_parser._walk.Root cause
tree-sitter 0.26 + tree-sitter-javascript 0.25 + tree-sitter-python 0.25 binding combination has nondeterministic SIGSEGV when walking deeply-nested ASTs. The crash occurs in
node.children/node.child_by_field_nameaccess during Python GC, and even withgc.disable()the binding's internal cleanup can invalidate Node pointers held across function boundaries.The previous two-pass JS parser design (find declarations → then walk each body) held
body_nodeNode references across function boundaries, which dangled when tree-sitter's internal cleanup ran between passes.Fix — four layers of defense
base_parser.walk_tree: convert recursive → iterative DFS with explicit(node, depth)stack, and disable cyclic GC for the walk duration. Addsparse_tree()method that returns the Tree object (not just root node) so callers can hold an explicit reference.base_parser.parse: keepself._last_treereference so the Tree is not collected while callers hold Node references from the most recent parse.js_backend_parser.extract_references: rewrite from two-pass (which heldbody_nodeNode references across passes → dangling pointers) to single-pass recursive walk that processes each declaration's body immediately. Body Node references never leave the callback frame._find_calls_in_scopealso converted from iterativewalk_treeto local recursive walk for the same reason. Files > 100 lines are skipped with a warning (pragmatic workaround — tree-sitter-javascript 0.25 still crashes on large files even with all the above mitigations).python_parser.extract_references: disable GC during parse+walk, addmax_depth=200guard to_walk. Files > 200 lines skipped with warning.outline_engine._outline_python/_outline_javascript: route to regex fallback for files > 200 / 100 lines respectively (socodelens scanstill generates outline.json without crash).Verification
Stable across 3 consecutive runs. Test suite: 1183 passed, 1 pre-existing failure (#118 — test expectation mismatch, unrelated to this fix).
Trade-offs
graph_nodes/graph_edgestables). This is a known limitation until tree-sitter bindings are upgraded.Test plan
codelens scan .on CodeLens repo completes without crashRisk
Medium. The skip threshold is conservative — large files will be missing from the graph, which could affect downstream analysis (dead-code, arch-metrics). But this is strictly better than crashing the entire scan. When tree-sitter bindings are upgraded to a version without the segfault, the thresholds can be raised or removed.
Related issues