8000 gh-134786: Py_TPFLAGS_MANAGED_WEAKREF implies Py_TPFLAGS_HAVE_GC too and force checking of its presence by sergey-miryanov · Pull Request #135863 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-134786: Py_TPFLAGS_MANAGED_WEAKREF implies Py_TPFLAGS_HAVE_GC too and force checking of its presence #135863

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

gh-134786: Py_TPFLAGS_MANAGED_WEAKREF implies Py_TPFLAGS_HAVE_GC too and force checking of its presence #135863

wants to merge 3 commits into from

Conversation

sergey-miryanov
Copy link
Contributor
@sergey-miryanov sergey-miryanov commented Jun 23, 2025 •
edited by github-actions bot
Loading

If we allow to use Py_TPFLAGS_MANAGED_WEAKREF without Py_TPFLAGS_HAVE_GC then we should change a way of calculation weakref list offsets.

cpython/Include/internal/pycore_object.h

Lines 804 to 847 in bda1218

/* Return the *address* of the object's weaklist. The address may be
* dereferenced to get the current head of the weaklist. This is useful
* for iterating over the linked list of weakrefs, especially when the
* list is being modified externally (e.g. refs getting removed).
*
* The returned pointer should not be used to change the head of the list
* nor should it be used to add, remove, or swap any refs in the list.
* That is the sole responsibility of the code in weakrefobject.c.
*/
static inline PyObject **
_PyObject_GET_WEAKREFS_LISTPTR(PyObject *op)
{
if (PyType_Check(op) &&
((PyTypeObject *)op)->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) {
PyInterpreterState *interp = _PyInterpreterState_GET();
managed_static_type_state *state = _PyStaticType_GetState(
interp, (PyTypeObject *)op);
return _PyStaticType_GET_WEAKREFS_LISTPTR(state);
}
// Essentially _PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET():
Py_ssize_t offset = Py_TYPE(op)->tp_weaklistoffset;
return (PyObject **)((char *)op + offset);
}
/* This is a special case of _PyObject_GET_WEAKREFS_LISTPTR().
* Only the most fundamental lookup path is used.
* Consequently, static types should not be used.
*
* For static builtin types the returned pointer will always point
* to a NULL tp_weaklist. This is fine for any deallocation cases,
* since static types are never deallocated and static builtin types
* are only finalized at the end of runtime finalization.
*
* If the weaklist for static types is actually needed then use
* _PyObject_GET_WEAKREFS_LISTPTR().
*/
static inline PyWeakReference **
_PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET(PyObject *op)
{
assert(!PyType_Check(op) ||
((PyTypeObject *)op)->tp_flags & Py_TPFLAGS_HEAPTYPE);
Py_ssize_t offset = Py_TYPE(op)->tp_weaklistoffset;
return (PyWeakReference **)((char *)op + offset);
}

As I understand from #95245 the main idea of moving dict pointer and weakref pointer to the preheader is to simplify access to them.

Another note: typeobject.c/compatible_for_assignment is checking Py_TPFLAGS_PREHEADER for lhs and rhs types. But Py_TPFLAGS_PREHEADER is a Py_TPFLAGS_MANAGED_WEAKREF | Py_TPFLAGS_MANAGED_DICT and might be true if one object is supporting GC and other not and we should handle this complicated case.

cpython/Objects/typeobject.c

Lines 7291 to 7296 in bda1218

/* The above does not check for the preheader */
if ((oldto->tp_flags & Py_TPFLAGS_PREHEADER) ==
((newto->tp_flags & Py_TPFLAGS_PREHEADER)))
{
return 1;
}

So, I propose to change requirements for Py_TPFLAGS_MANAGED_WEAKREF and also force checking of having Py_TPFLAGS_HAVE_GC set for Py_TPFLAGS_MANAGED_WEAKREF and Py_TPFLAGS_MANAGED_DICT.

📚 Documentation preview 📚: https://cpython-previews--135863.org.readthedocs.build/

@bedevere-app bedevere-app bot mentioned this pull request Jun 23, 2025
Open
@sergey-miryanov
Copy link
Contributor Author

@markshannon Please take a look.

@chris-se
Copy link

As the person who reported #134786: I would suggest making the documentation more explicit and replacing should with must here, especially since the code now errors out if Py_TPFLAGS_HAVE_GC is not set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Assignees
No one assigned
Labels
awaiting review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sergey-miryanov @chris-se
0