The use of `PyStackRef_AsPyObjectBorrow` makes it hard to track ownership of references, making analysis of escaping calls too difficult

[markshannon]

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-122034: Add StackRef variants of type checks to reduce the number of PyStackRef_AsPyObjectBorrow calls #122037

[colesbury]

I don't think we want to rewrite borrows as steals -- the problem is that this has a performance impact in the free-threaded build:

• It converts a deferred reference to a strong reference, which can inhibit scaling
• Even for non-deferred references, there's the extra cost of checking if it's deferred

I agree that replacing borrows with variants take _PyStackRef is generally a good idea, like you're doing in gh-122037.

[markshannon]

I don't think we want to rewrite borrows as steals -- the problem is that this has a performance impact in the free-threaded build

The problem is the lack of clarity about ownership.
Converting to steals doesn't have to be the solution, it is just one possible solution. Any alternative ideas?

[encukou]

Should we close this, or does it need more discussion?