[Python-checkins] bpo-45315: PyType_FromSpec: Copy spec->name and have the type own the memory for its name (GH-29103)
encukou
webhook-mailer at python.org
Thu Oct 21 05:46:31 EDT 2021
https://github.com/python/cpython/commit/8a310dd5f4242b5d28013c1905c6573477e3b957
commit: 8a310dd5f4242b5d28013c1905c6573477e3b957
branch: main
author: Petr Viktorin <encukou at gmail.com>
committer: encukou <encukou at gmail.com>
date: 2021-10-21T11:46:20+02:00
summary:
bpo-45315: PyType_FromSpec: Copy spec->name and have the type own the memory for its name (GH-29103)
files:
A Misc/NEWS.d/next/C API/2021-10-20-18-41-17.bpo-29103.CMRLyq.rst
M Include/cpython/object.h
M Lib/test/test_sys.py
M Modules/_testcapimodule.c
M Objects/typeobject.c
diff --git a/Include/cpython/object.h b/Include/cpython/object.h
index 849d5aa6a2c72..3a8a256e3b9ae 100644
--- a/Include/cpython/object.h
+++ b/Include/cpython/object.h
@@ -290,6 +290,7 @@ typedef struct _heaptypeobject {
PyObject *ht_name, *ht_slots, *ht_qualname;
struct _dictkeysobject *ht_cached_keys;
PyObject *ht_module;
+ char *_ht_tpname; // Storage for "tp_name"; see PyType_FromModuleAndSpec
/* here are optional user slots, followed by the members. */
} PyHeapTypeObject;
diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py
index 2ce40fcde9eb2..2182898837e50 100644
--- a/Lib/test/test_sys.py
+++ b/Lib/test/test_sys.py
@@ -1419,7 +1419,7 @@ def delx(self): del self.__x
'3P' # PyMappingMethods
'10P' # PySequenceMethods
'2P' # PyBufferProcs
- '5P')
+ '6P')
class newstyleclass(object): pass
# Separate block for PyDictKeysObject with 8 keys and 5 entries
check(newstyleclass, s + calcsize(DICT_KEY_STRUCT_FORMAT) + 32 + 21*calcsize("n2P"))
diff --git a/Misc/NEWS.d/next/C API/2021-10-20-18-41-17.bpo-29103.CMRLyq.rst b/Misc/NEWS.d/next/C API/2021-10-20-18-41-17.bpo-29103.CMRLyq.rst
new file mode 100644
index 0000000000000..e923bfd3ae293
--- /dev/null
+++ b/Misc/NEWS.d/next/C API/2021-10-20-18-41-17.bpo-29103.CMRLyq.rst
@@ -0,0 +1,3 @@
+:c:func:`PyType_FromSpec* <PyType_FromModuleAndSpec>` now copies the class name
+from the spec to a buffer owned by the class, so the original can be safely
+deallocated. Patch by Petr Viktorin.
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 6857241999ea9..5e4c577962601 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -1163,6 +1163,126 @@ test_get_type_name(PyObject *self, PyObject *Py_UNUSED(ignored))
}
+static PyObject *
+simple_str(PyObject *self) {
+ return PyUnicode_FromString("<test>");
+}
+
+
+static PyObject *
+test_type_from_ephemeral_spec(PyObject *self, PyObject *Py_UNUSED(ignored))
+{
+ // Test that a heap type can be created from a spec that's later deleted
+ // (along with all its contents).
+ // All necessary data must be copied and held by the class
+ PyType_Spec *spec = NULL;
+ char *name = NULL;
+ char *doc = NULL;
+ PyType_Slot *slots = NULL;
+ PyObject *class = NULL;
+ PyObject *instance = NULL;
+ PyObject *obj = NULL;
+ PyObject *result = NULL;
+
+ /* create a spec (and all its contents) on the heap */
+
+ const char NAME[] = "testcapi._Test";
+ const char DOC[] = "a test class";
+
+ spec = PyMem_New(PyType_Spec, 1);
+ if (spec == NULL) {
+ PyErr_NoMemory();
+ goto finally;
+ }
+ name = PyMem_New(char, sizeof(NAME));
+ if (name == NULL) {
+ PyErr_NoMemory();
+ goto finally;
+ }
+ memcpy(name, NAME, sizeof(NAME));
+
+ doc = PyMem_New(char, sizeof(DOC));
+ if (name == NULL) {
+ PyErr_NoMemory();
+ goto finally;
+ }
+ memcpy(doc, DOC, sizeof(DOC));
+
+ spec->name = name;
+ spec->basicsize = sizeof(PyObject);
+ spec->itemsize = 0;
+ spec->flags = Py_TPFLAGS_DEFAULT;
+ slots = PyMem_New(PyType_Slot, 3);
+ if (slots == NULL) {
+ PyErr_NoMemory();
+ goto finally;
+ }
+ slots[0].slot = Py_tp_str;
+ slots[0].pfunc = simple_str;
+ slots[1].slot = Py_tp_doc;
+ slots[1].pfunc = doc;
+ slots[2].slot = 0;
+ slots[2].pfunc = NULL;
+ spec->slots = slots;
+
+ /* create the class */
+
+ class = PyType_FromSpec(spec);
+ if (class == NULL) {
+ goto finally;
+ }
+
+ /* deallocate the spec (and all contents) */
+
+ // (Explicitly ovewrite memory before freeing,
+ // so bugs show themselves even without the debug allocator's help.)
+ memset(spec, 0xdd, sizeof(PyType_Spec));
+ PyMem_Del(spec);
+ spec = NULL;
+ memset(name, 0xdd, sizeof(NAME));
+ PyMem_Del(name);
+ name = NULL;
+ memset(doc, 0xdd, sizeof(DOC));
+ PyMem_Del(doc);
+ doc = NULL;
+ memset(slots, 0xdd, 3 * sizeof(PyType_Slot));
+ PyMem_Del(slots);
+ slots = NULL;
+
+ /* check that everything works */
+
+ PyTypeObject *class_tp = (PyTypeObject *)class;
+ PyHeapTypeObject *class_ht = (PyHeapTypeObject *)class;
+ assert(strcmp(class_tp->tp_name, "testcapi._Test") == 0);
+ assert(strcmp(PyUnicode_AsUTF8(class_ht->ht_name), "_Test") == 0);
+ assert(strcmp(PyUnicode_AsUTF8(class_ht->ht_qualname), "_Test") == 0);
+ assert(strcmp(class_tp->tp_doc, "a test class") == 0);
+
+ // call and check __str__
+ instance = PyObject_CallNoArgs(class);
+ if (instance == NULL) {
+ goto finally;
+ }
+ obj = PyObject_Str(instance);
+ if (obj == NULL) {
+ goto finally;
+ }
+ assert(strcmp(PyUnicode_AsUTF8(obj), "<test>") == 0);
+ Py_CLEAR(obj);
+
+ result = Py_NewRef(Py_None);
+ finally:
+ PyMem_Del(spec);
+ PyMem_Del(name);
+ PyMem_Del(doc);
+ PyMem_Del(slots);
+ Py_XDECREF(class);
+ Py_XDECREF(instance);
+ Py_XDECREF(obj);
+ return result;
+}
+
+
static PyObject *
test_get_type_qualname(PyObject *self, PyObject *Py_UNUSED(ignored))
{
@@ -5804,6 +5924,7 @@ static PyMethodDef TestMethods[] = {
{"test_get_statictype_slots", test_get_statictype_slots, METH_NOARGS},
{"test_get_type_name", test_get_type_name, METH_NOARGS},
{"test_get_type_qualname", test_get_type_qualname, METH_NOARGS},
+ {"test_type_from_ephemeral_spec", test_type_from_ephemeral_spec, METH_NOARGS},
{"get_kwargs", (PyCFunction)(void(*)(void))get_kwargs,
METH_VARARGS|METH_KEYWORDS},
{"getargs_tuple", getargs_tuple, METH_VARARGS},
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index aa0733361e58d..18bea476ea9f2 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -2778,6 +2778,7 @@ type_new_alloc(type_new_ctx *ctx)
et->ht_name = Py_NewRef(ctx->name);
et->ht_module = NULL;
+ et->_ht_tpname = NULL;
return type;
}
@@ -3435,25 +3436,42 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases)
goto fail;
}
+ type = &res->ht_type;
+ /* The flags must be initialized early, before the GC traverses us */
+ type->tp_flags = spec->flags | Py_TPFLAGS_HEAPTYPE;
+
/* Set the type name and qualname */
const char *s = strrchr(spec->name, '.');
- if (s == NULL)
+ if (s == NULL) {
s = spec->name;
- else
+ }
+ else {
s++;
+ }
- type = &res->ht_type;
- /* The flags must be initialized early, before the GC traverses us */
- type->tp_flags = spec->flags | Py_TPFLAGS_HEAPTYPE;
res->ht_name = PyUnicode_FromString(s);
- if (!res->ht_name)
+ if (!res->ht_name) {
+ goto fail;
+ }
+ res->ht_qualname = Py_NewRef(res->ht_name);
+
+ /* Copy spec->name to a buffer we own.
+ *
+ * Unfortunately, we can't use tp_name directly (with some
+ * flag saying that it should be deallocated with the type),
+ * because tp_name is public API and may be set independently
+ * of any such flag.
+ * So, we use a separate buffer, _ht_tpname, that's always
+ * deallocated with the type (if it's non-NULL).
+ */
+ Py_ssize_t name_buf_len = strlen(spec->name) + 1;
+ res->_ht_tpname = PyMem_Malloc(name_buf_len);
+ if (res->_ht_tpname == NULL) {
goto fail;
- res->ht_qualname = res->ht_name;
- Py_INCREF(res->ht_qualname);
- type->tp_name = spec->name;
+ }
+ type->tp_name = memcpy(res->_ht_tpname, spec->name, name_buf_len);
- Py_XINCREF(module);
- res->ht_module = module;
+ res->ht_module = Py_XNewRef(module);
/* Adjust for empty tuple bases */
if (!bases) {
@@ -4082,6 +4100,7 @@ type_dealloc(PyTypeObject *type)
_PyDictKeys_DecRef(et->ht_cached_keys);
}
Py_XDECREF(et->ht_module);
+ PyMem_Free(et->_ht_tpname);
Py_TYPE(type)->tp_free((PyObject *)type);
}
@@ -4273,10 +4292,12 @@ type_traverse(PyTypeObject *type, visitproc visit, void *arg)
Py_VISIT(type->tp_base);
Py_VISIT(((PyHeapTypeObject *)type)->ht_module);
- /* There's no need to visit type->tp_subclasses or
- ((PyHeapTypeObject *)type)->ht_slots, because they can't be involved
- in cycles; tp_subclasses is a list of weak references,
- and slots is a tuple of strings. */
+ /* There's no need to visit others because they can't be involved
+ in cycles:
+ type->tp_subclasses is a list of weak references,
+ ((PyHeapTypeObject *)type)->ht_slots is a tuple of strings,
+ ((PyHeapTypeObject *)type)->ht_*name are strings.
+ */
return 0;
}
More information about the Python-checkins
mailing list