Hi all, Sorry for the noise, I was wrong, and I retract. I was somehow mislead and hunted a phantom.
Best - Chris On 10.12.19 00:29, Christian Tismer wrote: > On 09.12.19 23:26, Nick Coghlan wrote: >> >> >> On Tue., 10 Dec. 2019, 5:17 am MRAB, <pyt...@mrabarnett.plus.com >> <mailto:pyt...@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). > > Thanks Nick for this nice analysis. And it's exactly this codepath that > is taken in the case of PySide: custom types all the time :-) > > what-a-relief - ly y'rs -- Chris > > > _______________________________________________ > Python-Dev mailing list -- python-dev@python.org > To unsubscribe send an email to python-dev-le...@python.org > https://mail.python.org/mailman3/lists/python-dev.python.org/ > Message archived at > https://mail.python.org/archives/list/python-dev@python.org/message/5MTJC47BHGZW6RKKU5JDAIFLV5P6W6JA/ > Code of Conduct: http://python.org/psf/codeofconduct/ > -- Christian Tismer :^) tis...@stackless.com Software Consulting : http://www.stackless.com/ Karl-Liebknecht-Str. 121 : https://github.com/PySide 14482 Potsdam : GPG key -> 0xFB7BEE0E phone +49 173 24 18 776 fax +49 (30) 700143-0023
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/5PBHW3HEQQ4KLRUATZ7YGX5TR3T24R4C/ Code of Conduct: http://python.org/psf/codeofconduct/
