Skip to content

[C API] Make Py_TYPE() opaque in limited C API 3.14 #120600

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

Closed
vstinner opened this issue Jun 16, 2024 · 11 comments
Closed

[C API] Make Py_TYPE() opaque in limited C API 3.14 #120600

vstinner opened this issue Jun 16, 2024 · 11 comments

Comments

@vstinner
Copy link
Member

vstinner commented Jun 16, 2024

In the limited C API 3.14 and newer, I propose to change Py_TYPE() and Py_SET_TYPE() implementation to opaque function calls to hide implementation details. I made a similar change for Py_REFCNT() and Py_SET_REFCNT() in Python 3.12.

The problem is that with Free Threading (PEP 703), the implementation of these functions become less trivial than just getting/setting an object member:

static inline PyTypeObject* Py_TYPE(PyObject *ob) {
    return (PyTypeObject *)_Py_atomic_load_ptr_relaxed(&ob->ob_type);
}

static inline void Py_SET_TYPE(PyObject *ob, PyTypeObject *type) {
    _Py_atomic_store_ptr(&ob->ob_type, type);
}

_Py_atomic_load_ptr_relaxed() and _Py_atomic_store_ptr() must now be called. But I would prefer to not "leak" such implementation detail into the limited C API.

cc @colesbury @Fidget-Spinner

Linked PRs

vstinner added a commit to vstinner/cpython that referenced this issue Jun 16, 2024
In the limited C API 3.14 and newer, Py_TYPE() and Py_SET_TYPE() are
now implemented as opaque function calls to hide implementation
details.
vstinner added a commit to vstinner/cpython that referenced this issue Jun 16, 2024
In the limited C API 3.14 and newer, Py_TYPE() and Py_SET_TYPE() are
now implemented as opaque function calls to hide implementation
details.

Add _Py_TYPE() and _Py_SET_TYPE() private functions.
@encukou
Copy link
Member

encukou commented Jun 17, 2024

I'd prefer if Py_TYPE was a public exported function. If it remains a macro, non-C languages will still need to access ob_type directly (or start using private API).

As for Py_SET_TYPE, you need knowledge of the internals to use it correctly. I support changing it to call an opaque function, but unlike Py_TYPE I'd prefer not exposing any more than it is (so: keep it as a macro).
The proper way to change a type is currently setting the __class__ attribute. We could add a new API function, like PyObject_SetType, that does the required checks and updates. And then deprecate Py_SET_TYPE.

Please add tests for the old implementation (getting/setting ob_type directly), to ensure existing extensions aren't unintentionally broken.

@vstinner
Copy link
Member Author

Please add tests for the old implementation (getting/setting ob_type directly), to ensure existing extensions aren't unintentionally broken.

_testcapi has test_set_type_size() which tests Py_TYPE() and Py_SET_TYPE():

    // Ensure that following tests don't modify the object,
    // to ensure that Py_DECREF() will not crash.
    assert(Py_TYPE(obj) == &PyList_Type);
    assert(Py_SIZE(obj) == 0);

    // bpo-39573: Test Py_SET_TYPE() and Py_SET_SIZE() functions.
    Py_SET_TYPE(obj, &PyList_Type);
    Py_SET_SIZE(obj, 0);

vstinner added a commit to vstinner/cpython that referenced this issue Jun 17, 2024
In the limited C API 3.14 and newer, Py_TYPE() is now implemented as
an opaque function call to hide implementation details.
@vstinner
Copy link
Member Author

vstinner commented Jun 17, 2024

I modified my PR to only reuse Py_TYPE() name (instead of adding a _Py_TYPE() function) and to leave Py_SET_TYPE() unchanged.

@encukou
Copy link
Member

encukou commented Jun 17, 2024

Sounds good, thanks!

I don't think it'll solve thread safety -- Py_TYPE returns a borrowed reference, and something could invalidate that reference while you're using it. But for the use cases where Py_TYPE is already safe, it's nice to have functions rather than macros in the limited API.

@vstinner
Copy link
Member Author

I don't think it'll solve thread safety -- Py_TYPE returns a borrowed reference

PyObject_Type(obj) already exists to get a strong reference.

I also wrote PEP 737 "C API to format a type fully qualified name" to avoid a common usage of Py_TYPE() borrowed reference. (The old usage can lead to a crash, see the PEP!)

@vstinner
Copy link
Member Author

Related change done in Python 3.12: Limited C API: implement Py_INCREF() and Py_DECREF() as function calls.

vstinner added a commit that referenced this issue Jun 18, 2024
In the limited C API 3.14 and newer, Py_TYPE() is now implemented as
an opaque function call to hide implementation details.
picnixz pushed a commit to picnixz/cpython that referenced this issue Jun 19, 2024
…120601)

In the limited C API 3.14 and newer, Py_TYPE() is now implemented as
an opaque function call to hide implementation details.
@nineteendo
Copy link
Contributor

Is there something left to do here?

@vstinner
Copy link
Member Author

Fixed by change 16f8e22.

@encukou
Copy link
Member

encukou commented Jun 20, 2024

There's still Py_SET_TYPE, which IMO could use a private opaque function.
Should that be done with a new issue?

@vstinner
Copy link
Member Author

There's still Py_SET_TYPE, which IMO could use a private opaque function. Should that be done with a new issue?

I suggest to open a new issue. If I understood correctly, Py_SET_TYPE() should not exist in the limited C API, since static types cannot be defined with the limited C API.

mrahtz pushed a commit to mrahtz/cpython that referenced this issue Jun 30, 2024
…120601)

In the limited C API 3.14 and newer, Py_TYPE() is now implemented as
an opaque function call to hide implementation details.
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
…120601)

In the limited C API 3.14 and newer, Py_TYPE() is now implemented as
an opaque function call to hide implementation details.
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
…120601)

In the limited C API 3.14 and newer, Py_TYPE() is now implemented as
an opaque function call to hide implementation details.
@vstinner
Copy link
Member Author

I created an issue for Py_REFCNT(): #124127

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants