| msg358198 - (view) |
Author: Christian Tismer (Christian.Tismer) *  |
Date: 2019-12-10 15:37 |
| By the new Py_TPFLAGS_METHOD_DESCRIPTOR flag, a new code path is activated, and when extension types like PySide create a new class, we observe negative refcounts. The reason is that the code in typeobject.c fkt. type_mro_modified calls lookup_maybe_method which returns a _borrowed_ reference. This happens in the "if (custom) {" branch. Removing all Py_XDECREF calls from the function fixes that. |
|
|
| msg358201 - (view) |
Author: Pablo Galindo Salgado (pablogsal) * |
Date: 2019-12-10 16:21 |
| From the PR: Christian, can you open the PR against master instead? We will backport the change to 3.8 after is merged. |
|
|
| msg358203 - (view) |
Author: Christian Tismer (Christian.Tismer) * |
Date: 2019-12-10 16:54 |
| No, this appears to be impossible. The function "type_mro_modified" exists as well, but there is no "if (custom) {" branch at all! On 10.12.19 17:21, Pablo Galindo Salgado wrote: > > Pablo Galindo Salgado <pablogsal@gmail.com> added the comment: > >>From the PR: > > Christian, can you open the PR against master instead? We will backport the change to 3.8 after is merged. > > ---------- > nosy: +pablogsal > > _______________________________________ > Python tracker <report@bugs.python.org> > <https://bugs.python.org/issue39016> > _______________________________________ > |
|
|
| msg358205 - (view) |
Author: Pablo Galindo Salgado (pablogsal) * |
Date: 2019-12-10 16:57 |
| > No, this appears to be impossible. Oh, I see. Apologies then for the misunderstunding. |
|
|
| msg358207 - (view) |
Author: Christian Tismer (Christian.Tismer) * |
Date: 2019-12-10 17:07 |
| On 10.12.19 17:57, Pablo Galindo Salgado wrote: > > Pablo Galindo Salgado <pablogsal@gmail.com> added the comment: > >> No, this appears to be impossible. > > Oh, I see. Apologies then for the misunderstunding. Well, but I think that is weird, too! Why should that custom clause be in 3.8 but not in master? |
|
|
| msg358228 - (view) |
Author: Christian Tismer (Christian.Tismer) * |
Date: 2019-12-10 22:13 |
| On 10.12.19 17:57, Pablo Galindo Salgado wrote: > > Pablo Galindo Salgado <pablogsal@gmail.com> added the comment: > >> No, this appears to be impossible. > > Oh, I see. Apologies then for the misunderstunding. No problem! You could as well have been right. I tried to move the patch to master and realized only then that there was nothing similar. Interesting btw., I should investigate when this divergence was introduced. Best -- Chris |
|
|
| msg358260 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2019-12-11 13:13 |
| lookup_maybe_method should not return a borrowed reference. It increfs its return value. |
|
|
| msg358268 - (view) |
Author: Christian Tismer (Christian.Tismer) * |
Date: 2019-12-11 15:48 |
| On 11.12.19 14:13, Petr Viktorin wrote: > > Petr Viktorin <encukou@gmail.com> added the comment: > > lookup_maybe_method should not return a borrowed reference. It increfs its return value. At second sight, this seems to be true. No idea why I was so convinced. Need to sleep and look again.. |
|
|
| msg358271 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2019-12-11 16:37 |
| The goal now should be to find a reasonably small reproducer. I'm trying to compile PySide to see what it does, but it's a big project and I'm a bit lost. Christian, you said in an e-mail that you have a workaround -- unsetting the Py_TPFLAGS_METHOD_DESCRIPTOR. Could you point me to the commit with that workaround, so I could try exploring the code in context? |
|
|
| msg358274 - (view) |
Author: Christian Tismer (Christian.Tismer) * |
Date: 2019-12-11 17:00 |
| On 11.12.19 17:37, Petr Viktorin wrote: > > Petr Viktorin <encukou@gmail.com> added the comment: > > The goal now should be to find a reasonably small reproducer. > > I'm trying to compile PySide to see what it does, but it's a big project and I'm a bit lost. > Christian, you said in an e-mail that you have a workaround -- unsetting the Py_TPFLAGS_METHOD_DESCRIPTOR. Could you point me to the commit with that workaround, so I could try exploring the code in context? Yes, it went into gerrit here: https://codereview.qt-project.org/c/pyside/pyside-setup/+/282705 The problem is that during PySide type creation the PyType_Ready function looks into the mro() method, which uses this new flag. When I temporary remove that, everything works. Cheers -- Chris |
|
|
| msg358275 - (view) |
Author: Petr Viktorin (petr.viktorin) * |
Date: 2019-12-11 17:05 |
| Thanks! (note: unfortunately I'll probably not have time for CPython until next week, but further investigation is on my list.) |
|
|
| msg358284 - (view) |
Author: Christian Tismer (Christian.Tismer) * |
Date: 2019-12-11 22:44 |
| Sorry, I believe I was wrong and lookup_maybe_method does not return a borrowed reference. _PyType_Lookup does and I was confused. Closing that for now. |
|
|