From 63b68b5531f769a69892b2d68c1fce56593fa5e1 Mon Sep 17 00:00:00 2001 From: Hai Shi Date: Mon, 11 May 2020 00:19:55 +0800 Subject: [PATCH 1/6] test --- Modules/_functoolsmodule.c | 265 +++++++++++++++++++------------------ 1 file changed, 135 insertions(+), 130 deletions(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index d158d3bae157b2..c73d7a9dcbc471 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -22,7 +22,20 @@ typedef struct { vectorcallfunc vectorcall; } partialobject; -static PyTypeObject partial_type; +typedef struct _functools_state { + /* this object is used delimit args and keywords in the cache keys */ + PyObject *kwd_mark; + PyTypeObject *partial_type; + PyTypeObject *lru_cache_type; +} _functools_state; + +static inline _functools_state* +get_functools_state(PyObject *module) +{ + void *state = PyModule_GetState(module); + assert(state != NULL); + return (_functools_state *)state; +} static void partial_setvectorcall(partialobject *pto); @@ -40,7 +53,10 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) pargs = pkw = NULL; func = PyTuple_GET_ITEM(args, 0); - if (Py_IS_TYPE(func, &partial_type) && type == &partial_type) { + PyObject *module = PyType_GetModule(type); + _functools_state *state = get_functools_state(module); + if (Py_IS_TYPE(func, state->partial_type) + && type == state->partial_type) { partialobject *part = (partialobject *)func; if (part->dict == NULL) { pargs = part->args; @@ -292,6 +308,12 @@ static PyMemberDef partial_memberlist[] = { "tuple of arguments to future partial calls"}, {"keywords", T_OBJECT, OFF(kw), READONLY, "dictionary of keyword arguments to future partial calls"}, + {"__weaklistoffset__", T_PYSSIZET, + offsetof(partialobject, weakreflist), READONLY}, + {"__dictoffset__", T_PYSSIZET, + offsetof(partialobject, dict), READONLY}, + {"__vectorcalloffset__", T_PYSSIZET, + offsetof(partialobject, vectorcall), READONLY}, {NULL} /* Sentinel */ }; @@ -418,49 +440,28 @@ static PyMethodDef partial_methods[] = { {NULL, NULL} /* sentinel */ }; -static PyTypeObject partial_type = { - PyVarObject_HEAD_INIT(NULL, 0) - "functools.partial", /* tp_name */ - sizeof(partialobject), /* tp_basicsize */ - 0, /* tp_itemsize */ - /* methods */ - (destructor)partial_dealloc, /* tp_dealloc */ - offsetof(partialobject, vectorcall),/* tp_vectorcall_offset */ - 0, /* tp_getattr */ - 0, /* tp_setattr */ - 0, /* tp_as_async */ - (reprfunc)partial_repr, /* tp_repr */ - 0, /* tp_as_number */ - 0, /* tp_as_sequence */ - 0, /* tp_as_mapping */ - 0, /* tp_hash */ - (ternaryfunc)partial_call, /* tp_call */ - 0, /* tp_str */ - PyObject_GenericGetAttr, /* tp_getattro */ - PyObject_GenericSetAttr, /* tp_setattro */ - 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | - Py_TPFLAGS_BASETYPE | - Py_TPFLAGS_HAVE_VECTORCALL, /* tp_flags */ - partial_doc, /* tp_doc */ - (traverseproc)partial_traverse, /* tp_traverse */ - 0, /* tp_clear */ - 0, /* tp_richcompare */ - offsetof(partialobject, weakreflist), /* tp_weaklistoffset */ - 0, /* tp_iter */ - 0, /* tp_iternext */ - partial_methods, /* tp_methods */ - partial_memberlist, /* tp_members */ - partial_getsetlist, /* tp_getset */ - 0, /* tp_base */ - 0, /* tp_dict */ - 0, /* tp_descr_get */ - 0, /* tp_descr_set */ - offsetof(partialobject, dict), /* tp_dictoffset */ - 0, /* tp_init */ - 0, /* tp_alloc */ - partial_new, /* tp_new */ - PyObject_GC_Del, /* tp_free */ +static PyType_Slot partial_type_slots[] = { + {Py_tp_dealloc, partial_dealloc}, + {Py_tp_repr, partial_repr}, + {Py_tp_call, partial_call}, + {Py_tp_getattro, PyObject_GenericGetAttr}, + {Py_tp_setattro, PyObject_GenericSetAttr}, + {Py_tp_doc, (void *)partial_doc}, + {Py_tp_traverse, partial_traverse}, + {Py_tp_methods, partial_methods}, + {Py_tp_members, partial_memberlist}, + {Py_tp_getset, partial_getsetlist}, + {Py_tp_new, partial_new}, + {Py_tp_free, PyObject_GC_Del}, + {0, 0} +}; + +static PyType_Spec partial_type_spec = { + .name = "functools.partial", + .basicsize = sizeof(partialobject), + .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | + Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_VECTORCALL, + .slots = partial_type_slots }; @@ -723,10 +724,6 @@ sequence is empty."); */ - -/* this object is used delimit args and keywords in the cache keys */ -static PyObject *kwd_mark = NULL; - struct lru_list_elem; struct lru_cache_object; @@ -786,10 +783,8 @@ typedef struct lru_cache_object { PyObject *weakreflist; } lru_cache_object; -static PyTypeObject lru_cache_type; - static PyObject * -lru_cache_make_key(PyObject *args, PyObject *kwds, int typed) +lru_cache_make_key(PyObject *module, PyObject *args, PyObject *kwds, int typed) { PyObject *key, *keyword, *value; Py_ssize_t key_size, pos, key_pos, kwds_size; @@ -827,9 +822,10 @@ lru_cache_make_key(PyObject *args, PyObject *kwds, int typed) Py_INCREF(item); PyTuple_SET_ITEM(key, key_pos++, item); } + _functools_state *state = get_functools_state(module); if (kwds_size) { - Py_INCREF(kwd_mark); - PyTuple_SET_ITEM(key, key_pos++, kwd_mark); + Py_INCREF(state->kwd_mark); + PyTuple_SET_ITEM(key, key_pos++, state->kwd_mark); for (pos = 0; PyDict_Next(kwds, &pos, &keyword, &value);) { Py_INCREF(keyword); PyTuple_SET_ITEM(key, key_pos++, keyword); @@ -868,12 +864,19 @@ uncached_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwd return result; } +static inline PyObject* +lru_cache_object_get_module(lru_cache_object *self) +{ + return PyType_GetModule(Py_TYPE(self)); +} + static PyObject * infinite_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds) { - PyObject *result; + PyObject *result, *module; Py_hash_t hash; - PyObject *key = lru_cache_make_key(args, kwds, self->typed); + module = lru_cache_object_get_module(self); + PyObject *key = lru_cache_make_key(module, args, kwds, self->typed); if (!key) return NULL; hash = PyObject_Hash(key); @@ -971,10 +974,11 @@ static PyObject * bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds) { lru_list_elem *link; - PyObject *key, *result, *testresult; + PyObject *key, *result, *testresult, *module; Py_hash_t hash; - key = lru_cache_make_key(args, kwds, self->typed); + module = lru_cache_object_get_module(self); + key = lru_cache_make_key(module, args, kwds, self->typed); if (!key) return NULL; hash = PyObject_Hash(key); @@ -1360,51 +1364,27 @@ static PyGetSetDef lru_cache_getsetlist[] = { {NULL} }; -static PyTypeObject lru_cache_type = { - PyVarObject_HEAD_INIT(NULL, 0) - "functools._lru_cache_wrapper", /* tp_name */ - sizeof(lru_cache_object), /* tp_basicsize */ - 0, /* tp_itemsize */ - /* methods */ - (destructor)lru_cache_dealloc, /* tp_dealloc */ - 0, /* tp_vectorcall_offset */ - 0, /* tp_getattr */ - 0, /* tp_setattr */ - 0, /* tp_as_async */ - 0, /* tp_repr */ - 0, /* tp_as_number */ - 0, /* tp_as_sequence */ - 0, /* tp_as_mapping */ - 0, /* tp_hash */ - (ternaryfunc)lru_cache_call, /* tp_call */ - 0, /* tp_str */ - 0, /* tp_getattro */ - 0, /* tp_setattro */ - 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | - Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_METHOD_DESCRIPTOR, - /* tp_flags */ - lru_cache_doc, /* tp_doc */ - (traverseproc)lru_cache_tp_traverse,/* tp_traverse */ - (inquiry)lru_cache_tp_clear, /* tp_clear */ - 0, /* tp_richcompare */ - offsetof(lru_cache_object, weakreflist), - /* tp_weaklistoffset */ - 0, /* tp_iter */ - 0, /* tp_iternext */ - lru_cache_methods, /* tp_methods */ - 0, /* tp_members */ - lru_cache_getsetlist, /* tp_getset */ - 0, /* tp_base */ - 0, /* tp_dict */ - lru_cache_descr_get, /* tp_descr_get */ - 0, /* tp_descr_set */ - offsetof(lru_cache_object, dict), /* tp_dictoffset */ - 0, /* tp_init */ - 0, /* tp_alloc */ - lru_cache_new, /* tp_new */ +static PyType_Slot lru_cache_type_slots[] = { + {Py_tp_dealloc, lru_cache_dealloc}, + {Py_tp_call, lru_cache_call}, + {Py_tp_doc, (void *)lru_cache_doc}, + {Py_tp_traverse, lru_cache_tp_traverse}, + {Py_tp_clear, lru_cache_tp_clear}, + {Py_tp_methods, lru_cache_methods}, + {Py_tp_getset, lru_cache_getsetlist}, + {Py_tp_descr_get, lru_cache_descr_get}, + {Py_tp_new, lru_cache_new} +}; + +static PyType_Spec lru_cache_type_spec = { + .name = "functools._lru_cache_wrapper", + .basicsize = sizeof(lru_cache_object), + .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | + Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_METHOD_DESCRIPTOR, + .slots = lru_cache_type_slots }; + /* module level code ********************************************************/ PyDoc_STRVAR(_functools_doc, @@ -1417,38 +1397,64 @@ static PyMethodDef _functools_methods[] = { {NULL, NULL} /* sentinel */ }; -static void -_functools_free(void *m) -{ - // FIXME: Do not clear kwd_mark to avoid NULL pointer dereferencing if we have - // other modules instances that could use it. Will fix when PEP-573 land - // and we could move kwd_mark to a per-module state. - // Py_CLEAR(kwd_mark); -} - static int _functools_exec(PyObject *module) { - PyTypeObject *typelist[] = { - &partial_type, - &lru_cache_type - }; - - if (!kwd_mark) { - kwd_mark = _PyObject_CallNoArg((PyObject *)&PyBaseObject_Type); - if (!kwd_mark) { + _functools_state *state = get_functools_state(module); + if (state->kwd_mark == NULL) { + state->kwd_mark = _PyObject_CallNoArg((PyObject *)&PyBaseObject_Type); + if (state->kwd_mark == NULL) { return -1; } } - for (size_t i = 0; i < Py_ARRAY_LENGTH(typelist); i++) { - if (PyModule_AddType(module, typelist[i]) < 0) { - return -1; - } + PyTypeObject *partial_type = + (PyTypeObject *)PyType_FromModuleAndSpec(module, &partial_type_spec, NULL); + if (partial_type == NULL) { + return -1; } + if (PyModule_AddType(module, partial_type) < 0) { + return -1; + } + state->partial_type = partial_type; + + PyTypeObject *lru_cache_type = + (PyTypeObject *)PyType_FromModuleAndSpec(module, &lru_cache_type_spec, NULL); + if (lru_cache_type == NULL) { + return -1; + } + if (PyModule_AddType(module, lru_cache_type) < 0) { + return -1; + } + state->lru_cache_type = lru_cache_type; + return 0; } +static int +_functools_traverse(PyObject *module, visitproc visit, void *arg) +{ + Py_VISIT(get_functools_state(module)->kwd_mark); + Py_VISIT(get_functools_state(module)->partial_type); + Py_VISIT(get_functools_state(module)->lru_cache_type); + return 0; +} + +static int +_functools_clear(PyObject *module) +{ + Py_CLEAR(get_functools_state(module)->kwd_mark); + Py_CLEAR(get_functools_state(module)->partial_type); + Py_CLEAR(get_functools_state(module)->lru_cache_type); + return 0; +} + +static void +_functools_free(void *module) +{ + _functools_clear((PyObject *)module); +} + static struct PyModuleDef_Slot _functools_slots[] = { {Py_mod_exec, _functools_exec}, {0, NULL} @@ -1456,14 +1462,13 @@ static struct PyModuleDef_Slot _functools_slots[] = { static struct PyModuleDef _functools_module = { PyModuleDef_HEAD_INIT, - "_functools", - _functools_doc, - 0, - _functools_methods, - _functools_slots, - NULL, - NULL, - _functools_free, + .m_name = "_functools", + .m_doc = _functools_doc, + .m_methods = _functools_methods, + .m_slots = _functools_slots, + .m_traverse = _functools_traverse, + .m_clear = _functools_clear, + .m_free = _functools_free, }; PyMODINIT_FUNC From 2ddffb00fc16261fdbb0a83f07d6912f67fae756 Mon Sep 17 00:00:00 2001 From: Hai Shi Date: Tue, 12 May 2020 23:47:10 +0800 Subject: [PATCH 2/6] update pep573 in functools extension module --- Modules/_functoolsmodule.c | 41 ++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index c73d7a9dcbc471..f03fe55b2c83d8 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -51,10 +51,14 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) return NULL; } - pargs = pkw = NULL; - func = PyTuple_GET_ITEM(args, 0); PyObject *module = PyType_GetModule(type); + if (module == NULL) { + return NULL; + } _functools_state *state = get_functools_state(module); + + pargs = pkw = NULL; + func = PyTuple_GET_ITEM(args, 0); if (Py_IS_TYPE(func, state->partial_type) && type == state->partial_type) { partialobject *part = (partialobject *)func; @@ -1364,6 +1368,14 @@ static PyGetSetDef lru_cache_getsetlist[] = { {NULL} }; +static PyMemberDef lru_cache_memberlist[] = { + {"__dictoffset__", T_PYSSIZET, + offsetof(lru_cache_object, dict), READONLY}, + {"__weaklistoffset__", T_PYSSIZET, + offsetof(lru_cache_object, weakreflist), READONLY}, + {NULL} /* Sentinel */ +}; + static PyType_Slot lru_cache_type_slots[] = { {Py_tp_dealloc, lru_cache_dealloc}, {Py_tp_call, lru_cache_call}, @@ -1371,9 +1383,11 @@ static PyType_Slot lru_cache_type_slots[] = { {Py_tp_traverse, lru_cache_tp_traverse}, {Py_tp_clear, lru_cache_tp_clear}, {Py_tp_methods, lru_cache_methods}, + {Py_tp_members, lru_cache_memberlist}, {Py_tp_getset, lru_cache_getsetlist}, {Py_tp_descr_get, lru_cache_descr_get}, - {Py_tp_new, lru_cache_new} + {Py_tp_new, lru_cache_new}, + {0, 0} }; static PyType_Spec lru_cache_type_spec = { @@ -1401,11 +1415,9 @@ static int _functools_exec(PyObject *module) { _functools_state *state = get_functools_state(module); + state->kwd_mark = _PyObject_CallNoArg((PyObject *)&PyBaseObject_Type); if (state->kwd_mark == NULL) { - state->kwd_mark = _PyObject_CallNoArg((PyObject *)&PyBaseObject_Type); - if (state->kwd_mark == NULL) { - return -1; - } + return -1; } PyTypeObject *partial_type = @@ -1434,18 +1446,20 @@ _functools_exec(PyObject *module) static int _functools_traverse(PyObject *module, visitproc visit, void *arg) { - Py_VISIT(get_functools_state(module)->kwd_mark); - Py_VISIT(get_functools_state(module)->partial_type); - Py_VISIT(get_functools_state(module)->lru_cache_type); + _functools_state *state = get_functools_state(module); + Py_VISIT(state->kwd_mark); + Py_VISIT(state->partial_type); + Py_VISIT(state->lru_cache_type); return 0; } static int _functools_clear(PyObject *module) { - Py_CLEAR(get_functools_state(module)->kwd_mark); - Py_CLEAR(get_functools_state(module)->partial_type); - Py_CLEAR(get_functools_state(module)->lru_cache_type); + _functools_state *state = get_functools_state(module); + Py_CLEAR(state->kwd_mark); + Py_CLEAR(state->partial_type); + Py_CLEAR(state->lru_cache_type); return 0; } @@ -1464,6 +1478,7 @@ static struct PyModuleDef _functools_module = { PyModuleDef_HEAD_INIT, .m_name = "_functools", .m_doc = _functools_doc, + .m_size = sizeof(_functools_state), .m_methods = _functools_methods, .m_slots = _functools_slots, .m_traverse = _functools_traverse, From 7074ba610167e671410662be811caeceb2a52f30 Mon Sep 17 00:00:00 2001 From: Hai Shi Date: Tue, 17 Nov 2020 12:49:02 +0800 Subject: [PATCH 3/6] Convert _functools module to use PyType_FromModuleAndSpec. --- .../2020-11-19-23-12-57.bpo-40137.bihl9O.rst | 1 + Modules/_functoolsmodule.c | 246 +++++++++--------- Modules/_pickle.c | 18 +- 3 files changed, 143 insertions(+), 122 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-11-19-23-12-57.bpo-40137.bihl9O.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-11-19-23-12-57.bpo-40137.bihl9O.rst b/Misc/NEWS.d/next/Core and Builtins/2020-11-19-23-12-57.bpo-40137.bihl9O.rst new file mode 100644 index 00000000000000..607fa0ecccf513 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2020-11-19-23-12-57.bpo-40137.bihl9O.rst @@ -0,0 +1 @@ +Convert functools module to use :c:func:`PyType_FromModuleAndSpec`. diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 552819db04c195..96502d7de79dc5 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -28,9 +28,11 @@ typedef struct _functools_state { PyObject *kwd_mark; PyTypeObject *partial_type; PyTypeObject *lru_cache_type; + PyTypeObject *keyobject_type; + PyTypeObject *lru_list_elem_type; } _functools_state; -static inline _functools_state* +static inline _functools_state * get_functools_state(PyObject *module) { void *state = PyModule_GetState(module); @@ -39,6 +41,7 @@ get_functools_state(PyObject *module) } static void partial_setvectorcall(partialobject *pto); +static struct PyModuleDef _functools_module; static PyObject * partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) @@ -52,7 +55,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) return NULL; } - PyObject *module = PyType_GetModule(type); + PyObject *module = _PyType_GetModuleByDef(type, &_functools_module); if (module == NULL) { return NULL; } @@ -136,6 +139,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) static void partial_dealloc(partialobject *pto) { + PyTypeObject *tp = Py_TYPE(pto); /* bpo-31095: UnTrack is needed before calling any callbacks */ PyObject_GC_UnTrack(pto); if (pto->weakreflist != NULL) @@ -144,7 +148,8 @@ partial_dealloc(partialobject *pto) Py_XDECREF(pto->args); Py_XDECREF(pto->kw); Py_XDECREF(pto->dict); - Py_TYPE(pto)->tp_free(pto); + tp->tp_free(pto); + Py_DECREF(tp); } @@ -466,7 +471,7 @@ static PyType_Spec partial_type_spec = { .basicsize = sizeof(partialobject), .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_VECTORCALL, - .slots = partial_type_slots + .slots = partial_type_slots }; @@ -478,12 +483,21 @@ typedef struct { PyObject *object; } keyobject; +static int +keyobject_clear(keyobject *ko) +{ + Py_CLEAR(ko->cmp); + Py_CLEAR(ko->object); + return 0; +} + static void keyobject_dealloc(keyobject *ko) { - Py_DECREF(ko->cmp); - Py_XDECREF(ko->object); + PyTypeObject *tp = Py_TYPE(ko); + keyobject_clear(ko); PyObject_FREE(ko); + Py_DECREF(tp); } static int @@ -494,15 +508,6 @@ keyobject_traverse(keyobject *ko, visitproc visit, void *arg) return 0; } -static int -keyobject_clear(keyobject *ko) -{ - Py_CLEAR(ko->cmp); - if (ko->object) - Py_CLEAR(ko->object); - return 0; -} - static PyMemberDef keyobject_members[] = { {"obj", T_OBJECT, offsetof(keyobject, object), 0, @@ -516,50 +521,41 @@ keyobject_call(keyobject *ko, PyObject *args, PyObject *kwds); static PyObject * keyobject_richcompare(PyObject *ko, PyObject *other, int op); -static PyTypeObject keyobject_type = { - PyVarObject_HEAD_INIT(&PyType_Type, 0) - "functools.KeyWrapper", /* tp_name */ - sizeof(keyobject), /* tp_basicsize */ - 0, /* tp_itemsize */ - /* methods */ - (destructor)keyobject_dealloc, /* tp_dealloc */ - 0, /* tp_vectorcall_offset */ - 0, /* tp_getattr */ - 0, /* tp_setattr */ - 0, /* tp_as_async */ - 0, /* tp_repr */ - 0, /* tp_as_number */ - 0, /* tp_as_sequence */ - 0, /* tp_as_mapping */ - 0, /* tp_hash */ - (ternaryfunc)keyobject_call, /* tp_call */ - 0, /* tp_str */ - PyObject_GenericGetAttr, /* tp_getattro */ - 0, /* tp_setattro */ - 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT, /* tp_flags */ - 0, /* tp_doc */ - (traverseproc)keyobject_traverse, /* tp_traverse */ - (inquiry)keyobject_clear, /* tp_clear */ - keyobject_richcompare, /* tp_richcompare */ - 0, /* tp_weaklistoffset */ - 0, /* tp_iter */ - 0, /* tp_iternext */ - 0, /* tp_methods */ - keyobject_members, /* tp_members */ - 0, /* tp_getset */ +static PyType_Slot keyobject_type_slots[] = { + {Py_tp_dealloc, keyobject_dealloc}, + {Py_tp_call, keyobject_call}, + {Py_tp_getattro, PyObject_GenericGetAttr}, + {Py_tp_traverse, keyobject_traverse}, + {Py_tp_clear, keyobject_clear}, + {Py_tp_richcompare, keyobject_richcompare}, + {Py_tp_members, keyobject_members}, + {0, 0} +}; + +static PyType_Spec keyobject_type_spec = { + .name = "functools.KeyWrapper", + .basicsize = sizeof(keyobject), + .flags = Py_TPFLAGS_DEFAULT, + .slots = keyobject_type_slots }; static PyObject * keyobject_call(keyobject *ko, PyObject *args, PyObject *kwds) { - PyObject *object; + PyObject *object, *module; keyobject *result; + _functools_state *state; static char *kwargs[] = {"obj", NULL}; if (!PyArg_ParseTupleAndKeywords(args, kwds, "O:K", kwargs, &object)) return NULL; - result = PyObject_New(keyobject, &keyobject_type); + + module = _PyType_GetModuleByDef(Py_TYPE(ko), &_functools_module); + if (module == NULL) { + return NULL; + } + state = get_functools_state(module); + result = PyObject_New(keyobject, state->keyobject_type); if (!result) return NULL; Py_INCREF(ko->cmp); @@ -577,9 +573,16 @@ keyobject_richcompare(PyObject *ko, PyObject *other, int op) PyObject *y; PyObject *compare; PyObject *answer; + PyObject *module; PyObject* stack[2]; + _functools_state *state; - if (!Py_IS_TYPE(other, &keyobject_type)) { + module = _PyType_GetModuleByDef(Py_TYPE(ko), &_functools_module); + if (module == NULL) { + return NULL; + } + state = get_functools_state(module); + if (!Py_IS_TYPE(other, state->keyobject_type)) { PyErr_Format(PyExc_TypeError, "other argument must be K instance"); return NULL; } @@ -613,10 +616,13 @@ functools_cmp_to_key(PyObject *self, PyObject *args, PyObject *kwds) PyObject *cmp; static char *kwargs[] = {"mycmp", NULL}; keyobject *object; + _functools_state *state; if (!PyArg_ParseTupleAndKeywords(args, kwds, "O:cmp_to_key", kwargs, &cmp)) return NULL; - object = PyObject_New(keyobject, &keyobject_type); + + state = get_functools_state(self); + object = PyObject_New(keyobject, state->keyobject_type); if (!object) return NULL; Py_INCREF(cmp); @@ -741,33 +747,23 @@ typedef struct lru_list_elem { static void lru_list_elem_dealloc(lru_list_elem *link) { + PyTypeObject *tp = Py_TYPE(link); Py_XDECREF(link->key); Py_XDECREF(link->result); PyObject_Del(link); + Py_DECREF(tp); } -static PyTypeObject lru_list_elem_type = { - PyVarObject_HEAD_INIT(&PyType_Type, 0) - "functools._lru_list_elem", /* tp_name */ - sizeof(lru_list_elem), /* tp_basicsize */ - 0, /* tp_itemsize */ - /* methods */ - (destructor)lru_list_elem_dealloc, /* tp_dealloc */ - 0, /* tp_vectorcall_offset */ - 0, /* tp_getattr */ - 0, /* tp_setattr */ - 0, /* tp_as_async */ - 0, /* tp_repr */ - 0, /* tp_as_number */ - 0, /* tp_as_sequence */ - 0, /* tp_as_mapping */ - 0, /* tp_hash */ - 0, /* tp_call */ - 0, /* tp_str */ - 0, /* tp_getattro */ - 0, /* tp_setattro */ - 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT, /* tp_flags */ +static PyType_Slot lru_list_elem_type_slots[] = { + {Py_tp_dealloc, lru_list_elem_dealloc}, + {0, 0} +}; + +static PyType_Spec lru_list_elem_type_spec = { + .name = "functools._lru_list_elem", + .basicsize = sizeof(lru_list_elem), + .flags = Py_TPFLAGS_DEFAULT, + .slots = lru_list_elem_type_slots }; @@ -868,18 +864,12 @@ uncached_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwd return result; } -static inline PyObject* -lru_cache_object_get_module(lru_cache_object *self) -{ - return PyType_GetModule(Py_TYPE(self)); -} - static PyObject * infinite_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds) { PyObject *result, *module; Py_hash_t hash; - module = lru_cache_object_get_module(self); + module = _PyType_GetModuleByDef(Py_TYPE(self), &_functools_module); PyObject *key = lru_cache_make_key(module, args, kwds, self->typed); if (!key) return NULL; @@ -980,8 +970,12 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds lru_list_elem *link; PyObject *key, *result, *testresult, *module; Py_hash_t hash; + _functools_state *state; - module = lru_cache_object_get_module(self); + module = _PyType_GetModuleByDef(Py_TYPE(self), &_functools_module); + if (module == NULL) { + return NULL; + } key = lru_cache_make_key(module, args, kwds, self->typed); if (!key) return NULL; @@ -1036,8 +1030,9 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds self->root.next == &self->root) { /* Cache is not full, so put the result in a new link */ + state = get_functools_state(module); link = (lru_list_elem *)PyObject_New(lru_list_elem, - &lru_list_elem_type); + state->lru_list_elem_type); if (link == NULL) { Py_DECREF(key); Py_DECREF(result); @@ -1231,22 +1226,31 @@ lru_cache_clear_list(lru_list_elem *link) } } +static int +lru_cache_tp_clear(lru_cache_object *self) +{ + lru_list_elem *list = lru_cache_unlink_list(self); + Py_CLEAR(self->func); + Py_CLEAR(self->cache); + Py_CLEAR(self->cache_info_type); + Py_CLEAR(self->dict); + lru_cache_clear_list(list); + return 0; +} + static void lru_cache_dealloc(lru_cache_object *obj) { - lru_list_elem *list; + PyTypeObject *tp = Py_TYPE(obj); /* bpo-31095: UnTrack is needed before calling any callbacks */ PyObject_GC_UnTrack(obj); - if (obj->weakreflist != NULL) + if (obj->weakreflist != NULL) { PyObject_ClearWeakRefs((PyObject*)obj); + } - list = lru_cache_unlink_list(obj); - Py_XDECREF(obj->cache); - Py_XDECREF(obj->func); - Py_XDECREF(obj->cache_info_type); - Py_XDECREF(obj->dict); - lru_cache_clear_list(list); - Py_TYPE(obj)->tp_free(obj); + lru_cache_tp_clear(obj); + tp->tp_free(obj); + Py_DECREF(tp); } static PyObject * @@ -1325,18 +1329,6 @@ lru_cache_tp_traverse(lru_cache_object *self, visitproc visit, void *arg) return 0; } -static int -lru_cache_tp_clear(lru_cache_object *self) -{ - lru_list_elem *list = lru_cache_unlink_list(self); - Py_CLEAR(self->func); - Py_CLEAR(self->cache); - Py_CLEAR(self->cache_info_type); - Py_CLEAR(self->dict); - lru_cache_clear_list(list); - return 0; -} - PyDoc_STRVAR(lru_cache_doc, "Create a cached callable that wraps another function.\n\ @@ -1393,9 +1385,9 @@ static PyType_Slot lru_cache_type_slots[] = { static PyType_Spec lru_cache_type_spec = { .name = "functools._lru_cache_wrapper", .basicsize = sizeof(lru_cache_object), - .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | - Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_METHOD_DESCRIPTOR, - .slots = lru_cache_type_slots + .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | + Py_TPFLAGS_METHOD_DESCRIPTOR, + .slots = lru_cache_type_slots }; @@ -1420,25 +1412,41 @@ _functools_exec(PyObject *module) return -1; } - PyTypeObject *partial_type = - (PyTypeObject *)PyType_FromModuleAndSpec(module, &partial_type_spec, NULL); - if (partial_type == NULL) { + state->partial_type = (PyTypeObject *)PyType_FromModuleAndSpec(module, + &partial_type_spec, NULL); + if (state->partial_type == NULL) { + return -1; + } + if (PyModule_AddType(module, state->partial_type) < 0) { + return -1; + } + + state->lru_cache_type = (PyTypeObject *)PyType_FromModuleAndSpec(module, + &lru_cache_type_spec, NULL); + if (state->lru_cache_type == NULL) { + return -1; + } + if (PyModule_AddType(module, state->lru_cache_type) < 0) { + return -1; + } + + state->keyobject_type = (PyTypeObject *)PyType_FromModuleAndSpec(module, + &keyobject_type_spec, NULL); + if (state->keyobject_type == NULL) { return -1; } - if (PyModule_AddType(module, partial_type) < 0) { + if (PyModule_AddType(module, state->keyobject_type) < 0) { return -1; } - state->partial_type = partial_type; - PyTypeObject *lru_cache_type = - (PyTypeObject *)PyType_FromModuleAndSpec(module, &lru_cache_type_spec, NULL); - if (lru_cache_type == NULL) { + state->lru_list_elem_type = (PyTypeObject *)PyType_FromModuleAndSpec( + module, &lru_list_elem_type_spec, NULL); + if (state->lru_list_elem_type == NULL) { return -1; } - if (PyModule_AddType(module, lru_cache_type) < 0) { + if (PyModule_AddType(module, state->lru_list_elem_type) < 0) { return -1; } - state->lru_cache_type = lru_cache_type; return 0; } @@ -1450,6 +1458,8 @@ _functools_traverse(PyObject *module, visitproc visit, void *arg) Py_VISIT(state->kwd_mark); Py_VISIT(state->partial_type); Py_VISIT(state->lru_cache_type); + Py_VISIT(state->keyobject_type); + Py_VISIT(state->lru_list_elem_type); return 0; } @@ -1460,6 +1470,8 @@ _functools_clear(PyObject *module) Py_CLEAR(state->kwd_mark); Py_CLEAR(state->partial_type); Py_CLEAR(state->lru_cache_type); + Py_CLEAR(state->keyobject_type); + Py_CLEAR(state->lru_list_elem_type); return 0; } @@ -1481,7 +1493,7 @@ static struct PyModuleDef _functools_module = { .m_size = sizeof(_functools_state), .m_methods = _functools_methods, .m_slots = _functools_slots, - .m_traverse = _functools_traverse, + .m_traverse = _functools_traverse, .m_clear = _functools_clear, .m_free = _functools_free, }; diff --git a/Modules/_pickle.c b/Modules/_pickle.c index ed8afefe4c74c8..3e0dc051cb2f46 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3637,11 +3637,19 @@ save_global(PicklerObject *self, PyObject *obj, PyObject *name) goto error; } if (cls != obj) { - Py_DECREF(cls); - PyErr_Format(st->PicklingError, - "Can't pickle %R: it's not the same object as %S.%S", - obj, module_name, global_name); - goto error; + /* bpo-40137: Compare types' name in case some type objects are + created in heap. */ + if (PyType_Check(cls) && PyType_Check(obj)) { + const char *cls_name = _PyType_Name((PyTypeObject *)cls); + const char *obj_name = _PyType_Name((PyTypeObject *)obj); + if (strcmp(cls_name, obj_name)) { + Py_DECREF(cls); + PyErr_Format(st->PicklingError, + "Can't pickle %R: it's not the same object as %S.%S", + obj, module_name, global_name); + goto error; + } + } } Py_DECREF(cls); From e9a51a5994fc081daed50d7b99d04f76282800dd Mon Sep 17 00:00:00 2001 From: Hai Shi Date: Tue, 24 Nov 2020 23:28:01 +0800 Subject: [PATCH 4/6] apply Christian's comment --- Lib/test/test_functools.py | 3 +-- Modules/_functoolsmodule.c | 52 +++++++++++++++++++++++--------------- Modules/_pickle.c | 18 ++++--------- 3 files changed, 37 insertions(+), 36 deletions(-) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index bee9f9112bf183..caeeb2712a1a48 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -27,8 +27,7 @@ py_functools = import_helper.import_fresh_module('functools', blocked=['_functools']) -c_functools = import_helper.import_fresh_module('functools', - fresh=['_functools']) +c_functools = import_helper.import_fresh_module('functools') decimal = import_helper.import_fresh_module('decimal', fresh=['_decimal']) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 96502d7de79dc5..c6f17a44e02100 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -43,6 +43,17 @@ get_functools_state(PyObject *module) static void partial_setvectorcall(partialobject *pto); static struct PyModuleDef _functools_module; +static inline _functools_state * +get_functools_state_by_type(PyTypeObject *type) +{ + PyObject *module = _PyType_GetModuleByDef(type, &_functools_module); + if (module == NULL) { + return NULL; + } + _functools_state *state = get_functools_state(module); + return state; +} + static PyObject * partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) { @@ -55,11 +66,10 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) return NULL; } - PyObject *module = _PyType_GetModuleByDef(type, &_functools_module); - if (module == NULL) { + _functools_state *state = get_functools_state_by_type(type); + if (state == NULL) { return NULL; } - _functools_state *state = get_functools_state(module); pargs = pkw = NULL; func = PyTuple_GET_ITEM(args, 0); @@ -542,7 +552,7 @@ static PyType_Spec keyobject_type_spec = { static PyObject * keyobject_call(keyobject *ko, PyObject *args, PyObject *kwds) { - PyObject *object, *module; + PyObject *object; keyobject *result; _functools_state *state; static char *kwargs[] = {"obj", NULL}; @@ -550,11 +560,10 @@ keyobject_call(keyobject *ko, PyObject *args, PyObject *kwds) if (!PyArg_ParseTupleAndKeywords(args, kwds, "O:K", kwargs, &object)) return NULL; - module = _PyType_GetModuleByDef(Py_TYPE(ko), &_functools_module); - if (module == NULL) { + state = get_functools_state_by_type(Py_TYPE(ko)); + if (state == NULL) { return NULL; } - state = get_functools_state(module); result = PyObject_New(keyobject, state->keyobject_type); if (!result) return NULL; @@ -573,15 +582,13 @@ keyobject_richcompare(PyObject *ko, PyObject *other, int op) PyObject *y; PyObject *compare; PyObject *answer; - PyObject *module; PyObject* stack[2]; _functools_state *state; - module = _PyType_GetModuleByDef(Py_TYPE(ko), &_functools_module); - if (module == NULL) { + state = get_functools_state_by_type(Py_TYPE(ko)); + if (state == NULL) { return NULL; } - state = get_functools_state(module); if (!Py_IS_TYPE(other, state->keyobject_type)) { PyErr_Format(PyExc_TypeError, "other argument must be K instance"); return NULL; @@ -784,7 +791,8 @@ typedef struct lru_cache_object { } lru_cache_object; static PyObject * -lru_cache_make_key(PyObject *module, PyObject *args, PyObject *kwds, int typed) +lru_cache_make_key(_functools_state *state, PyObject *args, + PyObject *kwds, int typed) { PyObject *key, *keyword, *value; Py_ssize_t key_size, pos, key_pos, kwds_size; @@ -822,7 +830,6 @@ lru_cache_make_key(PyObject *module, PyObject *args, PyObject *kwds, int typed) Py_INCREF(item); PyTuple_SET_ITEM(key, key_pos++, item); } - _functools_state *state = get_functools_state(module); if (kwds_size) { Py_INCREF(state->kwd_mark); PyTuple_SET_ITEM(key, key_pos++, state->kwd_mark); @@ -867,10 +874,14 @@ uncached_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwd static PyObject * infinite_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds) { - PyObject *result, *module; + PyObject *result; Py_hash_t hash; - module = _PyType_GetModuleByDef(Py_TYPE(self), &_functools_module); - PyObject *key = lru_cache_make_key(module, args, kwds, self->typed); + _functools_state *state; + state = get_functools_state_by_type(Py_TYPE(self)); + if (state == NULL) { + return NULL; + } + PyObject *key = lru_cache_make_key(state, args, kwds, self->typed); if (!key) return NULL; hash = PyObject_Hash(key); @@ -968,15 +979,15 @@ static PyObject * bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds) { lru_list_elem *link; - PyObject *key, *result, *testresult, *module; + PyObject *key, *result, *testresult; Py_hash_t hash; _functools_state *state; - module = _PyType_GetModuleByDef(Py_TYPE(self), &_functools_module); - if (module == NULL) { + state = get_functools_state_by_type(Py_TYPE(self)); + if (state == NULL) { return NULL; } - key = lru_cache_make_key(module, args, kwds, self->typed); + key = lru_cache_make_key(state, args, kwds, self->typed); if (!key) return NULL; hash = PyObject_Hash(key); @@ -1030,7 +1041,6 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds self->root.next == &self->root) { /* Cache is not full, so put the result in a new link */ - state = get_functools_state(module); link = (lru_list_elem *)PyObject_New(lru_list_elem, state->lru_list_elem_type); if (link == NULL) { diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 3e0dc051cb2f46..ed8afefe4c74c8 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -3637,19 +3637,11 @@ save_global(PicklerObject *self, PyObject *obj, PyObject *name) goto error; } if (cls != obj) { - /* bpo-40137: Compare types' name in case some type objects are - created in heap. */ - if (PyType_Check(cls) && PyType_Check(obj)) { - const char *cls_name = _PyType_Name((PyTypeObject *)cls); - const char *obj_name = _PyType_Name((PyTypeObject *)obj); - if (strcmp(cls_name, obj_name)) { - Py_DECREF(cls); - PyErr_Format(st->PicklingError, - "Can't pickle %R: it's not the same object as %S.%S", - obj, module_name, global_name); - goto error; - } - } + Py_DECREF(cls); + PyErr_Format(st->PicklingError, + "Can't pickle %R: it's not the same object as %S.%S", + obj, module_name, global_name); + goto error; } Py_DECREF(cls); From 5bb2dd51638191dfa7881b79de973994a65f1ac3 Mon Sep 17 00:00:00 2001 From: Hai Shi Date: Thu, 17 Dec 2020 00:23:21 +0800 Subject: [PATCH 5/6] apply petr's comment --- Modules/_functoolsmodule.c | 37 ++++++++++++------------------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 4497da509f5fd3..a89ee473db557b 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -28,7 +28,6 @@ typedef struct _functools_state { /* this object is used delimit args and keywords in the cache keys */ PyObject *kwd_mark; PyTypeObject *partial_type; - PyTypeObject *lru_cache_type; PyTypeObject *keyobject_type; PyTypeObject *lru_list_elem_type; } _functools_state; @@ -67,15 +66,13 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) return NULL; } - _functools_state *state = get_functools_state_by_type(type); - if (state == NULL) { - return NULL; - } - pargs = pkw = NULL; func = PyTuple_GET_ITEM(args, 0); - if (Py_IS_TYPE(func, state->partial_type) - && type == state->partial_type) { + if (Py_TYPE(func)->tp_new == partial_new) { + // The type of "func" might not be exactly the same type object + // as "type", but if it is called using partial_new, it must have the + // same memory layout (fn, args and kw members). + // We can use its underlying function directly and merge the arguments. partialobject *part = (partialobject *)func; if (part->dict == NULL) { pargs = part->args; @@ -555,19 +552,15 @@ keyobject_call(keyobject *ko, PyObject *args, PyObject *kwds) { PyObject *object; keyobject *result; - _functools_state *state; static char *kwargs[] = {"obj", NULL}; if (!PyArg_ParseTupleAndKeywords(args, kwds, "O:K", kwargs, &object)) return NULL; - state = get_functools_state_by_type(Py_TYPE(ko)); - if (state == NULL) { + result = PyObject_New(keyobject, Py_TYPE(ko)); + if (result == NULL) { return NULL; } - result = PyObject_New(keyobject, state->keyobject_type); - if (!result) - return NULL; Py_INCREF(ko->cmp); result->cmp = ko->cmp; Py_INCREF(object); @@ -584,13 +577,8 @@ keyobject_richcompare(PyObject *ko, PyObject *other, int op) PyObject *compare; PyObject *answer; PyObject* stack[2]; - _functools_state *state; - state = get_functools_state_by_type(Py_TYPE(ko)); - if (state == NULL) { - return NULL; - } - if (!Py_IS_TYPE(other, state->keyobject_type)) { + if (!Py_IS_TYPE(other, Py_TYPE(ko))) { PyErr_Format(PyExc_TypeError, "other argument must be K instance"); return NULL; } @@ -1437,12 +1425,13 @@ _functools_exec(PyObject *module) return -1; } - state->lru_cache_type = (PyTypeObject *)PyType_FromModuleAndSpec(module, + PyObject *lru_cache_type = PyType_FromModuleAndSpec(module, &lru_cache_type_spec, NULL); - if (state->lru_cache_type == NULL) { + if (lru_cache_type == NULL) { return -1; } - if (PyModule_AddType(module, state->lru_cache_type) < 0) { + if (PyModule_AddType(module, (PyTypeObject *)lru_cache_type) < 0) { + Py_DECREF(lru_cache_type); return -1; } @@ -1473,7 +1462,6 @@ _functools_traverse(PyObject *module, visitproc visit, void *arg) _functools_state *state = get_functools_state(module); Py_VISIT(state->kwd_mark); Py_VISIT(state->partial_type); - Py_VISIT(state->lru_cache_type); Py_VISIT(state->keyobject_type); Py_VISIT(state->lru_list_elem_type); return 0; @@ -1485,7 +1473,6 @@ _functools_clear(PyObject *module) _functools_state *state = get_functools_state(module); Py_CLEAR(state->kwd_mark); Py_CLEAR(state->partial_type); - Py_CLEAR(state->lru_cache_type); Py_CLEAR(state->keyobject_type); Py_CLEAR(state->lru_list_elem_type); return 0; From 1941d8553619c786ce5d30fa54aaacc056067199 Mon Sep 17 00:00:00 2001 From: Hai Shi Date: Thu, 17 Dec 2020 20:41:12 +0800 Subject: [PATCH 6/6] apply petr's comment --- Modules/_functoolsmodule.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index a89ee473db557b..b121ec7d14113c 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -42,6 +42,8 @@ get_functools_state(PyObject *module) static void partial_setvectorcall(partialobject *pto); static struct PyModuleDef _functools_module; +static PyObject * +partial_call(partialobject *pto, PyObject *args, PyObject *kwargs); static inline _functools_state * get_functools_state_by_type(PyTypeObject *type) @@ -68,9 +70,9 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) pargs = pkw = NULL; func = PyTuple_GET_ITEM(args, 0); - if (Py_TYPE(func)->tp_new == partial_new) { + if (Py_TYPE(func)->tp_call == (ternaryfunc)partial_call) { // The type of "func" might not be exactly the same type object - // as "type", but if it is called using partial_new, it must have the + // as "type", but if it is called using partial_call, it must have the // same memory layout (fn, args and kw members). // We can use its underlying function directly and merge the arguments. partialobject *part = (partialobject *)func;