Repository navigation
weakref subclass segfault #47350
Description
| BPO | 3100 |
|---|---|
| Nosy | @amauryfa |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2008-11-12.01:18:58.776>
created_at = <Date 2008-06-13.03:19:58.465>
labels = ['interpreter-core', 'type-crash']
title = 'weakref subclass segfault'
updated_at = <Date 2016-09-07.00:22:35.604>
user = 'https://bugs.python.org/Rhamphoryncus'bugs.python.org fields:
activity = <Date 2016-09-07.00:22:35.604>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2008-11-12.01:18:58.776>
closer = 'amaury.forgeotdarc'
components = ['Interpreter Core']
creation = <Date 2008-06-13.03:19:58.465>
creator = 'Rhamphoryncus'
dependencies = []
files = ['10613', '10614', '10625']
hgrepos = []
issue_num = 3100
keywords = ['patch']
message_count = 19.0
messages = ['68120', '68121', '68122', '68123', '68124', '68126', '68127', '68128', '68134', '68143', '68146', '68173', '68175', '68187', '68196', '68290', '68294', '75772', '274690']
nosy_count = 5.0
nosy_names = ['amaury.forgeotdarc', 'Rhamphoryncus', 'roudkerk', 'jnoller', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue3100'
versions = ['Python 2.6', 'Python 2.5', 'Python 3.0']
Activity
Rhamphoryncus commentedon Jun 13, 2008
$ ./python Python 2.6a3+ (unknown, Jun 12 2008, 20:10:55) [GCC 4.2.3 (Debian 4.2.3-1)] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> import multiprocessing.reduction [55604 refs] >>> [55604 refs] Segmentation fault (core dumped)Rhamphoryncus commentedon Jun 13, 2008
op is a KeyedRef instance. The instance being cleared from the module
is the multiprocessing.util._afterfork_registry.
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb7d626b0 (LWP 2287)]
0x0809a131 in _Py_ForgetReference (op=0xb7a9a814) at Objects/object.c:2022
2022 if (op == &refchain ||
(gdb) bt
#0 0x0809a131 in _Py_ForgetReference (op=0xb7a9a814) at
Objects/object.c:2022
#1 0x0809a1a0 in _Py_Dealloc (op=0xb7a9a814) at Objects/object.c:2043
#2 0x080b436e in tupledealloc (op=0xb79ad1f4) at Objects/tupleobject.c:169
#3 0x0809a1ab in _Py_Dealloc (op=0xb79ad1f4) at Objects/object.c:2044
#4 0x08065bdf in PyObject_CallFunctionObjArgs (callable=0xb79baa84)
at Objects/abstract.c:2716
#5 0x080cabc4 in handle_callback (ref=0xb7a9a814, callback=0xb79baa84)
at Objects/weakrefobject.c:864
#6 0x080cad6e in PyObject_ClearWeakRefs (object=0xb79bd624)
at Objects/weakrefobject.c:910
#7 0x08168971 in func_dealloc (op=0xb79bd624) at Objects/funcobject.c:453
#8 0x0809a1ab in _Py_Dealloc (op=0xb79bd624) at Objects/object.c:2044
#9 0x080b436e in tupledealloc (op=0xb79a65f4) at Objects/tupleobject.c:169
#10 0x0809a1ab in _Py_Dealloc (op=0xb79a65f4) at Objects/object.c:2044
#11 0x080b7c26 in clear_slots (type=0x82af4e4, self=0xb7a9a814)
at Objects/typeobject.c:821
#12 0x080b806e in subtype_dealloc (self=0xb7a9a814) at
Objects/typeobject.c:950
#13 0x0809a1ab in _Py_Dealloc (op=0xb7a9a814) at Objects/object.c:2044
#14 0x080915b2 in dict_dealloc (mp=0xb79b9674) at Objects/dictobject.c:907
#15 0x0809a1ab in _Py_Dealloc (op=0xb79b9674) at Objects/object.c:2044
#16 0x080915b2 in dict_dealloc (mp=0xb79b9494) at Objects/dictobject.c:907
#17 0x0809a1ab in _Py_Dealloc (op=0xb79b9494) at Objects/object.c:2044
#18 0x08068720 in instance_dealloc (inst=0xb79b6edc)
at Objects/classobject.c:668
#19 0x0809a1ab in _Py_Dealloc (op=0xb79b6edc) at Objects/object.c:2044
#20 0x08090517 in insertdict (mp=0xb79a5b74, key=0xb7a9ae38,
hash=-1896994012,
value=0x81bdd6c) at Objects/dictobject.c:455
#21 0x08090da6 in PyDict_SetItem (op=0xb79a5b74, key=0xb7a9ae38,
value=0x81bdd6c) at Objects/dictobject.c:697
#22 0x08095ad3 in _PyModule_Clear (m=0xb7a88334) at
Objects/moduleobject.c:125
#23 0x08111443 in PyImport_Cleanup () at Python/import.c:479
#24 0x08120cb3 in Py_Finalize () at Python/pythonrun.c:430
#25 0x0805b618 in Py_Main (argc=1, argv=0xbfbaf434) at Modules/main.c:623
#26 0x0805a2e6 in main (argc=0, argv=0x0) at ./Modules/python.c:23
Rhamphoryncus commentedon Jun 13, 2008
More specific test case.
[-]segfault after loading multiprocessing.reduction[/-][+]segfault from multiprocessing.util.register_after_fork[/+]Rhamphoryncus commentedon Jun 13, 2008
Very specific test case, eliminating multiprocessing entirely. It may
be an interaction between having the watched obj as its own key in the
WeakValueDictionary and the order in which the two modules are cleared.
[-]segfault from multiprocessing.util.register_after_fork[/-][+]segfault with WeakValueDictionary and module clearing[/+]Rhamphoryncus commentedon Jun 13, 2008
Specific enough yet? Seems the WeakValueDictionary and the module
clearing aren't necessary.
A subclass of weakref is created. The target of this weakref is added
as an attribute of the weakref. So long as a callback is present there
will be a crash on shutdown. However, if the callback prints the
attribute, you get a crash right then. The weakref claims to be dead,
which shouldn't be possible when the weakref's attributes have a strong
reference to the target.
[-]segfault with WeakValueDictionary and module clearing[/-][+]weakref subclass segfault[/+]Rhamphoryncus commentedon Jun 13, 2008
MyRef is dead and should be unreachable)
demise
calling the callback does enough damage to explain the segfault without
accessing MyRef's dict.
As I understand, a deleted weakref doesn't call its callback. However,
subtype_dealloc doesn't call the base type's tp_dealloc until *after* it
does everything else. Does it need a special case for weakrefs, or
maybe a tp_predealloc slot?
Rhamphoryncus commentedon Jun 13, 2008
Ahh, I missed a detail: when the callback is called the weakref has a
refcount of 0, which is ICNREFed to 1 when getting put in the args, then
drops down to 0 again when the args are DECREFed (causing it to get
_Py_ForgetReference to be called a second time, which segfaults.)
Rhamphoryncus commentedon Jun 13, 2008
Patch to add extra sanity checks to Py_INCREF (only if Py_DEBUG is set).
If the refcount is 0 or negative if calls Py_FatalError. This should
catch revival bugs such as this one a little more clearly.
The patch also adds a little more checking to _Py_ForgetReference, so
it's more likely to print an error rather than segfaulting.
amauryfa commentedon Jun 13, 2008
All the versions I tried (2.4, 2.5, 2.6, 3.0) crash with the given script
amauryfa commentedon Jun 13, 2008
It seems enough to simply skip deleted weakrefs in PyObject_ClearWeakRefs.
Here is a tentative patch.
amauryfa commentedon Jun 13, 2008
A new version of the patch, which tests the case of multiple weakrefs on
the same object, that get deleted together:
tp_dealloc of one weakref calls tp_dealloc of the second weakref, which
calls tp_dealloc of the referenced object. Since the weakrefs are being
deleted, no callback should fire.
Rhamphoryncus commentedon Jun 13, 2008
Well, my attempt at a patch didn't work, and yours does, so I guess I
have to support yours. ;)
Can you review my python-incref-from-zero patch? It verifies the
invariant that you need, that once an object hits a refcount of 0 it
won't get raised again. (The possibility of __del__ makes me worry, but
it *looks* okay.)
gcmodule.c has an inline copy of handle_callbacks. Is it possible a
collection could have the same problem we're fixing here?
Minor nit: you're asserting cbcalled, but you're not using the generic
callback, so it's meaningless.
Rhamphoryncus commentedon Jun 13, 2008
Ahh, it seems gcmodule already considers the weakref to be reachable
when it calls the callbacks, so it shouldn't be a problem.
amauryfa commentedon Jun 13, 2008
Rhamphoryncus commentedon Jun 14, 2008
Another minor nit: "if(current->ob_refcnt > 0)" should have a space
after the "if". Otherwise it's looking good.
amauryfa commentedon Jun 16, 2008
Committed r64309, with the suggested whitespace change.
Will backport.
Concerning the INCREF patch: would a simple "assert(refcnt>0)" be enough?
Rhamphoryncus commentedon Jun 16, 2008
Unfortunately, Py_INCREF is sometimes used in an expression (followed by
a comma). I wouldn't expect an assert to be valid there (and I'd want
to check ISO C to make sure it's portable, not just accepted by GCC).
I'd like if Py_INCREF and friends were made into static inline
functions, which *should* have identical performance (at least on GCC),
but that's a more significant change.
amauryfa commentedon Nov 12, 2008
This was fixed 4 months ago with r64309 (trunk, 2.6) and r64310 (2.5).
python-dev commentedon Sep 7, 2016
New changeset 3d8109fe6d82 by Gregory P. Smith in branch 'default':
Correct a comment in the test referencing the wrong issue number (bpo-3100
https://hg.python.org/cpython/rev/3d8109fe6d82