8000 bpo-45315: PyType_FromSpec: Copy spec->name and have the type own the… · python/cpython@8a310dd · GitHub
[go: up one dir, main page]

Skip to content

Commit 8a310dd

Browse files
authored
bpo-45315: PyType_FromSpec: Copy spec->name and have the type own the memory for its name (GH-29103)
1 parent 2cbf50e commit 8a310dd

File tree

5 files changed

+162
-16
lines changed

5 files changed

+162
-16
lines changed

Include/cpython/object.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ typedef struct _heaptypeobject {
290290
PyObject *ht_name, *ht_slots, *ht_qualname;
291291
struct _dictkeysobject *ht_cached_keys;
292292
PyObject *ht_module;
293+
char *_ht_tpname; // Storage for "tp_name"; see PyType_FromModuleAndSpec
293294
/* here are optional user slots, followed by the members. */
294295
} PyHeapTypeObject;
295296

Lib/test/test_sys.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1419,7 +1419,7 @@ def delx(self): del self.__x
14191419
'3P' # PyMappingMethods
14201420
'10P' # PySequenceMethods
14211421
'2P' # PyBufferProcs
1422-
'5P')
1422+
'6P')
14231423
class newstyleclass(object): pass
14241424
# Separate block for PyDictKeysObject with 8 keys and 5 entries
14251425
check(newstyleclass, s + calcsize(DICT_KEY_STRUCT_FORMAT) + 32 + 21*calcsize("n2P"))
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:c:func:`PyType_FromSpec* <PyType_FromModuleAndSpec>` now copies the class name
2+
from the spec to a buffer owned by the class, so the original can be safely
3+
deallocated. Patch by Petr Viktorin.

Modules/_testcapimodule.c

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,6 +1163,126 @@ test_get_type_name(PyObject *self, PyObject *Py_UNUSED(ignored))
11631163
}
11641164

11651165

1166+
static PyObject *
1167+
simple_str(PyObject *self) {
1168+
return PyUnicode_FromString("<test>");
1169+
}
1170+
1171+
1172+
static PyObject *
1173+
test_type_from_ephemeral_spec(PyObject *self, PyObject *Py_UNUSED(ignored))
1174+
{
1175+
// Test that a heap type can be created from a spec that's later deleted
1176+
// (along with all its contents).
1177+
// All necessary data must be copied and held by the class
1178+
PyType_Spec *spec = NULL;
1179+
char *name = NULL;
1180+
char *doc = NULL;
1181+
PyType_Slot *slots = NULL;
1182+
PyObject *class = NULL;
1183+
PyObject *instance = NULL;
1184+
PyObject *obj = NULL;
1185+
PyObject *result = NULL;
1186+
1187+
/* create a spec (and all its contents) on the heap */
1188+
1189+
const char NAME[] = "testcapi._Test";
1190+
const char DOC[] = "a test class";
1191+
1192+
spec = PyMem_New(PyType_Spec, 1);
1193+
if (spec == NULL) {
1194+
PyErr_NoMemory();
1195+
goto finally;
1196+
}
1197+
name = PyMem_New(char, sizeof(NAME));
1198+
if (name == NULL) {
1199+
PyErr_NoMemory();
1200+
goto finally;
1201+
}
1202+
memcpy(name, NAME, sizeof(NAME));
1203+
1204+
doc = PyMem_New(char, sizeof(DOC));
1205+
if (name == NULL) {
1206+
PyErr_NoMemory();
1207+
goto finally;
1208+
}
1209+
memcpy(doc, DOC, sizeof(DOC));
1210+
1211+
spec->name = name;
1212+
spec->basicsize = sizeof(PyObject);
1213+
spec->itemsize = 0;
1214+
spec->flags = Py_TPFLAGS_DEFAULT;
1215+
slots = PyMem_New(PyType_Slot, 3);
1216+
if (slots == NULL) {
1217+
PyErr_NoMemory();
1218+
goto finally;
1219+
}
1220+
slots[0].slot = Py_tp_str;
1221+
slots[0].pfunc = simple_str;
1222+
slots[1].slot = Py_tp_doc;
1223+
slots[1].pfunc = doc;
1224+
slots[2].slot = 0;
1225+
slots[2].pfunc = NULL;
1226+
spec->slots = slots;
1227+
1228+
/* create the class */
1229+
1230+
class = PyType_FromSpec(spec);
1231+
if (class == NULL) {
1232+
goto finally;
1233+
}
1234+
1235+
/* deallocate the spec (and all contents) */
1236+
1237+
// (Explicitly ovewrite memory before freeing,
1238+
// so bugs show themselves even without the debug allocator's help.)
1239+
memset(spec, 0xdd, sizeof(PyType_Spec));
1240+
PyMem_Del(spec);
1241+
spec = NULL;
1242+
memset(name, 0xdd, sizeof(NAME));
1243+
PyMem_Del(name);
1244+
name = NULL;
1245+
memset(doc, 0xdd, sizeof(DOC));
1246+
PyMem_Del(doc);
1247+
doc = NULL;
1248+
memset(slots, 0xdd, 3 * sizeof(PyType_Slot));
1249+
PyMem_Del(slots);
1250+
slots = NULL;
1251+
1252+
/* check that everything works */
1253+
1254+
PyTypeObject *class_tp = (PyTypeObject *)class;
1255+
PyHeapTypeObject *class_ht = (PyHeapTypeObject *)class;
1256+
assert(strcmp(class_tp->tp_name, "testcapi._Test") == 0);
1257+
assert(strcmp(PyUnicode_AsUTF8(class_ht->ht_name), "_Test") == 0);
1258+
assert(strcmp(PyUnicode_AsUTF8(class_ht->ht_qualname), "_Test") == 0);
1259+
assert(strcmp(class_tp->tp_doc, "a test class") == 0);
1260+
1261+
// call and check __str__
1262+
instance = PyObject_CallNoArgs(class);
1263+
if (instance == NULL) {
1264+
goto finally;
1265+
}
1266+
obj = PyObject_Str(instance);
1267+
if (obj == NULL) {
1268+
goto finally;
1269+
}
1270+
assert(strcmp(PyUnicode_AsUTF8(obj), "<test>") == 0);
1271+
Py_CLEAR(obj);
1272+
1273+
result = Py_NewRef(Py_None);
1274+
finally:
1275+
PyMem_Del(spec);
1276+
PyMem_Del(name);
1277+
PyMem_Del(doc);
1278+
PyMem_Del(slots);
1279+
Py_XDECREF(class);
1280+
Py_XDECREF(instance);
1281+
Py_XDECREF(obj);
1282+
return result;
1283+
}
1284+
1285+
11661286
static PyObject *
11671287
test_get_type_qualname(PyObject *self, PyObject *Py_UNUSED(ignored))
11681288
{
@@ -5804,6 +5924,7 @@ static PyMethodDef TestMethods[] = {
58045924
{"test_get_statictype_slots", test_get_statictype_slots, METH_NOARGS},
58055925
{"test_get_type_name", test_get_type_name, METH_NOARGS},
58065926
{"test_get_type_qualname", test_get_type_qualname, METH_NOARGS},
5927+
{"test_type_from_ephemeral_spec", test_type_from_ephemeral_spec, METH_NOARGS},
58075928
{"get_kwargs", (PyCFunction)(void(*)(void))get_kwargs,
58085929
METH_VARARGS|METH_KEYWORDS},
58095930
{"getargs_tuple", getargs_tuple, METH_VARARGS},

Objects/typeobject.c

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2778,6 +2778,7 @@ type_new_alloc(type_new_ctx *ctx)
27782778

27792779
et->ht_name = Py_NewRef(ctx->name);
27802780
et->ht_module = NULL;
2781+
et->_ht_tpname = NULL;
27812782

27822783
return type;
27832784
}
@@ -3435,25 +3436,42 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases)
34353436
goto fail;
34363437
}
34373438

