classification
Title: Py_LIMITED_API needs a PyType_GenericDealloc
Type: enhancement Stage:
Components: Versions: Python 3.5, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: bfroehle, haypo, larry, loewis, python-dev
Priority: normal Keywords: patch

Created on 2013-02-08 18:27 by bfroehle, last changed 2014-02-04 08:49 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
getslot.diff loewis, 2014-01-28 08:20 review
getslot2.diff loewis, 2014-01-30 14:23 review
Messages (14)
msg181689 - (view) Author: Bradley Froehle (bfroehle) * Date: 2013-02-08 18:27
I tried to implement a custom extension type using PyType_FromSpec and Py_LIMITED_API but couldn't implement tp_dealloc:

static void mytype_dealloc(mytypeobject *self)
{
    // free some fields in mytypeobject
    Py_TYPE(self)->tp_free((PyObject *) self); // XXX
}

The line marked XXX will not compile in Py_LIMITED_API because there is no access to the fields of PyTypeObject.  There doesn't seem to be any function in the API which just calls tp_free.

I suggest the addition of an API function (to be included in Py_LIMITED_API):

void
PyType_GenericDealloc(PyObject *self)
{
    Py_TYPE(self)->tp_free(self);
}
msg181690 - (view) Author: Bradley Froehle (bfroehle) * Date: 2013-02-08 18:30
I should mention that essentially what I'm advocating is renaming and exposing `object_dealloc` in Objects/typeobject.c.

The proper name is not obvious to me... should it be PyObject_GenericDealloc since it acts on objects? Or PyType_GenericDealloc since it will be stuck into one of the PyTypeObject slots?
msg209507 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-01-28 08:20
I propose a more general solution: add a function PyType_GetSlot.
msg209514 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2014-01-28 08:39
+    return  *(void**)(((char*)type) + slotoffsets[slot]);

New Python versions may add new slots. What do you think of returning NULL if the slot number is higher than the maximum slot?

It looks like "#define Py_tp_free 74" is the highest slot number since Python 3.2.

For example, Python 3.4 has a new "tp_finalize" slot, but I don't see it in typeslots.h.
msg209719 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-01-30 14:23
If the set of slots gets extended, extensions would have to opt-in to use the newer slots, i.e. the availability of slot numbers should be conditional at compile-time.

Returning 0 for slots not supported in a version seems fine to me as well (after reconsideration), as this is also what you would get if you just recompiled the old type with the new Python headers (i.e. all fields added at the end are 0-initialized).

As for slots added to 3.4: it would certainly possible to add them to the stable ABI, if we indeed trust that they are stable (i.e. won't go away until 4.0). That would have to be decided on a slot-by-slot case, preferably in individual roundup issues.
msg209722 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-30 16:22
I thought I replied to this... weird.

Do I understand correctly that it's basically impossible to write a proper custom deallocator in the limited API right now, because you can't get access to your base class's tp_free?

(If so, why didn't anybody notice?)
msg209725 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-30 16:27
Also, isn't it reasonable to pass in non-heap type objects?  I realize supporting this would complicate the implementation a great deal.
msg209769 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-01-31 11:41
It's certainly possible to write a "proper" dealloc - just call PyObject_Del directly. This is correct if the type isn't subclassable, and works if it actually isn't subclassed, or if the subclass object can also be released through PyObject_Del.

That this has *nobody* noticed yet isn't the case - the OP certainly noticed a year ago. Else, the limited API isn't in wide use yet, probably partly because it is too limited still for certain extension modules (but it is by design that it is limited at all, so that code might require active porting to use it, and may not be portable at all in certain cases).

If the typical use case is PyTYPE(self)->tp_free, then the type ought to be a heap type, so limiting the implementation to heap types should be sufficient. It would be feasible to extend it to non-heaptypes, although I do wonder what use case this would allow for.
msg209770 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-31 11:49
I was thinking of a subclass of an existing class, both implemented in C.  collections.Counter does that, but it uses _PyType_LookupId() which is in the limited API.

Would it be possible to achieve equivalent functionality by using _PyType_LookupId()?  If so, maybe instead of the proposed new function PyType_GetSlot, we should make _PyType_Lookup and _PyType_LookupId part of the public API.
msg209771 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-31 11:49
(And, I freely admit I don't understand all this that well and that could be a very dumb question.)
msg209813 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-01-31 19:21
I'm not sure we are looking at the same code base, I look at

http://hg.python.org/cpython/file/b56ce3410ca6/Lib/collections/__init__.py#l401

and ISTM that collections.Counter is *not* implemented in C. Also, according to

http://hg.python.org/cpython/file/b56ce3410ca6/Include/object.h#l486

I see that _PyType_LookupId is *not* in the limited API (and it really shouldn't).

In any case, _PyType_LookupId cannot replace PyType_GetSlot, since it returns a PyObject* from the namespace of the type. Many of the slots don't actually have a Python name (including tp_free, which is the OP's original use case), plus the value returned ought to be a function pointer, not a PyObject*.
msg210182 - (view) Author: Roundup Robot (python-dev) Date: 2014-02-04 08:33
New changeset 655d7a55c165 by Martin v. Löwis in branch 'default':
Issue #17162: Add PyType_GetSlot.
http://hg.python.org/cpython/rev/655d7a55c165
msg210183 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-02-04 08:33
Thanks for the reviews; this is now committed.
msg210184 - (view) Author: Roundup Robot (python-dev) Date: 2014-02-04 08:49
New changeset eaae4008327d by Victor Stinner in branch 'default':
Issue #17162: Fix compilation, replace non-breaking space with an ASCII space
http://hg.python.org/cpython/rev/eaae4008327d
History
Date User Action Args
2014-02-04 08:49:29python-devsetmessages: + msg210184
2014-02-04 08:33:50loewissetstatus: open -> closed
resolution: fixed
messages: + msg210183
2014-02-04 08:33:22python-devsetnosy: + python-dev
messages: + msg210182
2014-01-31 19:21:38loewissetmessages: + msg209813
2014-01-31 11:49:55larrysetmessages: + msg209771
2014-01-31 11:49:31larrysetmessages: + msg209770
2014-01-31 11:41:26loewissetmessages: + msg209769
2014-01-30 16:27:55larrysetmessages: + msg209725
2014-01-30 16:22:10larrysetmessages: + msg209722
2014-01-30 14:23:43loewissetfiles: + getslot2.diff

messages: + msg209719
2014-01-29 02:41:51larrysetnosy: + larry
2014-01-28 08:39:17hayposetnosy: + haypo
messages: + msg209514
2014-01-28 08:20:33loewissetfiles: + getslot.diff
keywords: + patch
messages: + msg209507
2013-02-08 18:51:55pitrousetnosy: + loewis
2013-02-08 18:30:16bfroehlesetmessages: + msg181690
2013-02-08 18:27:15bfroehlecreate