From 47eb700204e3248ec14d22f11f9ee1c850f9af51 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Sat, 28 Mar 2020 14:28:50 +0900 Subject: [PATCH 1/8] bpo-40077: Convert _abc module to use PyType_FromSpec() Replace statically allocated types with heap allocated types: use PyType_FromSpec(). Add a module state to store the _abc_data_type.. Add traverse, clear and free functions to the module. --- .../2020-03-28-14-28-46.bpo-40077.4C_xJm.rst | 3 + Modules/_abc.c | 101 +++++++++++++----- 2 files changed, 76 insertions(+), 28 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-03-28-14-28-46.bpo-40077.4C_xJm.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-03-28-14-28-46.bpo-40077.4C_xJm.rst b/Misc/NEWS.d/next/Core and Builtins/2020-03-28-14-28-46.bpo-40077.4C_xJm.rst new file mode 100644 index 00000000000000..379507acca1a96 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2020-03-28-14-28-46.bpo-40077.4C_xJm.rst @@ -0,0 +1,3 @@ +Replace statically allocated types with heap allocated types by using +:c:func:`PyType_FromSpec`. Add a module state to store the _abc_data_typ Add +traverse, clear and free functions to the module. diff --git a/Modules/_abc.c b/Modules/_abc.c index c991295d311a17..fc4e055ff0a2ee 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -20,6 +20,10 @@ _Py_IDENTIFIER(_abc_impl); _Py_IDENTIFIER(__subclasscheck__); _Py_IDENTIFIER(__subclasshook__); +typedef struct { + PyObject *_abc_data_type; +} _abcmodulestate; + /* A global counter that is incremented each time a class is registered as a virtual subclass of anything. It forces the negative cache to be cleared before its next use. @@ -27,6 +31,14 @@ _Py_IDENTIFIER(__subclasshook__); external code. */ static unsigned long long abc_invalidation_counter = 0; +static inline _abcmodulestate* +get_abc_state(PyObject *module) +{ + void *state = PyModule_GetState(module); + assert(state != NULL); + return (_abcmodulestate *)state; +} + /* This object stores internal state for ABCs. Note that we can use normal sets for caches, since they are never iterated over. */ @@ -41,10 +53,12 @@ typedef struct { static void abc_data_dealloc(_abc_data *self) { + PyTypeObject *tp = Py_TYPE(self); Py_XDECREF(self->_abc_registry); Py_XDECREF(self->_abc_cache); Py_XDECREF(self->_abc_negative_cache); - Py_TYPE(self)->tp_free(self); + tp->tp_free(self); + Py_DECREF(tp); } static PyObject * @@ -65,24 +79,30 @@ abc_data_new(PyTypeObject *type, PyObject *args, PyObject *kwds) PyDoc_STRVAR(abc_data_doc, "Internal state held by ABC machinery."); -static PyTypeObject _abc_data_type = { - PyVarObject_HEAD_INIT(NULL, 0) - "_abc_data", /*tp_name*/ - sizeof(_abc_data), /*tp_basicsize*/ - .tp_dealloc = (destructor)abc_data_dealloc, - .tp_flags = Py_TPFLAGS_DEFAULT, - .tp_alloc = PyType_GenericAlloc, - .tp_new = abc_data_new, +static PyType_Slot _abc_data_type_spec_slots[] = { + {Py_tp_doc, (void *)abc_data_doc}, + {Py_tp_dealloc, abc_data_dealloc}, + {Py_tp_alloc, PyType_GenericAlloc}, + {Py_tp_new, abc_data_new}, + {0, 0} +}; + +static PyType_Spec _abc_data_type_spec = { + .name = "_abc._abc_data", + .basicsize = sizeof(_abc_data), + .flags = Py_TPFLAGS_DEFAULT, + .slots = _abc_data_type_spec_slots, }; static _abc_data * -_get_impl(PyObject *self) +_get_impl(PyObject *module, PyObject *self) { + _abcmodulestate *state = get_abc_state(module); PyObject *impl = _PyObject_GetAttrId(self, &PyId__abc_impl); if (impl == NULL) { return NULL; } - if (!Py_IS_TYPE(impl, &_abc_data_type)) { + if (!Py_IS_TYPE(impl, (PyTypeObject *)state->_abc_data_type)) { PyErr_SetString(PyExc_TypeError, "_abc_impl is set to a wrong type"); Py_DECREF(impl); return NULL; @@ -179,7 +199,7 @@ static PyObject * _abc__reset_registry(PyObject *module, PyObject *self) /*[clinic end generated code: output=92d591a43566cc10 input=12a0b7eb339ac35c]*/ { - _abc_data *impl = _get_impl(self); + _abc_data *impl = _get_impl(module, self); if (impl == NULL) { return NULL; } @@ -206,7 +226,7 @@ static PyObject * _abc__reset_caches(PyObject *module, PyObject *self) /*[clinic end generated code: output=f296f0d5c513f80c input=c0ac616fd8acfb6f]*/ { - _abc_data *impl = _get_impl(self); + _abc_data *impl = _get_impl(module, self); if (impl == NULL) { return NULL; } @@ -241,7 +261,7 @@ static PyObject * _abc__get_dump(PyObject *module, PyObject *self) /*[clinic end generated code: output=9d9569a8e2c1c443 input=2c5deb1bfe9e3c79]*/ { - _abc_data *impl = _get_impl(self); + _abc_data *impl = _get_impl(module, self); if (impl == NULL) { return NULL; } @@ -391,13 +411,14 @@ static PyObject * _abc__abc_init(PyObject *module, PyObject *self) /*[clinic end generated code: output=594757375714cda1 input=8d7fe470ff77f029]*/ { + _abcmodulestate *state = get_abc_state(module); PyObject *data; if (compute_abstract_methods(self) < 0) { return NULL; } /* Set up inheritance registry. */ - data = abc_data_new(&_abc_data_type, NULL, NULL); + data = abc_data_new((PyTypeObject *)state->_abc_data_type, NULL, NULL); if (data == NULL) { return NULL; } @@ -446,7 +467,7 @@ _abc__abc_register_impl(PyObject *module, PyObject *self, PyObject *subclass) if (result < 0) { return NULL; } - _abc_data *impl = _get_impl(self); + _abc_data *impl = _get_impl(module, self); if (impl == NULL) { return NULL; } @@ -480,7 +501,7 @@ _abc__abc_instancecheck_impl(PyObject *module, PyObject *self, /*[clinic end generated code: output=b8b5148f63b6b56f input=a4f4525679261084]*/ { PyObject *subtype, *result = NULL, *subclass = NULL; - _abc_data *impl = _get_impl(self); + _abc_data *impl = _get_impl(module, self); if (impl == NULL) { return NULL; } @@ -576,7 +597,7 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, PyObject *ok, *subclasses = NULL, *result = NULL; Py_ssize_t pos; int incache; - _abc_data *impl = _get_impl(self); + _abc_data *impl = _get_impl(module, self); if (impl == NULL) { return NULL; } @@ -808,17 +829,41 @@ static struct PyMethodDef module_functions[] = { }; static int -_abc_exec(PyObject *module) +_abcmodule_exec(PyObject *module) { - if (PyType_Ready(&_abc_data_type) < 0) { + _abcmodulestate *state = get_abc_state(module); + state->_abc_data_type = PyType_FromSpec(&_abc_data_type_spec); + if (state->_abc_data_type == NULL) { return -1; } - _abc_data_type.tp_doc = abc_data_doc; + + return 0; +} + +static int +_abcmodule_traverse(PyObject *module, visitproc visit, void *arg) +{ + _abcmodulestate *state = get_abc_state(module); + Py_VISIT(state->_abc_data_type); return 0; } -static PyModuleDef_Slot _abc_slots[] = { - {Py_mod_exec, _abc_exec}, +static int +_abcmodule_clear(PyObject *module) +{ + _abcmodulestate *state = get_abc_state(module); + Py_CLEAR(state->_abc_data_type); + return 0; +} + +static void +_abcmodule_free(void *module) +{ + _abcmodule_clear((PyObject *)module); +} + +static PyModuleDef_Slot _abcmodule_slots[] = { + {Py_mod_exec, _abcmodule_exec}, {0, NULL} }; @@ -826,12 +871,12 @@ static struct PyModuleDef _abcmodule = { PyModuleDef_HEAD_INIT, "_abc", _abc__doc__, - 0, + sizeof(_abcmodulestate), module_functions, - _abc_slots, - NULL, - NULL, - NULL + _abcmodule_slots, + _abcmodule_traverse, + _abcmodule_clear, + _abcmodule_free, }; PyMODINIT_FUNC From 77560658cef448fe56a557430f1cbbdd07468bec Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Sat, 28 Mar 2020 23:36:57 +0900 Subject: [PATCH 2/8] bpo-40077: rename --- Modules/_abc.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Modules/_abc.c b/Modules/_abc.c index fc4e055ff0a2ee..5f1a83167a1e59 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -22,7 +22,7 @@ _Py_IDENTIFIER(__subclasshook__); typedef struct { PyObject *_abc_data_type; -} _abcmodulestate; +} _abcmodule_state; /* A global counter that is incremented each time a class is registered as a virtual subclass of anything. It forces the @@ -31,12 +31,12 @@ typedef struct { external code. */ static unsigned long long abc_invalidation_counter = 0; -static inline _abcmodulestate* +static inline _abcmodule_state* get_abc_state(PyObject *module) { void *state = PyModule_GetState(module); assert(state != NULL); - return (_abcmodulestate *)state; + return (_abcmodule_state *)state; } /* This object stores internal state for ABCs. @@ -97,7 +97,7 @@ static PyType_Spec _abc_data_type_spec = { static _abc_data * _get_impl(PyObject *module, PyObject *self) { - _abcmodulestate *state = get_abc_state(module); + _abcmodule_state *state = get_abc_state(module); PyObject *impl = _PyObject_GetAttrId(self, &PyId__abc_impl); if (impl == NULL) { return NULL; @@ -411,7 +411,7 @@ static PyObject * _abc__abc_init(PyObject *module, PyObject *self) /*[clinic end generated code: output=594757375714cda1 input=8d7fe470ff77f029]*/ { - _abcmodulestate *state = get_abc_state(module); + _abcmodule_state *state = get_abc_state(module); PyObject *data; if (compute_abstract_methods(self) < 0) { return NULL; @@ -816,7 +816,7 @@ _abc_get_cache_token_impl(PyObject *module) return PyLong_FromUnsignedLongLong(abc_invalidation_counter); } -static struct PyMethodDef module_functions[] = { +static struct PyMethodDef _abcmodule_methods[] = { _ABC_GET_CACHE_TOKEN_METHODDEF _ABC__ABC_INIT_METHODDEF _ABC__RESET_REGISTRY_METHODDEF @@ -831,7 +831,7 @@ static struct PyMethodDef module_functions[] = { static int _abcmodule_exec(PyObject *module) { - _abcmodulestate *state = get_abc_state(module); + _abcmodule_state *state = get_abc_state(module); state->_abc_data_type = PyType_FromSpec(&_abc_data_type_spec); if (state->_abc_data_type == NULL) { return -1; @@ -843,7 +843,7 @@ _abcmodule_exec(PyObject *module) static int _abcmodule_traverse(PyObject *module, visitproc visit, void *arg) { - _abcmodulestate *state = get_abc_state(module); + _abcmodule_state *state = get_abc_state(module); Py_VISIT(state->_abc_data_type); return 0; } @@ -851,7 +851,7 @@ _abcmodule_traverse(PyObject *module, visitproc visit, void *arg) static int _abcmodule_clear(PyObject *module) { - _abcmodulestate *state = get_abc_state(module); + _abcmodule_state *state = get_abc_state(module); Py_CLEAR(state->_abc_data_type); return 0; } @@ -871,8 +871,8 @@ static struct PyModuleDef _abcmodule = { PyModuleDef_HEAD_INIT, "_abc", _abc__doc__, - sizeof(_abcmodulestate), - module_functions, + sizeof(_abcmodule_state), + _abcmodule_methods, _abcmodule_slots, _abcmodule_traverse, _abcmodule_clear, From ec8ccc659d3e75bd45c2cd1ffbe947f7fde2a3a3 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Sun, 29 Mar 2020 09:37:00 +0900 Subject: [PATCH 3/8] bpo-40077: Update test_abc.py --- Lib/test/test_abc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_abc.py b/Lib/test/test_abc.py index 000e5838e3c712..7e9c47b3cacb96 100644 --- a/Lib/test/test_abc.py +++ b/Lib/test/test_abc.py @@ -326,7 +326,7 @@ class B: token_old = abc_get_cache_token() A.register(B) token_new = abc_get_cache_token() - self.assertNotEqual(token_old, token_new) + self.assertGreater(token_new, token_old) self.assertTrue(isinstance(b, A)) self.assertTrue(isinstance(b, (A,))) From c2ade98edba8db25f59deb4186cfe075ac7e1ded Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Mon, 30 Mar 2020 00:52:42 +0900 Subject: [PATCH 4/8] bpo-40077: Add comment for abc_invalidation_counter. --- Modules/_abc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Modules/_abc.c b/Modules/_abc.c index 5f1a83167a1e59..1c692bb345e9e4 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -24,7 +24,9 @@ typedef struct { PyObject *_abc_data_type; } _abcmodule_state; -/* A global counter that is incremented each time a class is +/* + FIXME: PEP 573: Move abc_invalidation_counter into _abcmodule_state. + A global counter that is incremented each time a class is registered as a virtual subclass of anything. It forces the negative cache to be cleared before its next use. Note: this counter is private. Use `abc.get_cache_token()` for From 0c0a8953c82783b0c000276dc81c6554d08cd6eb Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Mon, 30 Mar 2020 11:29:47 +0900 Subject: [PATCH 5/8] Update Modules/_abc.c Co-Authored-By: Victor Stinner --- Modules/_abc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Modules/_abc.c b/Modules/_abc.c index 1c692bb345e9e4..5bd55c5a52b369 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -24,13 +24,12 @@ typedef struct { PyObject *_abc_data_type; } _abcmodule_state; -/* - FIXME: PEP 573: Move abc_invalidation_counter into _abcmodule_state. - A global counter that is incremented each time a class is +/* A global counter that is incremented each time a class is registered as a virtual subclass of anything. It forces the negative cache to be cleared before its next use. Note: this counter is private. Use `abc.get_cache_token()` for external code. */ +// FIXME: PEP 573: Move abc_invalidation_counter into _abcmodule_state. static unsigned long long abc_invalidation_counter = 0; static inline _abcmodule_state* From 32eaf52a8d035d748c9a21f158d8fc141b9af91e Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Mon, 30 Mar 2020 11:31:06 +0900 Subject: [PATCH 6/8] bpo-40077: Remove NEWS.d --- .../Core and Builtins/2020-03-28-14-28-46.bpo-40077.4C_xJm.rst | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-03-28-14-28-46.bpo-40077.4C_xJm.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-03-28-14-28-46.bpo-40077.4C_xJm.rst b/Misc/NEWS.d/next/Core and Builtins/2020-03-28-14-28-46.bpo-40077.4C_xJm.rst deleted file mode 100644 index 379507acca1a96..00000000000000 --- a/Misc/NEWS.d/next/Core and Builtins/2020-03-28-14-28-46.bpo-40077.4C_xJm.rst +++ /dev/null @@ -1,3 +0,0 @@ -Replace statically allocated types with heap allocated types by using -:c:func:`PyType_FromSpec`. Add a module state to store the _abc_data_typ Add -traverse, clear and free functions to the module. From 98b9300d2151060a056ac62b505966378e4863e5 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Mon, 30 Mar 2020 22:37:37 +0900 Subject: [PATCH 7/8] bpo-40077: Apply code review. --- Modules/_abc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/_abc.c b/Modules/_abc.c index 5bd55c5a52b369..5dbe5c4130c8f4 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -82,9 +82,8 @@ PyDoc_STRVAR(abc_data_doc, static PyType_Slot _abc_data_type_spec_slots[] = { {Py_tp_doc, (void *)abc_data_doc}, - {Py_tp_dealloc, abc_data_dealloc}, - {Py_tp_alloc, PyType_GenericAlloc}, {Py_tp_new, abc_data_new}, + {Py_tp_dealloc, abc_data_dealloc}, {0, 0} }; From 14c3e52ad0d5d839a69a4cd68b4b837ac3f39230 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Mon, 30 Mar 2020 23:08:05 +0900 Subject: [PATCH 8/8] bpo-40077: Update the type of _abc_data_type. --- Modules/_abc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/_abc.c b/Modules/_abc.c index 5dbe5c4130c8f4..62709822f92335 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -21,7 +21,7 @@ _Py_IDENTIFIER(__subclasscheck__); _Py_IDENTIFIER(__subclasshook__); typedef struct { - PyObject *_abc_data_type; + PyTypeObject *_abc_data_type; } _abcmodule_state; /* A global counter that is incremented each time a class is @@ -102,7 +102,7 @@ _get_impl(PyObject *module, PyObject *self) if (impl == NULL) { return NULL; } - if (!Py_IS_TYPE(impl, (PyTypeObject *)state->_abc_data_type)) { + if (!Py_IS_TYPE(impl, state->_abc_data_type)) { PyErr_SetString(PyExc_TypeError, "_abc_impl is set to a wrong type"); Py_DECREF(impl); return NULL; @@ -418,7 +418,7 @@ _abc__abc_init(PyObject *module, PyObject *self) } /* Set up inheritance registry. */ - data = abc_data_new((PyTypeObject *)state->_abc_data_type, NULL, NULL); + data = abc_data_new(state->_abc_data_type, NULL, NULL); if (data == NULL) { return NULL; } @@ -832,7 +832,7 @@ static int _abcmodule_exec(PyObject *module) { _abcmodule_state *state = get_abc_state(module); - state->_abc_data_type = PyType_FromSpec(&_abc_data_type_spec); + state->_abc_data_type = (PyTypeObject *)PyType_FromSpec(&_abc_data_type_spec); if (state->_abc_data_type == NULL) { return -1; }