[Python-Dev] Re: weakref callback vs gc vs threads

[Tim Peters]

[Neil, on "half clear"ing]
> Crap.  That's got to be the problem that I've been bashing my head
> against for the past few hours.

I can believe that.  It was attractive because otherwise (as my patch
does) the guts of PyObject_ClearWeakRefs() get duplicated inline
inside gc, spread across distinct passes.

> Okay, how about we first clearly specify what needs to happen.  Afterwards
> we can figure how to do it while retaining binary compatibility. :-(

It's a good plan.

> Non-trash weakrefs to trash objects MUST have their callbacks
> invoked.

Yes.

>  Trash weakrefs MUST NOT have their callbacks invoked.

That's actually a kind of optimization.  It's never *necessary* to
invoke a callback from a trash weakref, and it *may* be catastrophic
to do so.  So the cheapest/easiest route is to avoid analyzing whether
this case is safe, and never do it.  IOW, "MUST NOT" is a correct
approach to this case, but looser approaches could suffice.

> Any weakref to a trash object MUST NOT reveal the trash (i.e. via the
> wr_object pointer).

Yes.

Other subtleties follow from those.  Chief example:  if you decide not
to invoke a callback in a trash weakref to a trash object, that can't
be accomplished correctly by decref'ing wr_callback then setting
wr_callback to NULL.  The problem is that wr_callback may itself be
weakly referenced by a different weakref with a different callback, so
decref'ing wr_callback may cause that other callback to run right
away, and that may not be safe.  So if setting wr_calback to NULL is
the strategy for preventing wr_callback from running, it can't be
carried out safely until after all weakrefs to trash have been
cleared.  That's why dealing with trash weakrefs required two passes
when fixing the last pile of bugs.

There are also subtleties about what "trash" means, exactly, here.  gc
has been ignoring the worst of these.  For example, if the referent of
a non-trash weakref is in the unreachable set, but the referent is
*also* in a cycle and has a __del__ method, the referent may well end
up in gc.garbage.  It's arguably wrong to trigger the callback in that
case.  It's also arguable that the user deserves whatever they get in
that case <0.7 wink>.

All that said, the only problem I have with the patch I posted is that
the abuse of a weakref's `hash` member breaks weak-key (but not
weak-value) dictionaries.  That doesn't cause any tests to fail, but
isn't acceptable (weak-key dicts don't work as intended).  Since I'm
not convinced there's a squeaky-clean approach just waiting to be
found here, I think I'll go back to repairing that glitch now.