3439+
type = &res->ht_type;
3440+
/* The flags must be initialized early, before the GC traverses us */
3441+
type->tp_flags = spec->flags | Py_TPFLAGS_HEAPTYPE;
3442+
34383443
/* Set the type name and qualname */
34393444
const char *s = strrchr(spec->name, '.');
3440-
if (s == NULL)
3445+
if (s == NULL) {
34413446
s = spec->name;
3442-
else
3447+
}
3448+
else {
34433449
s++;
3450+
}
34443451

3445-
type = &res->ht_type;
3446-
/* The flags must be initialized early, before the GC traverses us */
3447-
type->tp_flags = spec->flags | Py_TPFLAGS_HEAPTYPE;
34483452
res->ht_name = PyUnicode_FromString(s);
3449-
if (!res->ht_name)
3453+
if (!res->ht_name) {
3454+
goto fail;
3455+
}
3456+
res->ht_qualname = Py_NewRef(res->ht_name);
3457+
3458+
/* Copy spec->name to a buffer we own.
3459+
*
3460+
* Unfortunately, we can't use tp_name directly (with some
3461+
* flag saying that it should be deallocated with the type),
3462+
* because tp_name is public API and may be set independently
3463+
* of any such flag.
3464+
* So, we use a separate buffer, _ht_tpname, that's always
3465+
* deallocated with the type (if it's non-NULL).
3466+
*/
3467+
Py_ssize_t name_buf_len = strlen(spec->name) + 1;
3468+
res->_ht_tpname = PyMem_Malloc(name_buf_len);
3469+
if (res->_ht_tpname == NULL) {
34503470
goto fail;
3451-
res->ht_qualname = res->ht_name;
3452-
Py_INCREF(res->ht_qualname);
3453-
type->tp_name = spec->name;
3471+
}
3472+
type->tp_name = memcpy(res->_ht_tpname, spec->name, name_buf_len);
34543473

3455-
Py_XINCREF(module);
3456-
res->ht_module = module;
3474+
res->ht_module = Py_XNewRef(module);
34573475

34583476
/* Adjust for empty tuple bases */
34593477
if (!bases) {
@@ -4082,6 +4100,7 @@ type_dealloc(PyTypeObject *type)
40824100
_PyDictKeys_DecRef(et->ht_cached_keys);
40834101
}
40844102
Py_XDECREF(et->ht_module);
4103+
PyMem_Free(et->_ht_tpname);
40854104
Py_TYPE(type)->tp_free((PyObject *)type);
40864105
}
40874106

@@ -4273,10 +4292,12 @@ type_traverse(PyTypeObject *type, visitproc visit, void *arg)
42734292
Py_VISIT(type->tp_base);
42744293
Py_VISIT(((PyHeapTypeObject *)type)->ht_module);
42754294

4276-
/* There's no need to visit type->tp_subclasses or
4277-
((PyHeapTypeObject *)type)->ht_slots, because they can't be involved
4278-
in cycles; tp_subclasses is a list of weak references,
4279-
and slots is a tuple of strings. */
4295+
/* There's no need to visit others because they can't be involved
4296+
in cycles:
4297+
type->tp_subclasses is a list of weak references,
4298+
((PyHeapTypeObject *)type)->ht_slots is a tuple of strings,
4299+
((PyHeapTypeObject *)type)->ht_*name are strings.
4300+
*/
42804301

42814302
return 0;
42824303
}

0 commit comments

Comments
 (0)
0