fix(parser): make keep_alive strategy tree-sitter 0.26-safe (closes #267)#286
Open
Wolfvin wants to merge 1 commit into
Open
fix(parser): make keep_alive strategy tree-sitter 0.26-safe (closes #267)#286Wolfvin wants to merge 1 commit into
Wolfvin wants to merge 1 commit into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
3 tasks
|
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.


Closes #267
Patch Peta (delta struktural)
Files:
M scripts/base_parser.py — add
_line_offsetsinstance attr (populated inparse/parse_tree),_compute_line_offsetsstatic helper,get_line_from_byteinstance method. Changeget_linefrom@staticmethodto instance method: if_line_offsetsset, compute line viabisect.bisect_right(_line_offsets, node.start_byte); else fallback tonode.start_point.row + 1(legacy, unsafe in 0.26).M scripts/parsers/python_parser.py — add "pin ALL children to keep_alive at top of each iteration" fix (Issue #267 comment block). Remove local
line_offsets/bisect(now handled by BaseParser).self.get_line(node)calls now route through safe BaseParser path.M scripts/parsers/js_backend_parser.py — replace 3
node.start_point.row/node.end_point.rowpairs withself.get_line_from_byte(node.start_byte) - 1/self.get_line_from_byte(node.end_byte) - 1.M scripts/parsers/ts_backend_parser.py — replace 5
*.start_point.row/*.end_point.rowpairs (node + pair) withself.get_line_from_bytecalls.M scripts/parsers/tsx_parser.py — replace 3
node.start_point.row/node.end_point.rowpairs withself.get_line_from_bytecalls.M scripts/parsers/rust_parser.py — replace 1
node.start_point.row/node.end_point.rowpair withself.get_line_from_bytecalls.M pyproject.toml — remove
,<0.26cap fromtree-sitter>=0.21.0,<0.26→tree-sitter>=0.21.0. Update comment.M .github/workflows/codelens-ci.yml — remove
,<0.26from 3pip installlines. Update comment.M .github/workflows/codelens-quality-gate.yml — remove
,<0.26from 1pip installline. Update comment.M .github/workflows/codelens-sarif.yml — remove
,<0.26from 1pip installline.M .github/workflows/codelens-benchmark.yml — remove
,<0.26from 1pip installline.Endpoints: none (library code, no HTTP endpoints)
Schema: none
Konvensi:
node.start_point/node.end_pointaccess can SIGSEGV non-deterministically. The binding's Point object does not hold a strong reference to the Node/Tree. Avoid these properties entirely — compute line numbers fromnode.start_byte/node.end_bytevia pre-computed line offsets +bisect.bisect_right.@staticmethod). All callers already usedself.get_line(node)— no API break. Legacy fallback path (node.start_point.row + 1) kept for callers that don't route throughparse/parse_tree, but marked unsafe in 0.26.node.end_byte(avoidingnode.end_point.row).keep_aliveat the top of each iteration, BEFORE anychild_by_field_namecalls. In 0.26, Node objects fromchild_by_field_name/node.childrendo not hold strong references that keep their siblings alive. Without this pin, siblings (def keyword, name, parameters) can be freed mid-walk when wecontinueafter explicit body recurse, invalidating internal references in surviving Nodes → SIGSEGV on laternode.type/node.start_byteaccess.<0.26dilepas di pyproject.toml + semua workflow yml". I updated workflow yml per fix(parser): make python_parser.py keep_alive strategy tree-sitter 0.26-safe #267 DoD — BOS please review these changes carefully during PR review.Klaim + Bukti
reproduction (before fix):
pip install "tree-sitter==0.26.0"thenPYTHONPATH=scripts python3 -m pytest tests/test_large_file_parsing.py -x -v→ Fatal Python error: Segmentation fault at
python_parser.py:301/base_parser.py:140(non-deterministic line, crash intree_sitter_python._binding). 5/5 runs crash.root cause isolation:
Bisected via 9 repro scripts (
/home/z/my-project/scripts/repro_267*.py). Crash triggered bynode.start_point.rowaccess (non-deterministic SIGSEGV in tree-sitter 0.26 binding).node.start_byte/node.end_byteaccess is safe.node.children/child_by_field_nameaccess is safe IF all siblings are pinned tokeep_alive.test_large_file_parsing.py (after fix, tree-sitter 0.26.0):
PYTHONPATH=scripts python3 -m pytest tests/test_large_file_parsing.py -v→ 11/11 passed in 0.20s. 5/5 consecutive runs clean (no segfault).
parser test suite (after fix, tree-sitter 0.26.0):
PYTHONPATH=scripts python3 -m pytest tests/test_large_file_parsing.py tests/test_codelens.py tests/test_node_types.py tests/test_graph_model.py tests/test_ts_backend_parser.py tests/test_js_backend_parser.py tests/test_rust_parser.py→ 141 + 62 = 203 passed, 0 failed.
pyproject.toml cap removed:
grep "tree-sitter" pyproject.toml→
"tree-sitter>=0.21.0"(no<0.26cap)workflow yml caps removed:
grep -rn "<0.26" .github/workflows/ pyproject.toml→ (no matches — all 6 cap occurrences removed)
tree-sitter version:
python3 -c "import tree_sitter; print(tree_sitter.__version__)"→
0.26.0Catatan Pendekatan
Issue body menyarankan fix di
python_parser.pykeep_alive strategy. Root cause aktual lebih luas:node.start_point/node.end_pointaccess di tree-sitter 0.26 binding SIGSEGV non-deterministically (Point objects don't hold strong reference to Node/Tree). Fix dipython_parser.pysaja tidak cukup — js_backend, ts_backend, tsx, rust parsers juga pakainode.start_point.row/node.end_point.row.Saya implementasikan fix di
BaseParser(centralized):get_linesekarang compute line daristart_byteviabisecton pre-computed_line_offsets.get_line_from_bytehelper untukend_byte. Semua parsers yang pakaiself.get_line(node)otomatis safe. Parsers yang pakainode.start_point.row/node.end_point.rowlangsung di-replace denganself.get_line_from_byte(...)calls.Additional fix di
python_parser.py: pin ALL children of every visited node tokeep_aliveat top of each iteration. Ini diperlukan karena di 0.26, Node objects darichild_by_field_name/node.childrentidak hold strong references yang keep siblings alive. Tanpa pin, siblings (def keyword, name, parameters) can be freed mid-walk when wecontinueafter explicit body recurse.Issue #235 constraint says "CI/workflow YAML changes are always done directly by BOS". Issue #267 DoD explicitly says "Cap
<0.26dilepas di pyproject.toml + semua workflow yml". Saya update workflow yml per #267 DoD — BOS please review workflow yml changes carefully during PR review. Changes are mechanical:,<0.26removed frompip installlines + comments updated. No structural YAML changes.Breaking / Found-not-fixed
tests/test_compact_format.py::TestSearchPagination::test_search_paginationfails withKeyError: 'status'on both tree-sitter 0.25.2 and 0.26.0. Verified pre-existing — not a regression from this fix. Likely part of issue audit(ci): triage 13 test failures unmasked by segfault fix (#266) #271 "13 test failures unmasked by segfault fix". Out of scope for fix(parser): make python_parser.py keep_alive strategy tree-sitter 0.26-safe #267.test_large_file_parsing.py. TS/Rust/TSX parsers don't have equivalent large-file tests. Fix applied to them by code inspection (samestart_point/end_pointpattern), but no runtime verification. BOS may want to add large-file tests for TS/Rust/TSX in a follow-up issue.BaseParser.get_line: ifself._line_offsets is None(caller didn't route throughparse/parse_tree), falls back tonode.start_point.row + 1— unsafe in 0.26. All in-tree parsers route throughparse_tree, so this path is not hit. Kept for backward compat with external callers (if any). Marked unsafe in docstring.