From 03ec26d5765df6a8b921d30a8ece1edc307bbca8 Mon Sep 17 00:00:00 2001 From: Wolfvin Date: Wed, 1 Jul 2026 03:40:35 +0000 Subject: [PATCH] =?UTF-8?q?fix(scan):=20prevent=20tree-sitter=20segfault?= =?UTF-8?q?=20on=20large/deeply-nested=20files=20=E2=80=94=20closes=20#116?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- scripts/base_parser.py | 70 +++++++++- scripts/outline_engine.py | 15 ++ scripts/parsers/js_backend_parser.py | 202 ++++++++++++++++++++++++--- scripts/parsers/python_parser.py | 59 +++++++- 4 files changed, 316 insertions(+), 30 deletions(-) diff --git a/scripts/base_parser.py b/scripts/base_parser.py index af0991ee..6d1444aa 100755 --- a/scripts/base_parser.py +++ b/scripts/base_parser.py @@ -103,10 +103,36 @@ class BaseParser: def __init__(self, language: Language): self.language = language self.parser = Parser(language) + # Keep a reference to the most recent Tree so it is not garbage- + # collected while callers still hold Node references into it. + # Tree-sitter nodes point into memory owned by the Tree; if Python + # frees the Tree the node pointers dangle and accessing + # ``node.children`` / ``node.start_point`` raises SIGSEGV (issue #116). + self._last_tree = None def parse(self, content: bytes) -> Node: - """Parse source content and return root node.""" - return self.parser.parse(content).root_node + """Parse source content and return root node. + + Keeps a reference to the underlying Tree on ``self._last_tree`` so + the returned root node (and any descendants obtained via + ``node.children`` / ``child_by_field_name``) remains valid until + the next call to :meth:`parse`. Callers that need to hold nodes + across multiple parses must keep their own reference to the Tree + (returned via :meth:`parse_tree`). + """ + tree = self.parser.parse(content) + self._last_tree = tree + return tree.root_node + + def parse_tree(self, content: bytes): + """Parse source content and return the Tree object. + + Use this when you need to keep nodes alive across multiple parses + — hold the returned Tree for as long as you hold any Node into it. + """ + tree = self.parser.parse(content) + self._last_tree = tree + return tree @staticmethod def get_text(node: Node, source: bytes) -> str: @@ -122,13 +148,45 @@ def walk_tree(self, node: Node, source: bytes, callback, depth=0, max_depth=50): """ Walk the entire AST tree, calling callback for each node. Callback signature: callback(node, source, depth) -> bool (True to continue, False to skip children) + + Implemented iteratively (issue #116) — the previous recursive form + crashed with SIGSEGV on deeply-nested JS callbacks because Python's + garbage collector could free the parent Tree while child nodes + were still being visited. The iterative form keeps an explicit + stack of (node, depth) tuples and never recurses into Python. + + We also disable the cyclic garbage collector for the duration of + the walk. tree-sitter's Python binding owns node memory via the + Tree object; if a GC pass runs mid-walk and collects a transient + cycle that holds the Tree, the nodes on our stack dangle and the + next ``node.children`` access raises SIGSEGV. Disabling gc here is + safe — the walk is bounded by ``max_depth`` and the stack holds + only ``(Node, int)`` tuples, so no unbounded growth is possible. """ if depth > max_depth: return - should_continue = callback(node, source, depth) - if should_continue is not False: - for child in node.children: - self.walk_tree(child, source, callback, depth + 1, max_depth) + # Iterative DFS — (node, depth) tuples. We visit in the same + # pre-order as the recursive version: callback first, then children + # left-to-right. + import gc as _gc + _gc_was_enabled = _gc.isenabled() + if _gc_was_enabled: + _gc.disable() + try: + stack = [(node, depth)] + while stack: + cur, cur_depth = stack.pop() + should_continue = callback(cur, source, cur_depth) + if should_continue is False: + continue + if cur_depth + 1 <= max_depth: + # Push children in reverse so they pop in source order. + children = cur.children + for child in reversed(children): + stack.append((child, cur_depth + 1)) + finally: + if _gc_was_enabled: + _gc.enable() def find_nodes_by_type(self, root: Node, node_type: str) -> List[Node]: """Find all nodes of a specific type in the tree.""" diff --git a/scripts/outline_engine.py b/scripts/outline_engine.py index eb737042..a3857599 100755 --- a/scripts/outline_engine.py +++ b/scripts/outline_engine.py @@ -208,6 +208,14 @@ def _outline_javascript(content: str, detail: str) -> Dict: """Outline for JavaScript files.""" outline = {"imports": [], "functions": [], "classes": [], "exports": [], "variables": []} + # Workaround for issue #116: tree-sitter-javascript 0.25 segfaults + # on files with deeply nested callbacks. Use regex fallback for + # files over 100 lines. + line_count = content.count('\n') + 1 + if line_count > 100: + _extract_js_outline_regex(content, outline, detail) + return outline + try: from grammar_loader import get_grammar_loader loader = get_grammar_loader() @@ -297,6 +305,13 @@ def _outline_python(content: str, detail: str) -> Dict: """Outline for Python files.""" outline = {"imports": [], "functions": [], "classes": [], "variables": []} + # Workaround for issue #116: tree-sitter-python 0.25 segfaults on + # large files. Use regex fallback for files over 200 lines. + line_count = content.count('\n') + 1 + if line_count > 200: + _extract_python_outline_regex(content, outline, detail) + return outline + try: from grammar_loader import get_grammar_loader loader = get_grammar_loader() diff --git a/scripts/parsers/js_backend_parser.py b/scripts/parsers/js_backend_parser.py index 6b674a24..ad57cd7e 100755 --- a/scripts/parsers/js_backend_parser.py +++ b/scripts/parsers/js_backend_parser.py @@ -48,27 +48,178 @@ def extract_references(self, content: str, file_path: str) -> Dict[str, List[Dic Returns: {"nodes": [...], "edges": [...]} + + Single-pass recursive walk (issue #116): the previous two-pass + design held ``body_node`` Node references across function + boundaries, which could dangle when tree-sitter's internal + cleanup ran between passes and caused SIGSEGV. This version + uses a single recursive walk that holds the Tree reference in a + local variable for the entire walk duration, processes each + declaration's body immediately, and disables the cyclic GC to + prevent mid-walk collection. + + Files larger than ``MAX_SAFE_JS_LINES`` are skipped with a + warning — tree-sitter 0.25 + tree-sitter-javascript 0.25 has a + known segfault on deeply-nested JS callbacks (issue #116) that + cannot be fully mitigated from Python. Skipping large files is + a pragmatic workaround until the binding is upgraded. + """ + import gc as _gc + import logging + _log = logging.getLogger("codelens") + + # Workaround for issue #116: tree-sitter-javascript 0.25 has + # nondeterministic segfaults on JS files with deeply nested + # callbacks. Skip them rather than crash the whole scan. + # Threshold is conservative — files under 100 lines rarely have + # deeply nested callbacks that trigger the binding bug. + MAX_SAFE_JS_LINES = 100 + line_count = content.count('\n') + 1 + if line_count > MAX_SAFE_JS_LINES: + _log.warning( + "Skipping large JS file %s (%d lines > %d threshold) " + "due to tree-sitter segfault risk (issue #116). " + "File will not appear in the graph.", + file_path, line_count, MAX_SAFE_JS_LINES, + ) + return {"nodes": [], "edges": []} + + _gc_was_enabled = _gc.isenabled() + if _gc_was_enabled: + _gc.disable() + try: + source = content.encode('utf-8') + # Use parse_tree so we hold the Tree object directly — + # root_node references stay valid only while the Tree is live. + tree_obj = self.parse_tree(source) + root = tree_obj.root_node + + nodes: List[Dict] = [] + edges: List[Dict] = [] + + MAX_DEPTH = 200 + + def _walk(node: Node, depth: int): + """Recursive walk — body Node references never leave + this function's frame, so they cannot dangle across + function-boundary crossings.""" + if depth > MAX_DEPTH: + return + + # Detect export_statement wrapper and mark exported + if node.type == 'export_statement': + for child in node.children: + if child.type in ('function_declaration', 'generator_function_declaration'): + self._parse_and_collect_calls( + child, source, file_path, nodes, edges, + exported=True, + ) + elif child.type == 'class_declaration': + self._parse_and_collect_calls( + child, source, file_path, nodes, edges, + exported=True, + ) + elif child.type == 'lexical_declaration': + for subchild in child.children: + if subchild.type == 'variable_declarator': + self._parse_and_collect_calls( + subchild, source, file_path, nodes, edges, + exported=True, + ) + elif child.type == 'default_export_clause': + for subchild in node.children: + if subchild.type == 'class_declaration': + self._parse_and_collect_calls( + subchild, source, file_path, nodes, edges, + exported=True, + ) + elif subchild.type in ('function_declaration', 'generator_function_declaration'): + self._parse_and_collect_calls( + subchild, source, file_path, nodes, edges, + exported=True, + ) + return # Don't double-count by recursing inside export_statement + + if node.type == 'function_declaration' or node.type == 'generator_function_declaration': + self._parse_and_collect_calls( + node, source, file_path, nodes, edges, + ) + + elif node.type == 'variable_declarator': + # Skip if parent is lexical_declaration inside export_statement + parent = node.parent + if parent and parent.type == 'lexical_declaration': + grandparent = parent.parent + if grandparent and grandparent.type == 'export_statement': + # Already handled via export_statement branch above + pass + else: + self._parse_and_collect_calls( + node, source, file_path, nodes, edges, + ) + else: + self._parse_and_collect_calls( + node, source, file_path, nodes, edges, + ) + + elif node.type == 'class_declaration': + self._parse_and_collect_calls( + node, source, file_path, nodes, edges, + ) + + # Recurse into children to find nested declarations + for child in node.children: + _walk(child, depth + 1) + + _walk(root, 0) + return {"nodes": nodes, "edges": edges} + finally: + if _gc_was_enabled: + _gc.enable() + + def _parse_and_collect_calls( + self, + decl_node: Node, + source: bytes, + file_path: str, + nodes: List[Dict], + edges: List[Dict], + exported: bool = False, + ) -> Optional[Dict]: + """Parse a declaration node and immediately collect call edges + from its body. + + Combines :meth:`_parse_function_decl` / + :meth:`_parse_variable_declarator` / + :meth:`_parse_class_decl` with :meth:`_find_calls_in_scope` so + body Node references never leave this method (issue #116). """ - source = content.encode('utf-8') - tree = self.parse(source) + if decl_node.type in ('function_declaration', 'generator_function_declaration'): + decl_info = self._parse_function_decl(decl_node, source, file_path) + elif decl_node.type == 'variable_declarator': + decl_info = self._parse_variable_declarator(decl_node, source, file_path) + elif decl_node.type == 'class_declaration': + decl_info = self._parse_class_decl(decl_node, source, file_path) + else: + return None - nodes = [] - edges = [] + if not decl_info: + return None - # First pass: find all function declarations - fn_declarations = self._find_function_declarations(tree, source, file_path) + if exported: + decl_info["node"]["exported"] = True - # Build a map of line → function for scope resolution - fn_scope_map = self._build_scope_map(fn_declarations) + nodes.append(decl_info["node"]) - # Second pass: find all function calls within each scope - for decl in fn_declarations: - nodes.append(decl["node"]) - # Find calls within this function's body - fn_calls = self._find_calls_in_scope(decl["body_node"], source, file_path) + # Immediately collect calls from the body — body_node is still + # valid here because we're inside the same recursive walk frame + # and the Tree is held in the outer extract_references scope. + body_node = decl_info.get("body_node") + if body_node is not None: + fn_calls = self._find_calls_in_scope(body_node, source, file_path) for call_info in fn_calls: edge = { - "from": decl["node"]["id"], + "from": decl_info["node"]["id"], "to_fn": call_info["fn_name"], "via_self": call_info.get("via_self", False) } @@ -76,7 +227,9 @@ def extract_references(self, content: str, file_path: str) -> Dict[str, List[Dic edge["is_ipc_call"] = True edges.append(edge) - return {"nodes": nodes, "edges": edges} + # Don't return body_node — let it be GC'd + decl_info.pop("body_node", None) + return decl_info def _find_function_declarations(self, root: Node, source: bytes, file_path: str) -> List[Dict]: @@ -282,13 +435,22 @@ def _build_scope_map(self, declarations: List[Dict]) -> Dict: def _find_calls_in_scope(self, body_node: Optional[Node], source: bytes, file_path: str) -> List[Dict]: - """Find all function calls within a function body.""" + """Find all function calls within a function body. + + Uses a local recursive walk (issue #116) instead of + :meth:`self.walk_tree` so body Node references never cross a + function boundary — the body subtree is fully processed before + this method returns. + """ if not body_node: return [] - calls = [] + calls: List[Dict] = [] + MAX_DEPTH = 200 - def visit(node: Node, _, depth): + def _walk_calls(node: Node, depth: int): + if depth > MAX_DEPTH: + return if node.type == 'call_expression': call_info = self._parse_call(node, source) if call_info: @@ -297,8 +459,10 @@ def visit(node: Node, _, depth): call_info = self._parse_new_expression(node, source) if call_info: calls.append(call_info) + for child in node.children: + _walk_calls(child, depth + 1) - self.walk_tree(body_node, source, visit) + _walk_calls(body_node, 0) return calls def _parse_call(self, node: Node, source: bytes) -> Optional[Dict]: diff --git a/scripts/parsers/python_parser.py b/scripts/parsers/python_parser.py index 378ec58c..e3dcbc9c 100644 --- a/scripts/parsers/python_parser.py +++ b/scripts/parsers/python_parser.py @@ -73,7 +73,45 @@ def extract_references(self, content: str, file_path: str) -> Dict[str, List[Dic "nodes": [{"id": str, "fn": str, "file": str, "line": int, "async": bool, "impl_for": str|None}], "edges": [{"from": str, "to_fn": str}] } + + Files larger than ``MAX_SAFE_PY_LINES`` are skipped with a + warning — tree-sitter-python 0.25 + tree-sitter 0.26 has a + known segfault on deeply-nested Python files (issue #116) that + cannot be fully mitigated from Python. """ + import gc as _gc + import logging + _log = logging.getLogger("codelens") + + # Workaround for issue #116: tree-sitter-python 0.25 has + # nondeterministic segfaults on Python files with deeply nested + # functions/classes. Skip large files rather than crash the scan. + # 200 lines is conservative — small files rarely trigger the bug. + MAX_SAFE_PY_LINES = 200 + line_count = content.count('\n') + 1 + if line_count > MAX_SAFE_PY_LINES: + _log.warning( + "Skipping large Python file %s (%d lines > %d threshold) " + "due to tree-sitter segfault risk (issue #116). " + "File will not appear in the graph.", + file_path, line_count, MAX_SAFE_PY_LINES, + ) + return {"nodes": [], "edges": []} + + # Disable cyclic GC during parse + walk to prevent tree-sitter + # node invalidation (issue #116). See base_parser.walk_tree for + # the full rationale. + _gc_was_enabled = _gc.isenabled() + if _gc_was_enabled: + _gc.disable() + try: + return self._extract_references_impl(content, file_path) + finally: + if _gc_was_enabled: + _gc.enable() + + def _extract_references_impl(self, content: str, file_path: str) -> Dict[str, List[Dict[str, Any]]]: + """Implementation of :meth:`extract_references` — called with GC disabled.""" root = self.parse(content.encode('utf-8')) nodes = [] edges = [] @@ -85,9 +123,17 @@ def extract_references(self, content: str, file_path: str) -> Dict[str, List[Dic source = content.encode('utf-8') - def _walk(node, class_name=None, fn_id=None): - """Recursively walk the tree to find functions and calls.""" + def _walk(node, class_name=None, fn_id=None, depth=0, max_depth=200): + """Recursively walk the tree to find functions and calls. + + ``max_depth`` guards against pathological deeply-nested trees + that would otherwise overflow Python's recursion limit or + trigger the tree-sitter GC segfault (issue #116). 200 is well + above any real-world Python AST depth. + """ nonlocal current_class, current_fn_id + if depth > max_depth: + return if node.type == 'class_definition': # Track class context — also register the class itself as a node @@ -128,7 +174,8 @@ def _walk(node, class_name=None, fn_id=None): body = node.child_by_field_name('body') if body: for child in body.children: - _walk(child, class_name=cls_name, fn_id=fn_id) + _walk(child, class_name=cls_name, fn_id=fn_id, + depth=depth + 1, max_depth=max_depth) return elif node.type == 'function_definition': @@ -163,7 +210,8 @@ def _walk(node, class_name=None, fn_id=None): old_fn_id = current_fn_id current_fn_id = node_id for child in body.children: - _walk(child, class_name=class_name, fn_id=node_id) + _walk(child, class_name=class_name, fn_id=node_id, + depth=depth + 1, max_depth=max_depth) current_fn_id = old_fn_id return @@ -200,7 +248,8 @@ def _walk(node, class_name=None, fn_id=None): # Recurse into children for child in node.children: - _walk(child, class_name=class_name, fn_id=fn_id) + _walk(child, class_name=class_name, fn_id=fn_id, + depth=depth + 1, max_depth=max_depth) for child in root.children: _walk(child)