# HG changeset patch
# User Simon King
# Date 1361793329 -3600
# Node ID 4527023e5d315683ca72d45269bd991f9ab8fc7a
# Parent 2be910c1a975f7985e51faab2687d991c873ac0c
#14159: A safe callback for weak values of a TripleDict
diff --git a/sage/categories/homset.py b/sage/categories/homset.py
--- a/sage/categories/homset.py
+++ b/sage/categories/homset.py
@@ -76,8 +76,9 @@
# introduced in trac ticket #715
from weakref import KeyedRef
-from sage.structure.coerce_dict import TripleDict
+from sage.structure.coerce_dict import TripleDict, TripleDictEraserOnValue
_cache = TripleDict(53)
+_Callback = TripleDictEraserOnValue(_cache)
def Hom(X, Y, category=None):
"""
@@ -201,6 +202,26 @@
sage: Hom(PA,PJ, Rngs())
Set of Morphisms from to in Category of rngs
+ By :trac:`14159`, the cache of homsets uses a weak reference with
+ :class:`~sage.structure.coerce_dict.TripleDictEraserOnValue` as callback.
+ This avoids certain rare race conditions in invoking Python's cyclic
+ garbage collector, but still fixes a memory leak::
+
+ sage: R=[ZZ.quo(3^n) for n in [1..50]]
+ sage: C = type(Hom(R[0],R[0]))
+ sage: _ = gc.collect()
+ sage: l = len([x for x in gc.get_objects() if isinstance(x,C)])
+ sage: def test(R):
+ ... for i in range(len(R)):
+ ... for j in range(i,len(R)):
+ ... H=Hom(R[j],R[i])
+ sage: test(R)
+ sage: _ = gc.collect()
+ sage: l == len([x for x in gc.get_objects() if isinstance(x,C)])
+ True
+ sage: isinstance(Hom(R[10],R[10]), C)
+ True
+
.. TODO::
design decision: how much of the homset comes from the
@@ -220,9 +241,11 @@
except KeyError:
H = None
if H is not None:
- # Are domain or codomain breaking the unique parent condition?
- if H.domain() is X and H.codomain() is Y:
- return H
+ # Note H has just been found using the identities of X and Y as
+ # keys, so if the domain and codomain of H do not match by identity
+ # something went wrong and we want to know about it.
+ assert H.domain() is X and H.codomain() is Y
+ return H
try:
# Apparently X._Hom_ is supposed to be cached
@@ -248,9 +271,9 @@
except KeyError:
H = None
if H is not None:
- # Are domain or codomain breaking the unique parent condition?
- if H.domain() is X and H.codomain() is Y:
- return H
+ # We double check domain and codomain are expected (see above)
+ assert H.domain() is X and H.codomain() is Y
+ return H
# coercing would be incredibly annoying, since the domain and codomain
# are totally different objects
@@ -262,8 +285,12 @@
# of Homset in rings, schemes, ...
H = category.hom_category().parent_class(X, Y, category = category)
- ##_cache[key] = weakref.ref(H)
- _cache[key] = KeyedRef(H, _cache.eraser, (id(X),id(Y),id(category)))
+ # Since H is holding strong references to X,Y,category in the form of
+ # H.domain(), H.codomain(), H.homset_category(), it is OK to just store a
+ # weak reference to the homset: Even though X,Y,category are only weakly
+ # referenced by _cache, they can't be garbage unless H is as well. The
+ # callback below is safe---see trac #14159.
+ _cache[key] = KeyedRef(H, _Callback, (id(X),id(Y),id(category)))
return H
def hom(X, Y, f):
diff --git a/sage/structure/coerce_dict.pyx b/sage/structure/coerce_dict.pyx
--- a/sage/structure/coerce_dict.pyx
+++ b/sage/structure/coerce_dict.pyx
@@ -266,6 +266,104 @@
D._size -= 1
break
+cdef class TripleDictEraserOnValue(TripleDictEraser):
+ """
+ A callback function for keyed weak references usable in values of :class:`TripleDict`
+
+ NOTE:
+
+ See discussion at :trac:`14159`.
+
+ TESTS:
+
+ We create a :class:`TripleDict` and a class whose items allow for weak references.
+ ::
+
+ sage: from sage.structure.coerce_dict import TripleDictEraserOnValue, TripleDictEraser, TripleDict
+ sage: import gc
+ sage: T = TripleDict(13)
+ sage: EV = TripleDictEraserOnValue(T)
+ sage: E = TripleDictEraser(T)
+ sage: class Foo: pass
+ sage: a = Foo()
+ sage: b = Foo()
+ sage: c = Foo()
+ sage: d = Foo()
+ sage: UnsafeRef = weakref.KeyedRef(b, E, (id(a),id(a),id(a)))
+ sage: SafeRef = weakref.KeyedRef(c, EV, (id(a),id(a),id(a)))
+ sage: SafeRef2 = weakref.KeyedRef(d, EV, (id(a),id(a),id(a)))
+
+ When we use ``UnsafeRef`` as a value of a :class:`TripleDict`, overwrite
+ the value and then trigger the callback, we find that the overwritten item
+ was deleted::
+
+ sage: T[a,a,a] = UnsafeRef
+ sage: (a,a,a) in T
+ True
+ sage: T[a,a,a] = 5
+ sage: (a,a,a) in T
+ True
+ sage: del b
+ sage: _ = gc.collect()
+ sage: (a,a,a) in T
+ False
+
+ This is a side effect that can induce very difficult to debug
+ problems\---see :trac:`13387`. If we do the same with ``SafeRef``, then
+ the overwritten item is preserved::
+
+ sage: T[a,a,a] = SafeRef
+ sage: (a,a,a) in T
+ True
+ sage: T[a,a,a] = 5
+ sage: (a,a,a) in T
+ True
+ sage: del c
+ sage: _ = gc.collect()
+ sage: T[a,a,a]
+ 5
+
+ However, if the value is not overridden, then the callback will correctly
+ delete the corresponding item from the :class:`TripleDict`::
+
+ sage: T[a,a,a] = SafeRef2
+ sage: (a,a,a) in T
+ True
+ sage: del d
+ sage: _ = gc.collect()
+ sage: (a,a,a) in T
+ False
+
+ """
+ def __call__(self, r):
+ # The only difference wrt TripleDictEraser.__call__ is that we will
+ # only erase an item if its value is identic with the keyed reference
+ # r.
+ cdef PyObject * r_c = r
+ cdef TripleDict D =