subject:"\[Python\-Dev\] Re\: Python 3.8 error\?"

[Python-Dev] Re: Python 3.8 error?

2019-12-12 Thread Christian Tismer

Pardon, I meant "there is no Python 3.8 version, yet".
And this is wrong, the MacOS pip install shows

PyQt5-5.13.2-5.13.2-cp35.cp36.cp37.cp38-abi3-macosx_10_6_intel.whl

So probably we have some bad oversight, somewhere.

Cheers -- Chris


On 12.12.19 13:48, Christian Tismer wrote:
> On 12.12.19 13:04, Petr Viktorin wrote:
>> I'm quite interested in the rest of the story here. PySide is probably
>> the biggest open-source user of the limited API, so IMO it is relevant
>> to this list.
> 
> 
> I guess that PyQt5 is a similar huge user, and it may be that they have
> the same problem, since there is no 5.14 version yet ;-)
> 
> 
>> On 2019-12-11 23:48, Christian Tismer wrote:
>>> Hi all,
>>>
>>> Sorry for the noise, I was wrong, and I retract.
>>> I was somehow mislead and hunted a phantom.
>>
>> Does that mean that there was never a problem?
>> Or that a problem is still there, but we don't know if it's in CPython
>> or PySide or their interaction?
>> Or was the problem solved in PySide?
>> Do we need to document something better, or just aware of some caveat?
> 
> 
> The problem is still there.
> When PySide creates a new type, then PyType_Ready digs into the new
> type, fetches the "mro()" method and uses the
> Py_TPFLAGS_METHOD_DESCRIPTOR flag to decide how to handle it.
> 
> When I leave the flag in, we crash. So this is the current fix,
> and I have no idea why this affects us at all. Here is the last change
> to SbkObjectTypeTpNew() #503:
> 
> // The meta type creates a new type when the Python programmer
> extends a wrapped C++ class.
> auto type_new = reinterpret_cast(PyType_Type.tp_new);
> 
> // PYSIDE-939: This is a temporary patch that circumvents the problem
> // with Py_TPFLAGS_METHOD_DESCRIPTOR until this is finally solved.
> // PyType_Ready uses mro(). We need to temporarily remove the flag
> from it's type.
> // We cannot use PyMethodDescr_Type since it is not exported by
> Python 2.7 .
> static PyTypeObject *PyMethodDescr_TypePtr = Py_TYPE(
> PyObject_GetAttr(reinterpret_cast(&PyType_Type),
> Shiboken::PyName::mro()));
> auto hold = PyMethodDescr_TypePtr->tp_flags;
> PyMethodDescr_TypePtr->tp_flags &= ~Py_TPFLAGS_METHOD_DESCRIPTOR;
> auto *newType = reinterpret_cast(type_new(metatype,
> args, kwds));
> PyMethodDescr_TypePtr->tp_flags = hold;
> 
> 
> I am still not sure where the bug is and who has it.
> My understanding is that this flag should have no impact on PySide
> at all, but it has.
> 
> Thanks for taking care -- 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,  > 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("

[Python-Dev] Re: Python 3.8 error?

2019-12-12 Thread Christian Tismer

On 12.12.19 13:04, Petr Viktorin wrote:
> I'm quite interested in the rest of the story here. PySide is probably
> the biggest open-source user of the limited API, so IMO it is relevant
> to this list.

BTW., the Python 3.8 change to type refcounting is already
breaking the Limited API a bit, because we have to dynamically
figure that out in order to be version-independent.

I am not so sure if that whole change was worth to break it?

Cheers -- Chris

--
Christian Tismer-Sperling:^)   tis...@stackless.com
Software Consulting  : http://www.stackless.com/
Strandstraße 37  : https://github.com/PySide
24217 Schönberg  : GPG key -> 0xFB7BEE0E
phone +49 173 24 18 776  fax +49 (30) 700143-0023
___
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/FVUU6DSHC4KRB2X7H2CJHFTHBENNPYZM/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Python 3.8 error?

2019-12-12 Thread Christian Tismer

On 12.12.19 13:04, Petr Viktorin wrote:
> I'm quite interested in the rest of the story here. PySide is probably
> the biggest open-source user of the limited API, so IMO it is relevant
> to this list.

I guess that PyQt5 is a similar huge user, and it may be that they have
the same problem, since there is no 5.14 version yet ;-)

