fix(point): fix reference counting issues#466
Merged
Conversation
haxtibal
changed the title
ObserverOfTime
requested changes
Jul 7, 2026
ObserverOfTime
left a comment
ObserverOfTime
left a comment
Member
There was a problem hiding this comment.
Please remove the reproducers.
bdc2367 ('feat!: add edit method to point & range') turned Point from a pure Python NamedTuple into a C extension tuple subtype with PyGetSetDef getters for Point.row and Point.column. According C-API getters must return a *new* reference [1], but a borrowed reference was returned instead where refcount is 1 too low. This can cause premature free and pymalloc free-list corruption, eventually leading to SIGSEGV. Fix point_get_row and point_get_column to return Py_NewRef instead of the bare borrowed result. [1] https://docs.python.org/3/c-api/structures.html#c.PyGetSetDef
haxtibal
force-pushed
the
bugfix/point_refcnt
branch
from
ceb845b to
6f31f0e
Compare
point_new constructed the tuple with PyTuple_Pack(2, row_obj, col_obj), which increments the refcount of each argument. The C locals row_obj and col_obj were never Py_DECREF'd afterward, so the row/column PyLong objects were held at refcount 2 instead of 1 and were never freed. Fix by adding Py_XDECREF(row_obj) and Py_XDECREF(col_obj) after PyTuple_Pack. Fixes bdc2367 ('feat!: add edit method to point & range').
point_new allocated the tuple with PyTuple_Pack and then called Py_SET_TYPE to change ob_type to the Point heap type. Py_SET_TYPE is a bare pointer write that skips the bookkeeping PyObject_Init performs for heap types, causing _PyObject_GenericGetAttrWithDict to crash on CPython 3.13 when accessing .row or .column on a Point built via the Python constructor. Fix point_new to mirror point_new_internal: replace PyTuple_Pack + Py_SET_TYPE with PyTuple_New + PyTuple_SET_ITEM + PyObject_Init. Fixes bdc2367 ('feat!: add edit method to point & range').
haxtibal
force-pushed
the
bugfix/point_refcnt
branch
from
6f31f0e to
645a28e
Compare
Contributor
Author
Yes, sorry, committing them was an oversight. I removed them also from the commit message because I first thought you were referring to that. Let me know if I should bring them back in the commit message. |
ObserverOfTime
approved these changes
Jul 8, 2026
Member
|
Thank you! |
ObserverOfTime
changed the title
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
2 participants
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.
See individual commit messages for details and reproducers.
The issues were spotted in StrictDoc CI when it started to pick up py-tree-sitter 0.26.0. See strictdoc-project/strictdoc#2986.