[issue16602] weakref can return an object with 0 refcount
Amaury Forgeot d'Arc added the comment: PyWeakref_GET_OBJECT is also potentially dangerous: since the refcount is not incremented, it's very possible that the GC collects it. The only safe operation after PyWeakref_GET_OBJECT is to Py_XINCREF the result. Should we provide a PyWeakRef_LockObject()? -- nosy: +amaury.forgeotdarc ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Antoine Pitrou added the comment: The only safe operation after PyWeakref_GET_OBJECT is to Py_XINCREF the result. Should we provide a PyWeakRef_LockObject()? There's already a warning in the docs about that. I don't think an additional function is useful here. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Roundup Robot added the comment: New changeset 0748c22b37e5 by Antoine Pitrou in branch '3.2': Issue #16602: When a weakref's target was part of a long deallocation chain, the object could remain reachable through its weakref even though its refcount had dropped to zero. http://hg.python.org/cpython/rev/0748c22b37e5 New changeset 259c1636c884 by Antoine Pitrou in branch '3.3': Issue #16602: When a weakref's target was part of a long deallocation chain, the object could remain reachable through its weakref even though its refcount had dropped to zero. http://hg.python.org/cpython/rev/259c1636c884 New changeset 17e5acad302e by Antoine Pitrou in branch 'default': Issue #16602: When a weakref's target was part of a long deallocation chain, the object could remain reachable through its weakref even though its refcount had dropped to zero. http://hg.python.org/cpython/rev/17e5acad302e -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Roundup Robot added the comment: New changeset 7e771f0363e2 by Antoine Pitrou in branch '2.7': Issue #16602: When a weakref's target was part of a long deallocation chain, the object could remain reachable through its weakref even though its refcount had dropped to zero. http://hg.python.org/cpython/rev/7e771f0363e2 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Antoine Pitrou added the comment: Thank you very much for reporting this issue. I believe it is now fixed. -- resolution: - fixed stage: needs patch - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Eugene Toder added the comment: Thank you, Antoine. This change has one effect that's worth highlighting in NEWS at least -- the PyWeakref_GET_OBJECT() macro now evaluates its argument twice. This can break existing code where the argument has side-effects, e.g. o = PyWeakref_GET_OBJECT(p++). I found one such case in our code base, but I don't know how common this is. So this is something to watch out for when upgrading. I don't think there's a way to write PyWeakref_GET_OBJECT() in standard C90 without double evaluation. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Antoine Pitrou added the comment: I don't think there's a way to write PyWeakref_GET_OBJECT() in standard C90 without double evaluation. Agreed. We generally don't document whether our macros are compatible with side effects in parameters, and I think we'd like to keep it that way. People should simply avoid doing this kind of thing, as it's knowingly fragile, and trivial to avoid anyway. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Eugene Toder added the comment: People should simply avoid doing this kind of thing, as it's knowingly fragile, and trivial to avoid anyway. Is this documented in the C API guide, or somewhere else? In any case, notifying people so they can quickly check their code seems much nicer than having them figure this out the hard way. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Antoine Pitrou added the comment: People should simply avoid doing this kind of thing, as it's knowingly fragile, and trivial to avoid anyway. Is this documented in the C API guide, or somewhere else? I don't think so, but it's common C wisdom that you shouldn't pass arguments which have side effects to a macro, except if you are sure the macro allows it. In any case, notifying people so they can quickly check their code seems much nicer than having them figure this out the hard way. I see it as a double-edged sword: if we start adding a warning for this macro, people will expect us to do it for every other macro, which we aren't doing right now. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Antoine Pitrou added the comment: In any case, notifying people so they can quickly check their code seems much nicer than having them figure this out the hard way. I see it as a double-edged sword: if we start adding a warning for this macro, people will expect us to do it for every other macro, which we aren't doing right now. And also, as this issue shows, we may have to change it in a bugfix release, which means people shouldn't trust the fact that there's no such warning, anyway. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Changes by Andrew Svetlov andrew.svet...@gmail.com: -- nosy: +asvetlov ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Changes by Jesús Cea Avión j...@jcea.es: -- nosy: +jcea ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Antoine Pitrou added the comment: The easy fix would probably be to return Py_None when the weakref'ed object has a zero refcount. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Eugene Toder added the comment: Agreed. That's what I've put in my code as a work around. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Changes by Daniel Urban urban.dani...@gmail.com: -- nosy: +daniel.urban ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
New submission from Eugene Toder:
An interaction between weakrefs and trashcan can cause weakref to return the
object it's pointing to after object's refcount is already 0. Given that the
object is usually increfed and decrefed, this leads to double dealloc and
crashing or hanging.
Tested 2.6.6, 2.7.2 and 3.3.0 on win7.
In more details. The documentation for Py_TRASHCAN_SAFE_BEGIN tells to put it
right after PyObject_GC_UnTrack. This means that PyObject_ClearWeakRefs goes
after it, and, for example, in subtype_dealloc of Objects/typeobject.c this is
indeed the case. This means that if we get into a long chain of deallocations
and the trashcan kicks in, we get an object with 0 refcount and without cleared
weakrefs lying in trash_delete_later until we go PyTrash_UNWIND_LEVEL levels
up. During that time we can execute arbitrary python code, so all we need is
some code with an access to the weakref to dereference it.
The current recommendation of clearing weakrefs before clearing attributes
makes this less likely to happen, but it's still easy enough:
import weakref
class C:
def __init__(self, parent):
if not parent:
return
wself = weakref.ref(self)
def cb(wparent):
o = wself()
self.wparent = weakref.ref(parent, cb)
d = weakref.WeakKeyDictionary()
root = c = C(None)
for n in range(100):
d[c] = c = C(c)
print('deleting')
del root
print('done')
In this case weakref callback in WeakKeyDictionary deletes the reference to the
next object, causing the next level of destruction, until trashcan kicks in.
Trashcan delays clearing of weakrefs, allowing the second weakref's (wparent)
callback to see the dead object via wself that it captured.
Attached is a similar example with less weakrefs using __del__.
--
components: Interpreter Core
files: wr2.py
messages: 176874
nosy: eltoder, pitrou
priority: normal
severity: normal
status: open
title: weakref can return an object with 0 refcount
type: crash
versions: Python 2.6, Python 2.7, Python 3.1, Python 3.2, Python 3.3
Added file: http://bugs.python.org/file28204/wr2.py
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16602
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16602] weakref can return an object with 0 refcount
Christian Heimes added the comment: Thank you very much for bringing the issue to our attention. I've removed 2.6 and 3.1 because they are in security fix mode and this issue poses no security threat. -- nosy: +christian.heimes stage: - needs patch versions: +Python 3.4 -Python 2.6, Python 3.1 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16602 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
