So the cause of the problem seems to be this PyPy behavior, which differs from behavior we can observer on CPython: the object a weakref refers to may be collected and then, before the weakref's callback is called, some more Python code gets a chance to run. This means that the callback which cleans up the `data` dict might not run until after some code uses the `get` method. This confuses `get` because it thinks only an internal bookkeeping error can result in a key being present in `data` but the value of the weakref being `None`. On PyPy this is actually a legitimate case, since it's only a matter of the callback not having run /yet/.
This hypothesis is testable, actually. Adding any observable side-effect to the callback will prove that the callback is not being called in time - or that it is. And some experimentation seems to bear this out. A print statement at the top of `remove` doesn't show up until *after* the first Axiom test which fails this way - that is, the weakref callback has not been called until after the test has failed. So I am probably convinced this is the right direction to be going now. That said: 1) The `assert` in the `cache` method seems like it may run afoul of this very issue. The key may be present in `data` after the associated object has been collected. It seems this check needs to include inspection of the value of `self.data[k]()`. It is not an error to re-use a cache key if a weakref remains in `data` for the key, but the value of the weakref is `None`. 2) The code could probably use a few more comments. This stuff is not obvious. ;) 3) I think I convinced myself that the new try/except KeyError inside `remove` is also necessary - in case the weakref callback runs after the weakref has been discarded by the new `del` in `get` but PyPy runs its callback anyway (because the garbage collector hasn't collected the weakref object yet). Again, a comment or two explaining this would help the code a bunch, I think. 4) If you don't mind, `w`, `k`, and `f` are not ideal argument names for `createCacheRemoveCallback` (not to mention undocumented). Changes elsewhere in Axiom seem fine. Thanks! Feel free to merge after addressing the above. I'm also happy to take another look if you like. -- https://code.launchpad.net/~divmod-dev/divmod.org/829879-pypy-finalization-semantics/+merge/173053 Your team Divmod-dev is subscribed to branch lp:divmod.org. -- Mailing list: https://launchpad.net/~divmod-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~divmod-dev More help : https://help.launchpad.net/ListHelp
