-
-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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
Flawed assumptions about tp_dictoffset
in inheritance.
#95589
Comments
Here's the weakref version:
|
Proposed fix3.11Revert the inheritance behavior from 3.10 3.12For 3.12 we should re-introduce the breaking, but not crashing, change and extend it to weakrefs. We then need to provide a better, and properly documented, API for for C extension classes to reliably |
+1. Though IMO it would be better to treat 3.10 behavior (only For the better API, my current thinking is adding API for clearing/visiting only a single type's data, so e.g. |
As only one base class can have opaque data, there would be only be one valid There are only about 10 or so valid permutations of pre-headers, opaque data, and slots, so we can generate a complete set of efficient visit, clear and dealloc functions for use in subclasses. |
Can you do that today/tomorrow, for the RC?
Only one direct base, but its base can also have opaque data. Still it'd be only one chain to walk for data&slots. |
Sure. |
That's the responsibility of the direct base, since both must be written in C to do that. |
True, but they might be from different libraries with different release schedules. The other library might be CPython, with its changing requirements on what needs to be visited – or it could be pybind11 or numpy or whatever, CPython shouldn't need special treatment here for its pre-headers. |
Superseded by #95596. |
That's not possible. If there were then the only way they could be combined is in Python, which would raise a TypeError due to the incompatible layout. |
… inheritance (GH-96028) * Treat tp_weakref and tp_dictoffset like other opaque slots for multiple inheritance. * Document Py_TPFLAGS_MANAGED_DICT and Py_TPFLAGS_MANAGED_WEAKREF in what's new.
In Python, the
__dict__
and__weakref__
slots are treated specially (slots meaning__slots__
, nottp_slots
)They are automatically insert by the VM when creating a class.
In order to support inheritance, specifically multiple inheritance, the VM can lay out subclasses in ways that differ from the superclass.
This is OK, provided
__dict__
and__weakref__
are only accessed though thetp_dictoffset
andtp_weaklistoffset
offsets.But, if either field is accessed directly, then we access invalid memory and 💥
test.py:
$ python3.10 ~/test/test.py Segmentation fault (core dumped)
We have (accidentally) fixed this for
__dict__
in 3.11, although at the expense breaking backwards compatibility for some C extensions. However, the problem still remains for__weakref__
.Backwards incompatibility
The text was updated successfully, but these errors were encountered: