This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2012年06月22日 19:56 by pitrou, last changed 2022年04月11日 14:57 by admin. This issue is now closed.
| Messages (9) | |||
|---|---|---|---|
| msg163470 - (view) | Author: Antoine Pitrou (pitrou) * (Python committer) | Date: 2012年06月22日 19:56 | |
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; |
|||
| msg163532 - (view) | Author: Alyssa Coghlan (ncoghlan) * (Python committer) | Date: 2012年06月23日 05:59 | |
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: - ensuring __new__ is a static method - ensuring the standard attribute lookup machinery is configured - hooking up tp_as_number, tp_as_mapping, etc - ensuring GC support is configured correctly 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. |
|||
| msg163541 - (view) | Author: Martin v. Löwis (loewis) * (Python committer) | Date: 2012年06月23日 07:32 | |
> - ensuring __new__ is a static method This shouldn't be necessary. __new__ won't be a method at all, and not even exist. Instead, a type may or may not fill the tp_new slot. > - ensuring the standard attribute lookup machinery is configured This is what PyType_Ready does, no? > - hooking up tp_as_number, tp_as_mapping, etc This is indeed missing. Robin Schreiber is working on a patch. > - ensuring GC support is configured correctly This is the responsibility of the caller, as always with C types. |
|||
| msg163542 - (view) | Author: Martin v. Löwis (loewis) * (Python committer) | Date: 2012年06月23日 07:33 | |
In any case, one issue at a time, please. This issues is about a reference leak. |
|||
| msg163552 - (view) | Author: Alyssa Coghlan (ncoghlan) * (Python committer) | Date: 2012年06月23日 08:42 | |
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 #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: /* Sanity check for tp_dealloc. */ if ((type->tp_flags & Py_TPFLAGS_HEAPTYPE) && (type->tp_dealloc == type_dealloc)) { /* Type has been declared as a heap type, but has inherited the default allocator. This can happen when using the limited API to dynamically create types. */ type->tp_dealloc = subtype_dealloc; } |
|||
| msg163563 - (view) | Author: Antoine Pitrou (pitrou) * (Python committer) | Date: 2012年06月23日 09:40 | |
> However, once inheritance support is added by #15146 then it would be > wrong - the default slot entry would override an inherited one. It would not be wrong. subtype_dealloc will properly call a base class' tp_dealloc, if necessary. |
|||
| msg163572 - (view) | Author: Alyssa Coghlan (ncoghlan) * (Python committer) | Date: 2012年06月23日 10:15 | |
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" |
|||
| msg163600 - (view) | Author: Roundup Robot (python-dev) (Python triager) | Date: 2012年06月23日 12:48 | |
New changeset 1794308c1ea7 by Antoine Pitrou in branch '3.2': Issue #15142: Fix reference leak when deallocating instances of types created using PyType_FromSpec(). http://hg.python.org/cpython/rev/1794308c1ea7 New changeset 9945d7dfa72c by Antoine Pitrou in branch 'default': Issue #15142: Fix reference leak when deallocating instances of types created using PyType_FromSpec(). http://hg.python.org/cpython/rev/9945d7dfa72c |
|||
| msg163601 - (view) | Author: Antoine Pitrou (pitrou) * (Python committer) | Date: 2012年06月23日 12:49 | |
Ok, fixed, thanks. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:57:31 | admin | set | github: 59347 |
| 2012年06月23日 12:49:42 | pitrou | set | status: open -> closed resolution: fixed messages: + msg163601 stage: patch review -> resolved |
| 2012年06月23日 12:48:55 | python-dev | set | nosy:
+ python-dev messages: + msg163600 |
| 2012年06月23日 10:15:37 | ncoghlan | set | messages: + msg163572 |
| 2012年06月23日 09:40:12 | pitrou | set | messages: + msg163563 |
| 2012年06月23日 08:42:36 | ncoghlan | set | messages: + msg163552 |
| 2012年06月23日 07:33:56 | loewis | set | messages: + msg163542 |
| 2012年06月23日 07:33:04 | loewis | set | nosy:
+ Robin.Schreiber |
| 2012年06月23日 07:32:34 | loewis | set | messages: + msg163541 |
| 2012年06月23日 06:01:49 | ncoghlan | set | nosy:
+ daniel.urban |
| 2012年06月23日 05:59:54 | ncoghlan | set | nosy:
+ ncoghlan messages: + msg163532 |
| 2012年06月22日 19:56:06 | pitrou | create | |