[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Raymond Hettinger added the comment: False alarm. I misread the diff. All is well. -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Pablo Galindo Salgado added the comment: Also, the ordered dict dealloc is already doing that: https://github.com/python/cpython/blob/a856364cc920d8b16750fd1fadc902efb509754c/Objects/odictobject.c#L1372-L1373 -- ___ Python tracker

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Pablo Galindo Salgado added the comment: > I'm thinking that edit to tp_dealloc was incorrect. What edit are you referring to? PR 18749 only touches tp_clear and tp_traverse, not tp_dealloc -- ___ Python tracker

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Change by STINNER Victor : -- nosy: -vstinner ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Raymond Hettinger added the comment: Also a test should be added to make sure the weakref callbacks are still occurring. -- ___ Python tracker ___

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Raymond Hettinger added the comment: I'm thinking that edit to tp_dealloc was incorrect. The PyClear call should have been replaced (not removed) with the pattern shown in the C APIs docs ( https://docs.python.org/3/extending/newtypes.html?highlight=pyobject_clearw#weak-reference-support

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Pablo Galindo Salgado added the comment: > I could not reproduce the crash and from the discussion it seems resolved. Is > there anything left here? No, this should be fixed by now. I just forgot to close the issue. Thanks for the ping! -- ___

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Irit Katriel added the comment: I could not reproduce the crash and from the discussion it seems resolved. Is there anything left here? -- nosy: +iritkatriel ___ Python tracker

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

STINNER Victor added the comment: It seems like OrderedDict was the only type which visited weak references: $ scm.py grep 'VISIT.*weak' master: Grep 'VISIT.*weak' -- <4284 filenames> == Objects/odictobject.c:1457:Py_VISIT(od->od_weakreflist);

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

miss-islington added the comment: New changeset 72fff60d7649df88026838d8b5f14f541393f268 by Miss Islington (bot) in branch '3.7': bpo-39778: Add clarification about tp_traverse and ownership (GH-18754) https://github.com/python/cpython/commit/72fff60d7649df88026838d8b5f14f541393f268

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

miss-islington added the comment: New changeset 1827fc30f463786ebff13752e35c3224652bc94e by Miss Islington (bot) in branch '3.8': bpo-39778: Add clarification about tp_traverse and ownership (GH-18754) https://github.com/python/cpython/commit/1827fc30f463786ebff13752e35c3224652bc94e

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

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

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

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

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

miss-islington added the comment: New changeset 6df421fe87a9418d6c59f89dbc5d5573b6826855 by Pablo Galindo in branch 'master': bpo-39778: Add clarification about tp_traverse and ownership (GH-18754) https://github.com/python/cpython/commit/6df421fe87a9418d6c59f89dbc5d5573b6826855 --

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Pablo Galindo Salgado added the comment: New changeset 9ddcb914f9c2debe7c1359b2450cd1573e86b91c by Pablo Galindo in branch '3.8': [3.8] bpo-39778: Don't traverse weak-reference lists OrderedDict's tp_traverse and tp_clear (GH-18749) (GH-18756)

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

miss-islington added the comment: New changeset 69ded3944c202da972754644c0bbf7f77cc5e8ea by Miss Islington (bot) in branch '3.7': bpo-39778: Don't traverse weak-reference lists OrderedDict's tp_traverse and tp_clear (GH-18749)

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

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

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Change by miss-islington : -- nosy: +miss-islington nosy_count: 7.0 -> 8.0 pull_requests: +18110 pull_request: https://github.com/python/cpython/pull/18755 ___ Python tracker

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

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

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Pablo Galindo Salgado added the comment: New changeset 0c2b509f9d1d3a9065bc62c2407e1dc2ed70e9c2 by Pablo Galindo in branch 'master': bpo-39778: Don't traverse weak-reference lists OrderedDict's tp_traverse and tp_clear (GH-18749)

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

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

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Pablo Galindo Salgado added the comment: > I agree the tp_traverse docs should point out that weakref lists are special > this way, but I think the problem is unique to them - can't think of another > case where a container points to an object it doesn't own a reference to. Agreed, I was

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Tim Peters added the comment: After some thought, I'm sure the diagnosis is correct: the weakref list must be made invisible to gc. That is, simply don't traverse it at all. The crash is exactly what's expected from traversing objects a container doesn't own references to. I agree the

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Change by Pablo Galindo Salgado : -- assignee: -> pablogsal ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Pablo Galindo Salgado added the comment: I concur with Antoine and Tim. The GC already has the machinery to deal with weak references in the correct way (even more after recent bugfixes regarding callbacks). Traversing the weak reference list in incorrect because the object does not "own"

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Change by STINNER Victor : -- nosy: +vstinner ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Antoine Pitrou added the comment: Yes, I don't think other weakref-supporting objects traverse the weakreflist in their tp_traverse. -- nosy: +pitrou ___ Python tracker ___

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Tim Peters added the comment: I'm suspecting that maybe we shouldn't be doing Py_VISIT(od->od_weakreflist); at all - best I can tell from a quick (non-exhaustive!) scan, the objects in the weakref list aren't incref'ed to begin with. And even if they were, that line would only be

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Change by Raymond Hettinger : -- nosy: +eric.snow ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Tim Peters added the comment: Thanks for the succinct example! Although you don't need the print() ;-) I can confirm this crashes the same way under a current master build (albeit on Windows 64-bit). gc is calculating how many references in the current generation are accounted for by

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

Change by Leonard Lausen : -- type: -> crash ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue39778] collections.OrderedDict and weakref.ref raises "refcount is too small" assertion

New submission from Leonard Lausen : Below sample program, will raise "Modules/gcmodule.c:110: gc_decref: Assertion "gc_get_refs(g) > 0" failed: refcount is too small" on Python 3.8.2 debug build. On 3.7.6 debug build, "Modules/gcmodule.c:277: visit_decref: Assertion `_PyGCHead_REFS(gc) != 0'