> On 2019-12-11 23:48, Christian Tismer wrote:
>> Hi all,
>>
>> Sorry for the noise, I was wrong, and I retract.
>> I was somehow mislead and hunted a phantom.
> 
> Does that mean that there was never a problem?
> Or that a problem is still there, but we don't know if it's in CPython
> or PySide or their interaction?
> Or was the problem solved in PySide?
> Do we need to document something better, or just aware of some caveat?

The problem is still there.
When PySide creates a new type, then PyType_Ready digs into the new
type, fetches the "mro()" method and uses the
Py_TPFLAGS_METHOD_DESCRIPTOR flag to decide how to handle it.

When I leave the flag in, we crash. So this is the current fix,
and I have no idea why this affects us at all. Here is the last change
to SbkObjectTypeTpNew() #503:

// The meta type creates a new type when the Python programmer
extends a wrapped C++ class.
auto type_new = reinterpret_cast(PyType_Type.tp_new);

// PYSIDE-939: This is a temporary patch that circumvents the problem
// with Py_TPFLAGS_METHOD_DESCRIPTOR until this is finally solved.
// PyType_Ready uses mro(). We need to temporarily remove the flag
from it's type.
// We cannot use PyMethodDescr_Type since it is not exported by
Python 2.7 .
static PyTypeObject *PyMethodDescr_TypePtr = Py_TYPE(
PyObject_GetAttr(reinterpret_cast(&PyType_Type),
Shiboken::PyName::mro()));
auto hold = PyMethodDescr_TypePtr->tp_flags;
PyMethodDescr_TypePtr->tp_flags &= ~Py_TPFLAGS_METHOD_DESCRIPTOR;
auto *newType = reinterpret_cast(type_new(metatype,
args, kwds));
PyMethodDescr_TypePtr->tp_flags = hold;

I am still not sure where the bug is and who has it.
My understanding is that this flag should have no impact on PySide
at all, but it has.

Thanks for taking care -- 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, >>> > 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 (cust

[Python-Dev] Re: Python 3.8 error?

2019-12-12 Thread Petr Viktorin

I'm quite interested in the rest of the story here. PySide is probably 
the biggest open-source user of the limited API, so IMO it is relevant 
to this list.

On 2019-12-11 23:48, Christian Tismer wrote:

Hi all,

Sorry for the noise, I was wrong, and I retract.
I was somehow mislead and hunted a phantom.

Does that mean that there was never a problem?
Or that a problem is still there, but we don't know if it's in CPython 
or PySide or their interaction?

Or was the problem solved in PySide?
Do we need to document something better, or just aware of some caveat?

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, 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/AONFRH53HED5RN7VVT375JDMORGKEFQV/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Python 3.8 error?

2019-12-11 Thread Christian Tismer

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, > > 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/

[Python-Dev] Re: Python 3.8 error?

2019-12-09 Thread Christian Tismer

On 09.12.19 23:26, Nick Coghlan wrote:
> 
> 
> On Tue., 10 Dec. 2019, 5:17 am MRAB,  > 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

-- 
Christian Tismer-Sperling:^)   tis...@stackless.com
Software Consulting  : http://www.stackless.com/
Karl-Liebknecht-Str. 121 : https://www.qt.io/qt-for-python
14482 Potsdam: GPG key -> 0xE7301150FB7BEE0E
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/5MTJC47BHGZW6RKKU5JDAIFLV5P6W6JA/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Python 3.8 error?

2019-12-09 Thread Nick Coghlan

On Tue., 10 Dec. 2019, 5:17 am MRAB,  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.




>
___
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/QUVDUH6X2UKULIDTZQR67LECT54WOCZN/
Code of Conduct: http://python.org/psf/codeofconduct/

[Python-Dev] Re: Python 3.8 error?

2019-12-09 Thread MRAB


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, 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(&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(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/

[Python-Dev] Re: Python 3.8 error?

[Python-Dev] Re: Python 3.8 error?

[Python-Dev] Re: Python 3.8 error?

[Python-Dev] Re: Python 3.8 error?

[Python-Dev] Re: Python 3.8 error?

[Python-Dev] Re: Python 3.8 error?

[Python-Dev] Re: Python 3.8 error?

[Python-Dev] Re: Python 3.8 error?

8 matches

Site Navigation

Mail list logo

Footer information