The use of `PyStackRef_AsPyObjectBorrow` makes it hard to track ownership of references, making analysis of escaping calls too difficult
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 77.2k
- Forks
- 35.9k
- PR merge metrics
- PR metrics pending
Description
The problem with PyStackRef_AsPyObjectBorrow is that it is not clear whether the reference is owned by the stack ref or the pointer.
Borrowing references across calls is fine but in less structured code, it is error prone and very hard to analyze.
The solution is change most, ideally all, uses of PyStackRef_AsPyObjectBorrow to PyStackRef_AsPyObjectSteal so that the ownership of the reference is clear.
E.g.
inst(UNARY_NEGATIVE, (value -- res)) {
PyObject *val_o = PyStackRef_AsPyObjectBorrow(value);
PyObject *res_o = PyNumber_Negative(val_o);
PyStackRef_CLOSE(value);
ERROR_IF(res_o == NULL, error);
res = PyStackRef_FromPyObjectSteal(res_o);
}
would become:
inst(UNARY_NEGATIVE, (value -- res)) {
PyObject *val_o = PyStackRef_AsPyObjectSteal(value);
PyObject *res_o = PyNumber_Negative(val_o);
Py_DECREF(val_o);
ERROR_IF(res_o == NULL, error);
res = PyStackRef_FromPyObjectSteal(res_o);
}
This ensures that during the escaping call to PyNumber_Negative, the reference to the value a PyObject *, so will not be reclaimed by the garbage collector.
Linked PRs
- gh-122037
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Search the CPython source for uses of PyStackRef_AsPyObjectBorrow and review the PyStackRef_AsPyObjectSteal and PyStackRef_CLOSE ownership patterns shown in the issue. Determine which uses cross calls that can escape, then verify that the relevant references remain valid and ownership is explicit; compare the work with linked PR gh-122037.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, python
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100