On Tue., 10 Dec. 2019, 5:17 am MRAB, <python@mrabarnett.plus.com> wrote:
On 2019-12-09 18:22, Christian Tismer wrote:
Hi Nick,
after staring long at the code, I fount something funny in typeobject.c #286 ff:
static void type_mro_modified(PyTypeObject *type, PyObject *bases) { /* Check that all base classes or elements of the MRO of type are able to be cached. This function is called after the base classes or mro of the type are altered.
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, 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 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); mro_meth = lookup_maybe_method( (PyObject *)type, &PyId_mro, &unbound); if (mro_meth == NULL) goto clear; type_mro_meth = lookup_maybe_method( (PyObject *)&PyType_Type, &PyId_mro, &unbound); if (type_mro_meth == NULL) goto clear; if (mro_meth != type_mro_meth) goto clear; Py_XDECREF(mro_meth); Py_XDECREF(type_mro_meth); }
Look at the "if (custom)" clause. "mro_meth = lookup_maybe_method(" uses lookup_maybe_method which gives a borrowed reference. The same holds for "type_mro_meth".
But then both are decreffed, which IMHO is not correct.
Look at what happens at the label "clear": it DECREFs them.
If mro_meth != NULL or mro_meth != type_mro_meth, they'll get DECREFed at "clear".
I believe Christian's point is that this entire "if (custom) {" branch looks suspicious, as it assumes "lookup_maybe_method" will increment the refcount on the returned object. If that assumption is incorrect, we're going to get DECREFs without a preceding INCREF. The specific code path is also obscure enough that it's plausible the test suite may not currently cover it (as it requires doing something that calls "type_mro_modified" on a type with a custom metaclass). Cheers, Nick.