[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Change by Petr Viktorin : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker ___

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

miss-islington added the comment: New changeset bcbe5c59dde5fcb9ad21991c2afd91837b14bbd5 by Miss Islington (bot) in branch '3.9': bpo-40217: Ensure Py_VISIT(Py_TYPE(self)) is always called for PyType_FromSpec types (reverts GH-19414) (GH-20264)

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Change by miss-islington : -- pull_requests: +19739 pull_request: https://github.com/python/cpython/pull/20490 ___ Python tracker ___

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

miss-islington added the comment: New changeset 1cf15af9a6f28750f37b08c028ada31d38e818dd by Pablo Galindo in branch 'master': bpo-40217: Ensure Py_VISIT(Py_TYPE(self)) is always called for PyType_FromSpec types (reverts GH-19414) (GH-20264)

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Change by Petr Viktorin : -- pull_requests: +19690 stage: resolved -> patch review pull_request: https://github.com/python/cpython/pull/20433 ___ Python tracker ___

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Petr Viktorin added the comment: Thank you for responding so quickly! > Petr, do you mind reviewing it? Yes, but I'll only get to it next week. -- ___ Python tracker ___

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: > If you add a Py_INCREF before, the crash disappears. To be clear: the other crash is still in the reproduced: the one that Petr describes in his comment. In PR 20264 I have prepared the changes that I proposed previously (including the revert of

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: Ok, I found the problem. The problem is that the reproduced does not correctly work the reference count of base_class because when construction get tuple of bases: PyObject *bases = PyTuple_New(1); result = PyTuple_SetItem(bases, 0, base_class); if

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: I have been playing with the reproducer and I am a bit confused: The reproducer crashes in the same way even after reverting PR 19414 so it does not seem related to it. This is what I get: >>> import reproducer >>> reproducer.Modules/gcmodule.c:114:

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: > What would be the downsides of reverting and documenting that tp_traverse > must visit Py_TYPE(self)? Not much IMHO, I actually would prefer this solution over any automatic "hacks". The reason is that any hack will be technically violating the

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Change by Petr Viktorin : Added file: https://bugs.python.org/file49182/reproducer.c ___ Python tracker ___ ___ Python-bugs-list mailing

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Petr Viktorin added the comment: Ha! I think I found the issue in PySide now. It's different, but it's still a CPython issue. It's actually a variant of the "defining class" problem from PEP 573. It's legal to get the value of a slot, using PyType_GetSlot. It's reasonable to assume that

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Petr Viktorin added the comment: Indeed, it seems my initial analysis was incorrect. I can't reproduce this outside of PySide. I'll close this bug and invesatigate more. Sorry for the noise. -- resolution: -> fixed stage: patch review -> resolved status: open -> closed

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: Actually, I have been thinking about this more and I cannot really trace how the bug could happen. We control the type allocation, but Py_tp_alloc controls the instance allocation and that can be anything, the current patch should not interfere with

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Change by Pablo Galindo Salgado : -- pull_requests: +19550 stage: resolved -> patch review pull_request: https://github.com/python/cpython/pull/20264 ___ Python tracker ___

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: Maybe we should revert this change and modify the "how to port to ..." section as we did in bpo-35810 to tell users that they need to manually visit the parent in classes created with PyType_FromSpec --

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Change by Pablo Galindo Salgado : -- Removed message: https://bugs.python.org/msg369463 ___ Python tracker ___ ___ Python-bugs-list

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: Maybe we should revert -- priority: normal -> release blocker ___ Python tracker ___ ___

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: Re-opening the issue -- resolution: fixed -> status: closed -> open ___ Python tracker ___

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Petr Viktorin added the comment: It looks like the fix breaks types that override Py_tp_alloc but not Py_tp_traverse (as some types in PySide do.) I'll investigate more and work on fixing that if someone doesn't beat me to it. -- nosy: +petr.viktorin

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

STINNER Victor added the comment: The issue has been fixed in the master branch, thanks Pablo! I close the issue. -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

STINNER Victor added the comment: > Please backport to 3.8, then it will become part of 3.8.3rc1 which I'll be > releasing tomorrow. I propose to *not* fix this bug in Python 3.8: * Python 3.8 stdlib doesn't seem to be impacted by this bug * The number of third party C extension modules

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: New changeset 91a5ae18351027867e99c96db5ea235d9c42e47a by Pablo Galindo in branch 'master': bpo-40217: Clean code in PyType_FromSpec_Alloc and add NEWS entry (GH-19733) https://github.com/python/cpython/commit/91a5ae18351027867e99c96db5ea235d9c42e47a

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Łukasz Langa added the comment: Please backport to 3.8, then it will become part of 3.8.3rc1 which I'll be releasing tomorrow. -- nosy: +lukasz.langa ___ Python tracker ___

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

STINNER Victor added the comment: FYI I closed bpo-40149: the commit 0169d3003be3d072751dd14a5c84748ab63a249f fixed test_threading leak. -- For master, it would be nice to have a NEWS entry, and maybe also a What's New in Python 3.9 entry, maybe in the "Changes in the C API" section. --

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Change by Pablo Galindo Salgado : -- pull_requests: +19054 pull_request: https://github.com/python/cpython/pull/19733 ___ Python tracker ___

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

STINNER Victor added the comment: > Serhiy: would it be possible to fix this issue in alpha6? Can we merge PR > 19414? In the lack of reply, I think that the best to merge PR 19414. I asked Lukasz (Python 3.9 release manager) in private and he is ok to merge this PR. He plans to wait until

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

STINNER Victor added the comment: New changeset 0169d3003be3d072751dd14a5c84748ab63a249f by Pablo Galindo in branch 'master': bpo-40217: Ensure Py_VISIT(Py_TYPE(self)) is always called for PyType_FromSpec types (GH-19414)

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

STINNER Victor added the comment: Serhiy: would it be possible to fix this issue in alpha6? Can we merge PR 19414? In the lack of reply, I think that the best to merge PR 19414. -- ___ Python tracker

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Tim Peters added the comment: There is no possible world in which the best answer is "hack gcmodule.c" ;-) I haven't tried to dig into the details here, but Pablo's approach looked spot-on to me: put the solution near the source of the problem. The problem is specific to a relatively tiny

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

STINNER Victor added the comment: > I do not think it is right. Please wait, I'll submit an alternate solution. What is your idea? How would it be different than PR 19414? Refleak buildbots are broken for almost one month (bpo-40149), it would be nice to get a fix soon. In Python 3.9.0a6 if

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

STINNER Victor added the comment: There are limited options to fix this issue: (A) Revert commit 364f0b0f19cc3f0d5e63f571ec9163cf41c62958 It would reintroduced bpo-35810 bug. Moreover, C extension modules which have been modified to call Py_DECREF(Py_TYPE(self)) in tp_dealloc which suddenly

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Serhiy Storchaka added the comment: I do not think it is right. Please wait, I'll submit an alternate solution. -- ___ Python tracker ___

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

STINNER Victor added the comment: Tests should be added, maybe based on msg365963 examples. But I would prefer to get this bug fixed in 3.9.0a6 (if possible), tests can be added later. -- ___ Python tracker

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

STINNER Victor added the comment: I'm not comfortable with requesting authors of all C extensions to modify their tp_traverse function. As Pablo explained, the type is already visited on subtypes instances. I approved PR 19414. -- ___ Python

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: Sorry when I said: > If we do the same experiment after PR19314: I meant: If we do the same experiment after PR 19414: -- ___ Python tracker

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: I have made some investigation, and I think a form of this bug was there from a long time and does not really relate to heap types. For instance consider this code on Python3.7 (so commit 364f0b0f19cc3f0d5e63f571ec9163cf41c62958 is not present). If

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: In https://github.com/python/cpython/pull/19414 I have attached a proof of concept of a wrapper for tp_traverse that only affects types created by PyType_FromSpec that will call Py_VISIT(Py_TYPE(self)) and then the user function. --

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Change by Pablo Galindo Salgado : -- keywords: +patch pull_requests: +18774 stage: -> patch review pull_request: https://github.com/python/cpython/pull/19414 ___ Python tracker

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: > Now we changed rules. A strong reference is created implicitly. Who is > responsible to manage a strong reference? Whose who created it, ant it is the > interpreter, not the user. Even if we decide that the patch that introduced the new rules

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Serhiy Storchaka added the comment: The problem is that we suddenly changed rules. It was not required that the object's type should be visited in tp_visit. It was incorrect to visit it, because object did not have strong reference to its type. User never created it, and it was not created

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: > We cannot change all user code, so we should change the interpreter code so > that it will work correctly with existing user code. If we made a change that make all user code suddenly incorrect, that change should be reverted. The GC has clear

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Serhiy Storchaka added the comment: Recently many static allocated types were converted to heap allocated types (using PyType_FromSpec). Now you need to add Py_VISIT(Py_TYPE(self)) in all corresponding tp_visit implementations. And since even official example for heap allocated types do not

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: > Hummm, I think we may just be missing a Py_VISIT(Py_TYPE(self))here: Checking it more closely, that is incorrect, so we are not missing that visitation :( -- ___ Python tracker

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: Hummm, I think we may just be missing a Py_VISIT(Py_TYPE(self))here: https://github.com/python/cpython/blob/master/Objects/typeobject.c#L3562 A class owns references to its type (type) or another metaclass Tim, could you confirm that that is the

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: Also, as I mentioned, you don't need to modify all objects tp_traverse, only it's type.tp_traverse slot. For instance, all python objects know how to traverse stuff because they share the same tp_traverse:

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Serhiy Storchaka added the comment: Interesting, this issue may be related to issue24379. The problem with the proposed implementation of subscript was that it created a reference loop, and not all links in this loop were visible by GC. -- ___

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: >>> Since tp_visit is GC specific thing, I think it wold be more convenient >>> make a special case for object types. I don't think that follows: The gc defers the logic of what and what should not be visited to the tp_traverse implementation of

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Serhiy Storchaka added the comment: It would be inconvenient to require adding Py_VISIT(Py_TYPE(self)) in all tp_visit implementations of heap allocated types (including third-party extensions). Since tp_visit is GC specific thing, I think it wold be more convenient make a special case for

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Tim Peters added the comment: If object A owns a strong reference to object B, and A participates in cyclic gc, and B may be part of a cycle, then it's necessary and sufficient that A's type's tp_traverse implementation invoke Py_VISIT() passing A's pointer to B. It would be a Really Bad

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: Also, heap types (created with type_new()) are already taking into account the type when visiting references: https://github.com/python/cpython/blob/master/Objects/typeobject.c#L -- ___ Python tracker

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Pablo Galindo Salgado added the comment: > I propose to modify the GC to take bpo-35810 in account. What? The GC is agnostic of what it receives. It works with objects in general that implement the gc support, but it does not care about what those objects are. The only specal case are

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Change by hai shi : -- nosy: +shihai1991 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

Change by Dong-hee Na : -- nosy: +corona10 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue40217] The garbage collector doesn't take in account that objects of heap allocated types hold a strong reference to their type

New submission from STINNER Victor : The bpo-35810 modified the object allocate to hold a *strong* reference to the type in PyObject.ob_type, whereas PyObject.ob_type is a *borrowed* references if the type is statically allocated. commit 364f0b0f19cc3f0d5e63f571ec9163cf41c62958 Author: Eddie