Antoine Pitrou added the comment:
finalize4.patch repairs the comment typos, adds a new comment, and
removes the unused `old` argument. I think the code is ready to ship
with this.
Thanks! So do I.
--
___
Python tracker rep...@bugs.python.org
Larry Hastings added the comment:
I'm totally on board with you guys checking this in for 3.4.1.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21435
___
Changes by Tim Peters t...@python.org:
--
assignee: - tim.peters
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21435
___
___
Python-bugs-list
Antoine Pitrou added the comment:
Tim's analysis is spot on, and finalize3.patch looks good to me (there's some
strange commenting style there - do the carets ^ mean something special?).
Still, I hope we can find a way to write a test case.
--
___
Antoine Pitrou added the comment:
The new info: its (the list object's) gc_refs also changed from
GC_TENTATIVELY_UNREACHABLE to GC_UNTRACKED, That the object became
untracked is wholly consistent with that its gc_next became NULL but
not its gc_prev.
Could that be the trashcan mecanism?
Antoine Pitrou added the comment:
Larry, once this patch is finalized, I think it is a good candidate for 3.4.1.
--
nosy: +larry
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21435
___
Tim Peters added the comment:
Antoine, the carets are a symptom of my flu. My fingers jump on the keyboard
from sneezing and coughing, and my eyes are watery so I don't see the typos.
But I believe that can be fixed ;-)
I doubt the trashcan cruft is responsible, for several reasons: I
Tim Peters added the comment:
OK! This has nothing to do with the trashcan mechanism.
The list object whose gc_next gets stomped on is not itself in a cycle. It's
an empty list, and just happens to be a value in a dict, which in turn is a
value in another dict. Its refcount falls to 0 as
Tim Peters added the comment:
finalize4.patch repairs the comment typos, adds a new comment, and removes the
unused `old` argument. I think the code is ready to ship with this.
I still don't have a reasonably simple fails-before-works-after test case.
But that doesn't bother me much,
Tim Peters added the comment:
xxx.py provokes a closely related death on my box, in a debug build (where
0xdbdbdbdb happened to be an invalid memory address). There are no imports so
asyncio is definitely off the hook ;-)
The code is senseless, and changing just about anything makes the
STINNER Victor added the comment:
Similar(?) issue #20526:
Modules/gcmodule.c:379: visit_decref: Assertion `((gc)-gc.gc_refs (1)) !=
0' failed.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21435
Tim Peters added the comment:
A bit more info: recall that, when deleting a type object (for class B), the
previous (list) object's gc_next got overwritten with NULL. The new info: its
(the list object's) gc_refs also changed from GC_TENTATIVELY_UNREACHABLE to
GC_UNTRACKED, That the
Tim Peters added the comment:
Attaching a marginally cleaner version of the patch, with comments.
I think it's clear this addresses _some_ real fatal problems, but they're rare:
for a problem to show up, it has to be the case that finalize() reduces the
refcount of the current object to 0,
Tim Peters added the comment:
Oh, fudge. There are other failure modes in the test suite after I took out
the seemingly redundant incref/decref pair. Adding it back in finalize3.patch.
--
Added file: http://bugs.python.org/file35167/finalize3.patch
Guido van Rossum added the comment:
Thanks Tim! I'm not sure who should review the patch, but it's not me. :-) I've
verified that the patch stops the example program from segfaulting on OSX, and
I assume you've verified it on Windows -- that's good enough for me. We don't
see this happening
Tim Peters added the comment:
Guido, it's best if Antoine reviewed it - he wrote the relevant PEP, and the
code I'm changing. I'm sure he'll agree it's a necessary change.
About the rarity of the bug, not only do the refcounts and cyclic structure
have to be just right, we also need exactly
New submission from Peter Inglesby:
The following code causes a segfault when run under Python3.4+ on OSX10.9.
# segfaulter.py
import asyncio
class A:
pass
class B:
def __init__(self, future):
self.future = future
def __del__(self):
self.a = None
Tim Golden added the comment:
I can confirm that this crashes on Windows as well. Failure actually occurs in
gcmodule.c:finalize_garbage. Adding Guido as it looks as it's tied to asyncio.
--
nosy: +Guido.van.Rossum, tim.golden
___
Python tracker
Guido van Rossum added the comment:
Confirmed too (OSX 10.9 again, CPython 3.4 or 3.5; but it doesn't crash with
CPython 3.3). There is no C code in asyncio, so I'm not sure how asyncio can be
directly responsible for this crash. Probably some of the GC improvements have
an edge case that is
Antoine Pitrou added the comment:
Here is the gdb backtrace. The `gc` variable is equal to NULL.
0x0043d254 in finalize_garbage (collectable=0x7fffdc40,
old=0x8fdae0 generations+64)
at Modules/gcmodule.c:788
788 if (!_PyGCHead_FINALIZED(gc)
(gdb) bt
#0
Guido van Rossum added the comment:
So should we just add
if (!gc)
break;
at the top of the for-loop body? That prevents the crash for me. But why is
it needed?
--
___
Python tracker rep...@bugs.python.org
Tim Peters added the comment:
Guido, that's no good. The outer loop is traversing a doubly-linked circular
list, and it should be flatly impossible for gc to ever be NULL - the list
structure is insanely damaged if any gc_next or gc_prev field reachable from it
is NULL (gc always comes from
Tim Peters added the comment:
Thought question: suppose finalize_garbage() is called with a collectable list
all of whose members are in fact going to be destroyed?
I don't see how the loop iteration logic could reliably work then. For
concreteness, suppose there's only object - A - in the
Tim Peters added the comment:
Noting that in a Windows debug build, at death gc is 0xdbdbdbdb, so we are in
reading up free'd memory (0xdb is pymalloc's dead byte marker).
--
___
Python tracker rep...@bugs.python.org
Tim Peters added the comment:
Also noting that the loop looks funny because it appears never to process the
first element in the input list (unless the input list contains only one
element). That is, the loop starts by processing the second element in the
list, and exits as soon as gc makes
Tim Peters added the comment:
Ah, fudge - I think I forgot that these circular lists also have dummy heads,
in which case it's A Good Thing the first element is skipped, and it should
be impossible for one to become physically empty (the dummy header should
always survive intact). Never
Tim Peters added the comment:
Sorry for the earlier noise. I'm fighting a flu and my head is mush :-(
Anyway, this doesn't look obvious. We get to this point:
if (Py_REFCNT(op) == 1) {
/* op will be destroyed */
gc = gc-gc.gc_prev;
}
27 matches
Mail list logo
