On 2019-12-09 18:22, Christian Tismer wrote:
On 08.12.19 09:49, Nick Coghlan wrote:
On Fri., 6 Dec. 2019, 3:31 am Christian Tismer, <tis...@stackless.com
<mailto:tis...@stackless.com>> wrote:
Hi guys,
during the last few weeks I have been struggling quite much
in order to make PySide run with Python 3.8 at all.
The expected problems were refcounting leaks due to changed
handling of heaptypes. But in fact, the runtime behavior was
much worse, because I always got negative refcounts!
After exhaustive searching through the different 3.8 commits, I could
isolate the three problems with logarithmic search.
The hard problem was this:
Whenever PySide creates a new type, it crashes in PyType_Ready.
The reason is the existence of the Py_TPFLAGS_METHOD_DESCRIPTOR
flag.
During the PyType_Ready call, the function mro() is called.
This mro() call results in a negative refcount, because something
behaves differently since this flag is set by default in mro().
When I patched this flag away during the type_new call, everything
worked ok. I don't understand why this problem affects PySide
at all. Here is the code that would normally be only the newType line:
// PYSIDE-939: This is a temporary patch that circumvents the
problem
// with Py_TPFLAGS_METHOD_DESCRIPTOR until this is finally solved.
PyObject *ob_PyType_Type = reinterpret_cast<PyObject
*>(&PyType_Type);
PyObject *mro = PyObject_GetAttr(ob_PyType_Type,
Shiboken::PyName::mro());
auto hold = Py_TYPE(mro)->tp_flags;
Py_TYPE(mro)->tp_flags &= ~Py_TPFLAGS_METHOD_DESCRIPTOR;
auto *newType = reinterpret_cast<SbkObjectType *>(type_new(metatype,
args, kwds));
Py_TYPE(mro)->tp_flags = hold;
Isn't this manipulating the flags in the tuple type, rather than
anything on a custom object? Or is "mro" a custom object rather than an
MRO tuple?
If anything, given the combination of factors required to reproduce the
problem, I would guess that there might be a ref counting problem in the
__set_owner__ invocations when called on a new type rather than a
regular instance, and that was somehow affected by the change to
increment the type refcount in PyObject_Init rather than
PyType_GenericAlloc.
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".
_______________________________________________
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/EZ3ETF5LJEO3Q2NKUXWRZ54IQK744NVZ/
Code of Conduct: http://python.org/psf/codeofconduct/
