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
Fix reference leak with types created using PyType_FromSpec #59347
Comments
The following patch seems to fix the issue explained in http://mail.python.org/pipermail/python-dev/2012-June/120659.html : diff --git a/Objects/typeobject.c b/Objects/typeobject.c
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -2417,6 +2417,10 @@ PyType_FromSpec(PyType_Spec *spec)
if (res->ht_type.tp_dictoffset) {
res->ht_cached_keys = _PyDict_NewKeysForClass();
}
+ if (res->ht_type.tp_dealloc == NULL) {
+ /* It's a heap type, so needs the heap types' dealloc */
+ res->ht_type.tp_dealloc = subtype_dealloc;
+ }
if (PyType_Ready(&res->ht_type) < 0)
goto fail; |
That does look like it will fix the leak, but now I'm actually thinking there's more code from type_new that should also be executed in the PyType_FromSpec case. I mean things like:
If that's all happening somehow, it could use a comment, because I certainly can't see it. If not, we probably need to factor out some helper functions that type_new and PyType_FromSpec can both call to make sure everything is fully configured. |
This shouldn't be necessary. __new__ won't be a method at all,
This is what PyType_Ready does, no?
This is indeed missing. Robin Schreiber is working on a patch.
This is the responsibility of the caller, as always with C types. |
In any case, one issue at a time, please. This issues is about a reference leak. |
You're right, I was confusing what happens automatically for classes defined in Python (i.e. the full treatment in type_new) vs those defined statically (i.e. just the parts in PyType_Ready). Given that PyType_FromSpec doesn't currently support inheritance, providing a default tp_dealloc before the inherit_slots() call in PyType_Ready would work OK in the near term. However, once inheritance support is added by bpo-15146 then it would be wrong - the default slot entry would override an inherited one. So, I think this adjustment actually needs to be handled in PyType_Ready, at some point after the inherit_slots() call. Something like:
|
It would not be wrong. subtype_dealloc will properly call a base class' |
True, I didn't follow the bouncing ball far enough. In that, case I think all that is needed is a comment like: "subtype_dealloc walks the MRO to call the base dealloc function, so it is OK to block inheritance of the slot" |
New changeset 1794308c1ea7 by Antoine Pitrou in branch '3.2': New changeset 9945d7dfa72c by Antoine Pitrou in branch 'default': |
Ok, fixed, thanks. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: