Ticket #14159: trac_14159_safer_callback.patch

File trac_14159_safer_callback.patch, 7.2 KB (added by SimonKing, 7 years ago)

A safe callback for weak values of a TripleDict

  • sage/categories/homset.py

    # HG changeset patch
    # User Simon King <simon.king@uni-jena.de>
    # 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 b  
    7676# introduced in trac ticket #715
    7777
    7878from weakref import KeyedRef
    79 from sage.structure.coerce_dict import TripleDict
     79from sage.structure.coerce_dict import TripleDict, TripleDictEraserOnValue
    8080_cache = TripleDict(53)
     81_Callback = TripleDictEraserOnValue(_cache)
    8182
    8283def Hom(X, Y, category=None):
    8384    """
     
    201202        sage: Hom(PA,PJ, Rngs())
    202203        Set of Morphisms from <type 'sage.structure.parent.Parent'> to <type 'sage.structure.parent.Parent'> in Category of rngs
    203204
     205    By :trac:`14159`, the cache of homsets uses a weak reference with
     206    :class:`~sage.structure.coerce_dict.TripleDictEraserOnValue` as callback.
     207    This avoids certain rare race conditions in invoking Python's cyclic
     208    garbage collector, but still fixes a memory leak::
     209
     210        sage: R=[ZZ.quo(3^n) for n in [1..50]]
     211        sage: C = type(Hom(R[0],R[0]))
     212        sage: _ = gc.collect()
     213        sage: l = len([x for x in gc.get_objects() if isinstance(x,C)])
     214        sage: def test(R):
     215        ...    for i in range(len(R)):
     216        ...        for j in range(i,len(R)):
     217        ...            H=Hom(R[j],R[i])
     218        sage: test(R)
     219        sage: _ = gc.collect()
     220        sage: l == len([x for x in gc.get_objects() if isinstance(x,C)])
     221        True
     222        sage: isinstance(Hom(R[10],R[10]), C)
     223        True
     224
    204225    .. TODO::
    205226
    206227        design decision: how much of the homset comes from the
     
    220241    except KeyError:
    221242        H = None
    222243    if H is not None:
    223         # Are domain or codomain breaking the unique parent condition?
    224         if H.domain() is X and H.codomain() is Y:
    225             return H
     244        # Note H has just been found using the identities of X and Y as
     245        # keys, so if the domain and codomain of H do not match by identity
     246        # something went wrong and we want to know about it.
     247        assert H.domain() is X and H.codomain() is Y
     248        return H
    226249
    227250    try:
    228251        # Apparently X._Hom_ is supposed to be cached
     
    248271    except KeyError:
    249272        H = None
    250273    if H is not None:
    251         # Are domain or codomain breaking the unique parent condition?
    252         if H.domain() is X and H.codomain() is Y:
    253             return H
     274        # We double check domain and codomain are expected (see above)
     275        assert H.domain() is X and H.codomain() is Y
     276        return H
    254277
    255278    # coercing would be incredibly annoying, since the domain and codomain
    256279    # are totally different objects
     
    262285    # of Homset in rings, schemes, ...
    263286    H = category.hom_category().parent_class(X, Y, category = category)
    264287           
    265     ##_cache[key] = weakref.ref(H)
    266     _cache[key] = KeyedRef(H, _cache.eraser, (id(X),id(Y),id(category)))
     288    # Since H is holding strong references to X,Y,category in the form of
     289    # H.domain(), H.codomain(), H.homset_category(), it is OK to just store a
     290    # weak reference to the homset: Even though X,Y,category are only weakly
     291    # referenced by _cache, they can't be garbage unless H is as well. The
     292    # callback below is safe---see trac #14159.
     293    _cache[key] = KeyedRef(H, _Callback, (id(X),id(Y),id(category)))
    267294    return H
    268295
    269296def hom(X, Y, f):
  • sage/structure/coerce_dict.pyx

    diff --git a/sage/structure/coerce_dict.pyx b/sage/structure/coerce_dict.pyx
    a b  
    266266                D._size -= 1
    267267                break
    268268
     269cdef class TripleDictEraserOnValue(TripleDictEraser):
     270    """
     271    A callback function for keyed weak references usable in values of :class:`TripleDict`
     272
     273    NOTE:
     274
     275    See discussion at :trac:`14159`.
     276
     277    TESTS:
     278
     279    We create a :class:`TripleDict` and a class whose items allow for weak references.
     280    ::
     281
     282        sage: from sage.structure.coerce_dict import TripleDictEraserOnValue, TripleDictEraser, TripleDict
     283        sage: import gc
     284        sage: T = TripleDict(13)
     285        sage: EV = TripleDictEraserOnValue(T)
     286        sage: E = TripleDictEraser(T)
     287        sage: class Foo: pass
     288        sage: a = Foo()
     289        sage: b = Foo()
     290        sage: c = Foo()
     291        sage: d = Foo()
     292        sage: UnsafeRef = weakref.KeyedRef(b, E, (id(a),id(a),id(a)))
     293        sage: SafeRef = weakref.KeyedRef(c, EV, (id(a),id(a),id(a)))
     294        sage: SafeRef2 = weakref.KeyedRef(d, EV, (id(a),id(a),id(a)))
     295
     296    When we use ``UnsafeRef`` as a value of a :class:`TripleDict`, overwrite
     297    the value and then trigger the callback, we find that the overwritten item
     298    was deleted::
     299
     300        sage: T[a,a,a] = UnsafeRef
     301        sage: (a,a,a) in T
     302        True
     303        sage: T[a,a,a] = 5
     304        sage: (a,a,a) in T
     305        True
     306        sage: del b
     307        sage: _ = gc.collect()
     308        sage: (a,a,a) in T
     309        False
     310
     311    This is a side effect that can induce very difficult to debug
     312    problems\---see :trac:`13387`. If we do the same with ``SafeRef``, then
     313    the overwritten item is preserved::
     314
     315        sage: T[a,a,a] = SafeRef
     316        sage: (a,a,a) in T
     317        True
     318        sage: T[a,a,a] = 5
     319        sage: (a,a,a) in T
     320        True
     321        sage: del c
     322        sage: _ = gc.collect()
     323        sage: T[a,a,a]
     324        5
     325
     326    However, if the value is not overridden, then the callback will correctly
     327    delete the corresponding item from the :class:`TripleDict`::
     328
     329        sage: T[a,a,a] = SafeRef2
     330        sage: (a,a,a) in T
     331        True
     332        sage: del d
     333        sage: _ = gc.collect()
     334        sage: (a,a,a) in T
     335        False
     336
     337    """
     338    def __call__(self, r):
     339        # The only difference wrt TripleDictEraser.__call__ is that we will
     340        # only erase an item if its value is identic with the keyed reference
     341        # r.
     342        cdef PyObject * r_c = <PyObject*>r
     343        cdef TripleDict D = <object>PyWeakref_GetObject(self.D)
     344        if D is None:
     345            return
     346        cdef list buckets = D.buckets
     347        if buckets is None:
     348            return
     349        # r is a (weak) reference (typically to a parent), and it knows the
     350        # stored key of the unique triple r() had been part of.
     351        # We remove that unique triple from self.D
     352        cdef Py_ssize_t k1,k2,k3
     353        k1,k2,k3 = r.key
     354        cdef Py_ssize_t h = (k1 + 13*k2 ^ 503*k3)
     355        cdef list bucket = <object>PyList_GET_ITEM(buckets, (<size_t>h) % PyList_GET_SIZE(buckets))
     356        cdef Py_ssize_t i
     357        for i from 0 <= i < PyList_GET_SIZE(bucket) by 7:
     358            if PyInt_AsSsize_t(PyList_GET_ITEM(bucket, i))==k1 and \
     359               PyInt_AsSsize_t(PyList_GET_ITEM(bucket, i+1))==k2 and \
     360               PyInt_AsSsize_t(PyList_GET_ITEM(bucket, i+2))==k3:
     361                if PyList_GET_ITEM(bucket, i+6)!=r_c:
     362                    return
     363                del bucket[i:i+7]
     364                D._size -= 1
     365                break
     366
    269367cdef class MonoDict:
    270368    """
    271369    This is a hashtable specifically designed for (read) speed in