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.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:59:24 | admin | set | github: 83197 |
2019-12-11 22:46:06 | Christian.Tismer | set | status: open -> closed resolution: not a bug |
2019-12-11 22:45:03 | Christian.Tismer | set | stage: patch review -> resolved |
2019-12-11 22:44:24 | Christian.Tismer | set | messages:
+ msg358284 |
2019-12-11 17:05:49 | petr.viktorin | set | messages:
+ msg358275 |
2019-12-11 17:00:43 | Christian.Tismer | set | messages:
+ msg358274 |
2019-12-11 16:37:27 | petr.viktorin | set | messages:
+ msg358271 |
2019-12-11 15:48:54 | Christian.Tismer | set | messages:
+ msg358268 |
2019-12-11 13:13:40 | petr.viktorin | set | nosy:
+ petr.viktorin messages:
+ msg358260
|
2019-12-10 22:13:15 | Christian.Tismer | set | messages:
+ msg358228 |
2019-12-10 17:07:32 | Christian.Tismer | set | messages:
+ msg358207 |
2019-12-10 16:57:16 | pablogsal | set | messages:
+ msg358205 |
2019-12-10 16:54:54 | Christian.Tismer | set | messages:
+ msg358203 |
2019-12-10 16:21:43 | pablogsal | set | nosy:
+ pablogsal messages:
+ msg358201
|
2019-12-10 15:38:18 | Christian.Tismer | set | keywords:
+ patch stage: patch review pull_requests:
+ pull_request17029 |
2019-12-10 15:37:03 | Christian.Tismer | create | |