[Neil]
I had to change _PyWeakref_ClearRef() since it was also clearing the weakref list of the trash object.
[Tim]
That was really its *purpose*. If a trash weakref with a callback isn't removed from the referent's list of weakrefs, then the callback will trigger when PyObject_ClearWeakRefs() is invoked on the referent. The purpose of _PyWeakref_ClearRef() was to ensure that the callback never triggers.
[Neil]
But it's okay of the callback triggers, as long as the callback doesn't reference trash.
Indeed, the callback must trigger if a weakref W's referent is in CT (cyclic trash) but W is not in CT (and W has a callback, of course). If W is also in CT, it's not necessary to invoke the callback, and it can be disastrous to invoke it (that was the key to fixing the *last* pile of bugs, which were subtler than this pile). The meaning of "clearing a weakref" has become ambiguous -- so far, to me (and I believe to Fred too) it's always meant both "set wr_object to Py_None" and "remove the weakref from its referent's list of weakrefs". You're sometimes using it to mean only the former. That may be a clever thing to do -- unsure yet.
Now it just sets wr_object to Py_None.
That won't stop the callback from triggering. It also means (see earlier comment) that PyObject_ClearWeakRefs() will never remove the weakref from the list either, although I'm not sure that does real harm.
I'm trying to figure out PyObject_ClearWeakRefs() right now.
For each weakref W in the object's list of weakrefs, it clears W (in the dual sense above), and invokes W's callback (if any). If W->wr_object is Py_None, the callback is still invoked, but W isn't removed from its referent's list of weakrefs then. I don't think Fred intended that wr_object could be None at the same time wr_callback is non-NULL (Fred?), so the consequences of doing "half a clear" are only half clear <wink>.
I also made some serious simplifications to gcmodule by just treating trash weakref objects with callbacks the same as objects with __del__ methods (i.e. move them to the finalizers list and then do the transitive closure of that set).
Does that mean they can end up in gc.garbage too? If so, I don't think that's sellable.
I think so. That can be easily changed though. What we can't do is invoke those callbacks.
Some callbacks, yes, but others must be invoked. For example, in any sensible use of a weak dictionary, it's vital that callbacks on weakrefs to cyclic trash get invoked; the weakrefs themselves aren't in CT in such cases (the weak dict holds strong references to the weakrefs, so for as long as the weak dict isn't trash, the weakrefs aren't trash either). It would be great if you've found a simpler way to get this all working. The most sensitive tests for this stuff are test_gc (with the diffs from my patch on SF), test_weakref, and test_descr. But they're not verifying that things don't end up in gc.garbage, so that's something to look out for.