Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Py_HOLD_REF() macro to hold a strong reference to a Python object while executing code #99481

Closed
vstinner opened this issue Nov 14, 2022 · 3 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@vstinner
Copy link
Member

Reference counting in Python is complicated. It's never obvious if we are holding a strong reference or a borrowed reference to a Python object.

If we have a borrowed reference to a Python object, to make sure that it cannot be deleted while we execute arbitrary Python code, it's needed to creating a temporary strong reference by increasing temporarily its reference counter. Example from Modules/_abc.c:

    subclasses = PyObject_CallMethod(self, "__subclasses__", NULL);
    (...)
    PyObject *scls = PyList_GET_ITEM(subclasses, pos);

    Py_INCREF(scls);  // create a temporary strong reference
    int r = PyObject_IsSubclass(subclass, scls);
    Py_DECREF(scls);  // delete the temporary strong reference

The scls variable is assigned to a borrowed reference to a list item. The list is the result of the __subclasses__() method. The code creates a temporarily strong reference to call PyObject_IsSubclass().

IMO in terms of semantics, such code is hard to understand if read aloud in English, since INCREF and DECREF modify the object in-place. Py_INCREF() and Py_DECREF() are like the low-level implementation, I would prefer to have an abstraction on top of it.

That's also why I added Py_NewRef() in Python 3.10: Py_NewRef() semantics is simpler: it creates a new strong reference. In terms of semantics, it doesn't modify the object in-place, even if it's possible to write something like var = Py_NewRef(var) (I discourage to write such code). See issue #99300 for the usage of the Py_NewRef() function.

It would be nice to have a macro to clarify the scope of the temporary strong reference rather than having to INCREF/DECREF temporarily. Something like:

#define Py_HOLD_REF(var, code) \
    Py_INCREF(var); \
    code; \
    Py_DECREF(var)

Example of usage:

Py_HOLD_REF(scls,
    int r = PyObject_IsSubclass(subclass, scls);
);

Example holding two (different) strong references:

Py_HOLD_REF(keys,
    Py_HOLD_REF(values,
        PyObject *res = PyTuple_Pack(2, keys, values);
    )    // note: there is no ";" here
);

Alternative compact syntax:

Py_HOLD_REF(keys, Py_HOLD_REF(values,
    PyObject *res = PyTuple_Pack(2, keys, values);
));

Obviously, the macro name is open for bikeshedding :-)

@vstinner vstinner added the type-bug An unexpected behavior, bug, or error label Nov 14, 2022
@vstinner
Copy link
Member Author

See also issue #98724 and comments on my PR #99100 which discuss Py_CLEAR(), Py_SETREF() and Py_XSETREF().

@vstinner
Copy link
Member Author

I'm not sure that such macro makes the code easier to understand or to maintain. I close the issue.

@vstinner
Copy link
Member Author

In programming languages like C++, it would be possible to create such API by relying on destructors. There are CPython C++ binding which uses such API to hold the GIL for example: the GIL is released at the end of the scope of a variable which "holds the GIL".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

1 participant