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

[Raymond Hettinger]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Pablo Galindo Salgado]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Pablo Galindo Salgado]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[STINNER Victor]

Change by STINNER Victor :


--
nosy:  -vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Raymond Hettinger]

Raymond Hettinger  added the comment:

Also a test should be added to make sure the weakref callbacks are still 
occurring.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Raymond Hettinger]

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
 ):

if (self->weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *) self);

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Pablo Galindo Salgado]

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!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Irit Katriel]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[STINNER Victor]

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);

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[miss-islington]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[miss-islington]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[miss-islington]

Change by miss-islington :


--
pull_requests: +18117
pull_request: https://github.com/python/cpython/pull/18762

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[miss-islington]

Change by miss-islington :


--
pull_requests: +18118
pull_request: https://github.com/python/cpython/pull/18763

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[miss-islington]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Pablo Galindo Salgado]

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)
https://github.com/python/cpython/commit/9ddcb914f9c2debe7c1359b2450cd1573e86b91c


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[miss-islington]

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)
https://github.com/python/cpython/commit/69ded3944c202da972754644c0bbf7f77cc5e8ea


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
pull_requests: +18111
pull_request: https://github.com/python/cpython/pull/18756

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[miss-islington]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
pull_requests: +18109
pull_request: https://github.com/python/cpython/pull/18754

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Pablo Galindo Salgado]

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)
https://github.com/python/cpython/commit/0c2b509f9d1d3a9065bc62c2407e1dc2ed70e9c2


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Pablo Galindo Salgado]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Pablo Galindo Salgado]

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 thinking a small warning or something because this is not the 
first time I see similar errors or users confused about what they should and 
should not track (with this I mention that we should say that "only objects 
that are *owned* should be tracked, and then the weakref warning or something 
similar). I will prepare a PR soon.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Tim Peters]

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 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
assignee:  -> pablogsal

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Pablo Galindo Salgado]

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" the weak references to it, as the weak references can die even 
if the object is alive. Also, as Tim mentions, the traverse will be called on 
the head of the list, in the same way if you do object.__weakref__ you will 
only get the HEAD of the list:

>>> import weakref
>>> class A: ...
>>> a = A()
>>> w1 = weakref.ref(a)
>>> w2 = weakref.ref(a, lambda *args: None) # Use a callback to avoid re-using 
>>> the original weakref
>>> id(w1)
4328697104
>>> id(w2)
4328758864
>>> id(a.__weakref__)
4328697104

I think that this is not very well documented, as there is no pointers on what 
should and should not be traversed in 
https://docs.python.org/3.8/c-api/typeobj.html#c.PyTypeObject.tp_traverse.

I will prepare a PR to the documentation if everybody agrees and another one 
removing the traverse unless someone sees that something else is at play.

--
nosy: +pablogsal

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[STINNER Victor]

Change by STINNER Victor :


--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Antoine Pitrou]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Tim Peters]

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 looking at the head of the list, ignoring all the non-head 
weakrefs after the head.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Raymond Hettinger]

Change by Raymond Hettinger :


--
nosy: +eric.snow

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Tim Peters]

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 intra-generation references, and gc's visit_decref() is getting called 
back by odictobject.c's odict_traverse(), on line

Py_VISIT(od->od_weakreflist);

gc has found a second pointer to od->od_weakreflist, which is more than its 
refcount (1) claims exist.

Python's weakref implementation gives me headaches, so I'm adding Raymond to 
the nosy list under the hope the problem will be obvious to him.

--
components: +Extension Modules -C API
nosy: +rhettinger, tim.peters
stage:  -> needs patch
versions: +Python 3.7, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Leonard Lausen]

Change by Leonard Lausen :


--
type:  -> crash

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Leonard Lausen]

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' failed." is raised.

```
import collections
import gc
import weakref

hooks_dict = collections.OrderedDict()
hooks_dict_ref = weakref.ref(hooks_dict)
gc.collect()

print('Hello world')
```


The complete error message on 3.8.2 debug build is

```
Modules/gcmodule.c:110: gc_decref: Assertion "gc_get_refs(g) > 0" failed: 
refcount is too small
Memory block allocated at (most recent call first):
  File "/home/$USER/test.py", line 6

object  : 
type: weakref
refcount: 1
address : 0x7ff788208a70
Fatal Python error: _PyObject_AssertFailed
Python runtime state: initialized

Current thread 0x7ff789f9c080 (most recent call first):
  File "/home/$USER/test.py", line 7 in 
zsh: abort  PYTHONTRACEMALLOC=1 python ~/test.py
```

--
components: C API
messages: 362846
nosy: leezu
priority: normal
severity: normal
status: open
title: collections.OrderedDict and weakref.ref raises "refcount is too small" 
assertion
versions: Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com