$ ./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)

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

More specific test case.

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.

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.

1. MyRef is released from the module as part of shutdown
2. MyRef's subtype_dealloc DECREFs its dictptr (not clearing it, as
MyRef is dead and should be unreachable)
3. the dict DECREFs the Dummy (MyRef's target)
4. Dummy's subtype_dealloc calls PyObject_ClearWeakRefs to notify of its
demise
5. the callback is called, with the dead MyRef as an argument
6. If MyRef's dict is accessed a segfault occurs.  Presumably just
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?

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

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.

All the versions I tried (2.4, 2.5, 2.6, 3.0) crash with the given script

It seems enough to simply skip deleted weakrefs in PyObject_ClearWeakRefs.
Here is a tentative patch.

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.

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.

Ahh, it seems gcmodule already considers the weakref to be reachable
when it calls the callbacks, so it shouldn't be a problem.

> you're asserting cbcalled, but you're not using the generic callback, 
> so it's meaningless.
The new patch corrects this

Another minor nit: "if(current->ob_refcnt > 0)" should have a space
after the "if".  Otherwise it's looking good.

Committed r64309, with the suggested whitespace change.
Will backport.

Concerning the INCREF patch: would a simple "assert(refcnt>0)" be enough?

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.

This was fixed 4 months ago with r64309 (trunk, 2.6) and r64310 (2.5).

New changeset 3d8109fe6d82 by Gregory P. Smith in branch 'default':
Correct a comment in the test referencing the wrong issue number (issue3100
https://hg.python.org/cpython/rev/3d8109fe6d82