#14159: Don't install callbacks on values of TripleDict, MonoDict
------------------------------+---------------------------------------------
Reporter: nbruin | Owner: tbd
Type: defect | Status: needs_work
Priority: major | Milestone: sage-5.8
Component: memleak | Resolution:
Keywords: | Work issues: Make part the coverage script
happy
Report Upstream: N/A | Reviewers: Nils Bruin
Authors: Simon King | Merged in:
Dependencies: #13387 | Stopgaps:
------------------------------+---------------------------------------------
Comment (by nbruin):
Replying to [comment:22 SimonKing]:
(just saw your new post. Definitely an option. If you see merit I'm fine
with that. Below response I wrote to your earlier message)
> I thought you diagnosed that this was the cause of one problem we saw?
No, what I'm pretty sure we saw at some point was GC ''happening'' during
dictionary value setting. While looking at the code I noticed this
callback without proof that it's safe.
> 1. Some parent X, Y give rise to a homset H, in a category C.
> 2. H and X get deleted, but the callbacks are not performed yet. A new
parent X2 is created, and by mere coincidence we have id(X)==id(X2).
> 3. The homset H2 with respect to X2, Y and C is created. It thus
overrides H in the cache.
> 4. Finally, the callback of H is performed, erasing H2 from the cache.
Boom
Yes, that's exactly the scenario I envisaged. I don't have a proof that
this is theoretically possible, though.
The weakref to X is referenced by this dictionary that is still in use, so
it's not garbage. I think that means the callback to X is guaranteed to
happen before deallocation of X (and hence before the creation of X2).
Furthermore X can only be found to be garbage if H is found to be garbage
(because of the strong `domain` reference), so the callback on H has to
happen in the same GC as the deallocation of X. So as long as we're not
creating parents during callbacks, I think that also means X2 cannot be
created in the same memory location as X while the callback on H hasn't
happened yet.
So we have a choice:
- make a very involved argument, carefully reasoning about the specifics
of python's GC system (for which we don't actually have a formal axiom
system)
- be on the safe side and just put in a cheap test.
> Do you prefer that I erase this statement?
perhaps "race condition we couldn't quite rule out"
> Thinking of it: Suppose we are now considering the callback of X, which,
in the above scenario, has not been called prior to the creation of X2.
Would it not delete the new item, too? So, shouldn't we add a test in
`TripleDictEraser.__call__(self, r)`, so that an item is deleted only if
one of the references from the keys of this item is identical with r?
No, in order to set the value, the caller must be holding strong
references to the keys. As far as I know, weakref callback and
deallocation only get separated inside GC. So as long as we're not calling
set in callbacks, I think we're prevented from it (note that for H we
needed, in addition, that H holds a reference to X AND that key triples do
not get reassigned.)
> I wouldn't like to put this into `TripleDict`. After all, it is
impossible to prevent all thinkable typing mistakes.
Well, this is one where you have to type the same data ''twice''. But I
agree: We have only one use case. Should we get more I think it's worth
reconsidering.
> Additional idea: We could create TripleDict with an optional parameter
weak_values. If this parameter is True, then the set/get methods would
automatically use a weak reference on the value, with the correct key for
the callback. Hence, we do not need to exploit implementation details when
we apply a triple dict with weak values in sage.categories.homset
Well, conceptually that's a different concept. Whether we want to
implement that in the same structure (advantage: code reuse, disadvantage:
increased complexity, possibly efficiency loss) is an engineering call.
Again, for one use case I think it's not worth it. Should we find the need
more often for such "fully weak associations" (we found the coercion
system has tables that are a bit like this) we can reconsider.
At the moment I'm getting a feeling we're entering the realm of
overengineering.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/14159#comment:26>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
