From 278036fc11f2bd307209816fc8a8f861371c78df Mon Sep 17 00:00:00 2001 From: Julien Palard Date: Tue, 7 May 2019 16:58:31 +0200 Subject: [PATCH 1/7] No type cache for types with specialized mro, invalidation is hard. --- Objects/typeobject.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index eeaae1f9f78947..8d207cac091ba9 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1925,6 +1925,8 @@ mro_invoke(PyTypeObject *type) &unbound); if (mro_meth == NULL) return NULL; + type->tp_flags &= ~(Py_TPFLAGS_HAVE_VERSION_TAG| + Py_TPFLAGS_VALID_VERSION_TAG); mro_result = call_unbound_noarg(unbound, mro_meth, (PyObject *)type); Py_DECREF(mro_meth); } From 3fb14a99ebbcecea39c4e45642240cff53cf0849 Mon Sep 17 00:00:00 2001 From: Julien Palard Date: Tue, 7 May 2019 21:49:36 +0200 Subject: [PATCH 2/7] FIX: Don't disable method cache custom types that do not implement mro(). --- Objects/typeobject.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 8d207cac091ba9..545c71faf72c7b 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -287,32 +287,43 @@ type_mro_modified(PyTypeObject *type, PyObject *bases) { Unset HAVE_VERSION_TAG and VALID_VERSION_TAG if the type has a custom MRO that includes a type which is not officially - super type. + super type, or if the type implements its own mro() method. Called from mro_internal, which will subsequently be called on each subclass when their mro is recursively updated. */ Py_ssize_t i, n; int clear = 0; + int custom = (Py_TYPE(type) != &PyType_Type); if (!PyType_HasFeature(type, Py_TPFLAGS_HAVE_VERSION_TAG)) return; - n = PyTuple_GET_SIZE(bases); - for (i = 0; i < n; i++) { - PyObject *b = PyTuple_GET_ITEM(bases, i); - PyTypeObject *cls; + if (custom) { + PyObject *mro_meth = lookup_method((PyObject *)type, &PyId_mro, + &unbound); + PyObject *type_mro_meth = lookup_method(&PyType_Type, &PyId_mro, + &unbound); + if (mro_meth == NULL || type_mro_meth == NULL || + mro_meth != type_mro_meth) + clear = 1; + } + if (!clear) { + n = PyTuple_GET_SIZE(bases); + for (i = 0; i < n; i++) { + PyObject *b = PyTuple_GET_ITEM(bases, i); + PyTypeObject *cls; - assert(PyType_Check(b)); - cls = (PyTypeObject *)b; + assert(PyType_Check(b)); + cls = (PyTypeObject *)b; - if (!PyType_HasFeature(cls, Py_TPFLAGS_HAVE_VERSION_TAG) || - !PyType_IsSubtype(type, cls)) { - clear = 1; - break; + if (!PyType_HasFeature(cls, Py_TPFLAGS_HAVE_VERSION_TAG) || + !PyType_IsSubtype(type, cls)) { + clear = 1; + break; + } } } - if (clear) type->tp_flags &= ~(Py_TPFLAGS_HAVE_VERSION_TAG| Py_TPFLAGS_VALID_VERSION_TAG); @@ -1925,8 +1936,6 @@ mro_invoke(PyTypeObject *type) &unbound); if (mro_meth == NULL) return NULL; - type->tp_flags &= ~(Py_TPFLAGS_HAVE_VERSION_TAG| - Py_TPFLAGS_VALID_VERSION_TAG); mro_result = call_unbound_noarg(unbound, mro_meth, (PyObject *)type); Py_DECREF(mro_meth); } From beac98bda7a3f4678527aba0e951abbc85cf3079 Mon Sep 17 00:00:00 2001 From: Julien Palard Date: Tue, 7 May 2019 22:38:29 +0200 Subject: [PATCH 3/7] fixing implem. --- Objects/typeobject.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 545c71faf72c7b..90e5c292669da2 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -78,6 +78,9 @@ slot_tp_new(PyTypeObject *type, PyObject *args, PyObject *kwds); static void clear_slotdefs(void); +static PyObject * +lookup_method(PyObject *self, _Py_Identifier *attrid, int *unbound); + /* * finds the beginning of the docstring's introspection signature. * if present, returns a pointer pointing to the first '('. @@ -300,9 +303,11 @@ type_mro_modified(PyTypeObject *type, PyObject *bases) { return; if (custom) { + _Py_IDENTIFIER(mro); + int unbound; PyObject *mro_meth = lookup_method((PyObject *)type, &PyId_mro, &unbound); - PyObject *type_mro_meth = lookup_method(&PyType_Type, &PyId_mro, + PyObject *type_mro_meth = lookup_method((PyObject *)&PyType_Type, &PyId_mro, &unbound); if (mro_meth == NULL || type_mro_meth == NULL || mro_meth != type_mro_meth) From dde48ab4cce847cf93bd4a66cb49a79bd89882cc Mon Sep 17 00:00:00 2001 From: Julien Palard Date: Wed, 8 May 2019 16:32:17 +0200 Subject: [PATCH 4/7] Avoid storing error flags, also decref. --- Objects/typeobject.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 90e5c292669da2..cad20b43c88cea 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -79,7 +79,7 @@ static void clear_slotdefs(void); static PyObject * -lookup_method(PyObject *self, _Py_Identifier *attrid, int *unbound); +lookup_maybe_method(PyObject *self, _Py_Identifier *attrid, int *unbound); /* * finds the beginning of the docstring's introspection signature. @@ -305,13 +305,19 @@ type_mro_modified(PyTypeObject *type, PyObject *bases) { if (custom) { _Py_IDENTIFIER(mro); int unbound; - PyObject *mro_meth = lookup_method((PyObject *)type, &PyId_mro, - &unbound); - PyObject *type_mro_meth = lookup_method((PyObject *)&PyType_Type, &PyId_mro, - &unbound); + PyObject *mro_meth = lookup_maybe_method( + (PyObject *)type, &PyId_mro, &unbound); + PyObject *type_mro_meth = lookup_maybe_method( + (PyObject *)&PyType_Type, &PyId_mro, &unbound); if (mro_meth == NULL || type_mro_meth == NULL || mro_meth != type_mro_meth) + { clear = 1; + } + if (mro_meth != NULL) + Py_DECREF(mro_meth); + if (type_mro_meth != NULL) + Py_DECREF(type_mro_meth); } if (!clear) { n = PyTuple_GET_SIZE(bases); From 724362d0ccc45978c0dfb73dc38cceff2bb87b5e Mon Sep 17 00:00:00 2001 From: Julien Palard Date: Wed, 8 May 2019 16:36:58 +0200 Subject: [PATCH 5/7] news entry --- .../Core and Builtins/2019-05-08-16-36-51.bpo-28866.qCv_bj.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-05-08-16-36-51.bpo-28866.qCv_bj.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-05-08-16-36-51.bpo-28866.qCv_bj.rst b/Misc/NEWS.d/next/Core and Builtins/2019-05-08-16-36-51.bpo-28866.qCv_bj.rst new file mode 100644 index 00000000000000..69017293649cd5 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-05-08-16-36-51.bpo-28866.qCv_bj.rst @@ -0,0 +1,2 @@ +Avoid caching attributes of classes which type defines mro() to avoid a hard +cache invalidation problem. From c65583b92e165d2a93dce36949f4f62a10bf7ce6 Mon Sep 17 00:00:00 2001 From: Julien Palard Date: Wed, 8 May 2019 19:43:18 +0200 Subject: [PATCH 6/7] Clear as soon as we're getting an error. --- Objects/typeobject.c | 54 +++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index cad20b43c88cea..ddbc81d1d37182 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -296,48 +296,46 @@ type_mro_modified(PyTypeObject *type, PyObject *bases) { each subclass when their mro is recursively updated. */ Py_ssize_t i, n; - int clear = 0; int custom = (Py_TYPE(type) != &PyType_Type); + int unbound; + PyObject *mro_meth = NULL; + PyObject *type_mro_meth = NULL; if (!PyType_HasFeature(type, Py_TPFLAGS_HAVE_VERSION_TAG)) return; if (custom) { _Py_IDENTIFIER(mro); - int unbound; - PyObject *mro_meth = lookup_maybe_method( + mro_meth = lookup_maybe_method( (PyObject *)type, &PyId_mro, &unbound); - PyObject *type_mro_meth = lookup_maybe_method( + if (mro_meth == NULL) + goto clear; + type_mro_meth = lookup_maybe_method( (PyObject *)&PyType_Type, &PyId_mro, &unbound); - if (mro_meth == NULL || type_mro_meth == NULL || - mro_meth != type_mro_meth) - { - clear = 1; - } - if (mro_meth != NULL) - Py_DECREF(mro_meth); - if (type_mro_meth != NULL) - Py_DECREF(type_mro_meth); + if (type_mro_meth == NULL) + goto clear; + if (mro_meth != type_mro_meth) + goto clear; } - if (!clear) { - n = PyTuple_GET_SIZE(bases); - for (i = 0; i < n; i++) { - PyObject *b = PyTuple_GET_ITEM(bases, i); - PyTypeObject *cls; + n = PyTuple_GET_SIZE(bases); + for (i = 0; i < n; i++) { + PyObject *b = PyTuple_GET_ITEM(bases, i); + PyTypeObject *cls; - assert(PyType_Check(b)); - cls = (PyTypeObject *)b; + assert(PyType_Check(b)); + cls = (PyTypeObject *)b; - if (!PyType_HasFeature(cls, Py_TPFLAGS_HAVE_VERSION_TAG) || - !PyType_IsSubtype(type, cls)) { - clear = 1; - break; - } + if (!PyType_HasFeature(cls, Py_TPFLAGS_HAVE_VERSION_TAG) || + !PyType_IsSubtype(type, cls)) { + goto clear; } } - if (clear) - type->tp_flags &= ~(Py_TPFLAGS_HAVE_VERSION_TAG| - Py_TPFLAGS_VALID_VERSION_TAG); + return; + clear: + Py_XDECREF(mro_meth); + Py_XDECREF(type_mro_meth); + type->tp_flags &= ~(Py_TPFLAGS_HAVE_VERSION_TAG| + Py_TPFLAGS_VALID_VERSION_TAG); } static int From cce9c148eee99fc7904097802d72c4cd17a10a7f Mon Sep 17 00:00:00 2001 From: Julien Palard Date: Fri, 24 May 2019 14:16:17 +0200 Subject: [PATCH 7/7] FIX: Reference leak. --- Objects/typeobject.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index ddbc81d1d37182..65cb86c6bebced 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -316,6 +316,8 @@ type_mro_modified(PyTypeObject *type, PyObject *bases) { goto clear; if (mro_meth != type_mro_meth) goto clear; + Py_XDECREF(mro_meth); + Py_XDECREF(type_mro_meth); } n = PyTuple_GET_SIZE(bases); for (i = 0; i < n; i++) {