Merge c95598e into b5cf152

traits/adaptation/cached_adapter_factory.py

@@ -13,10 +13,9 @@
 """ An adapter factory that caches adapters per instance. """
 
 
-import weakref
-
 from traits.api import Any, Bool, HasTraits, Property
 from traits.util.api import import_symbol
+from traits.util.weakiddict import WeakIDKeyDict
 
 
 class CachedAdapterFactory(HasTraits):
@@ -68,7 +67,7 @@ def _get_is_empty(self):
 
     _adapter_cache = Any
     def __adapter_cache_default(self):
-        return weakref.WeakKeyDictionary()
+        return WeakIDKeyDict()
 
     #: Shadow trait for the corresponding property.
     _factory = Any

traits/tests/test_dynamic_notifiers.py

@@ -85,6 +85,20 @@ def low_priority_second(self):
     def high_priority_second(self):
         self.prioritized_notifications.append(3)
 
+
+class UnhashableDynamicNotifiers(DynamicNotifiers):
+    """ Class that cannot be used as a key in a dictionary.
+    """
+
+    a_list = List
+
+    def __hash__(self):
+        raise NotImplementedError()
+
+    def __eq__(self):
+        raise NotImplementedError()
+
+
 # 'ok' function listeners
 
 calls_0 = []
@@ -282,6 +296,33 @@ def obj_collected_callback(weakref):
 
         self.assertEqual(obj_collected, [True])
 
+    def test_unhashable_object_can_be_garbage_collected(self):
+        # Make sure that an unhashable trait object can be garbage collected
+        # even though there are listener to its traits.
+
+        import weakref
+
+        obj = UnhashableDynamicNotifiers()
+        obj.on_trait_change(function_listener_0, 'a_list:ok')
+        # Changing a List trait is the easiest way to trigger a check into the
+        # weak key dict.
+        obj.a_list.append(UnhashableDynamicNotifiers())
+
+        # Create a weak reference to `obj` with a callback that flags when the
+        # object is finalized.
+        obj_collected = []
+
+        def obj_collected_callback(weakref):
+            obj_collected.append(True)
+
+        obj_weakref = weakref.ref(obj, obj_collected_callback)
+
+        # Remove reference to `obj`, and check that the weak reference
+        # callback has been called, indicating that it has been collected.
+        del obj
+
+        self.assertEqual(obj_collected, [True])
+
     def test_creating_notifiers_dont_create_cyclic_garbage(self):
         gc.collect()
         DynamicNotifiers()

traits/traits_listener.py

@@ -28,7 +28,6 @@
 import re
 import string
 import weakref
-from weakref import WeakKeyDictionary
 from string import whitespace
 from types import MethodType
 
@@ -41,6 +40,7 @@
 from .trait_types import Str, Int, Bool, Instance, List, Enum, Any
 from .trait_errors import TraitError
 from .trait_notifiers import TraitChangeNotifyWrapper
+from .util.weakiddict import WeakIDKeyDict
 
 #---------------------------------------------------------------------------
 #  Constants:
@@ -291,7 +291,7 @@ class ListenerItem ( ListenerBase ):
     #: A dictionary mapping objects to a list of all current active
     #: (*name*, *type*) listener pairs, where *type* defines the type of
     #: listener, one of: (SIMPLE_LISTENER, LIST_LISTENER, DICT_LISTENER).
-    active = Instance( WeakKeyDictionary, () )
+    active = Instance( WeakIDKeyDict, () )
 
     #-- 'ListenerBase' Class Method Implementations ----------------------------
 

traits/util/tests/test_weakidddict.py

@@ -0,0 +1,128 @@
+import gc
+import unittest
+
+import six
+
+from ..weakiddict import WeakIDDict, WeakIDKeyDict
+
+
+class AllTheSame(object):
+    def __hash__(self):
+        return 42
+
+    def __eq__(self, other):
+        return isinstance(other, type(self))
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+
+class WeakreffableInt(object):
+    def __init__(self, value):
+        self.value = value
+
+    def __hash__(self):
+        return hash(self.value)
+
+    def __eq__(self, other):
+        if isinstance(other, int):
+            return self.value == other
+        else:
+            return self.value == other.value
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+
+class TestWeakIDDict(unittest.TestCase):
+    if six.PY2:
+        assertCountEqual = unittest.TestCase.assertItemsEqual
+
+    def test_weak_keys(self):
+        wd = WeakIDKeyDict()
+
+        keep = []
+        dont_keep = []
+        values = list(range(10))
+        for n, i in enumerate(values, 1):
+            key = AllTheSame()
+            if not (i % 2):
+                keep.append(key)
+            else:
+                dont_keep.append(key)
+            wd[key] = i
+            del key
+            # No keys or values have been deleted, yet.
+            self.assertEqual(len(wd), n)
+
+        # Delete half of the keys.
+        self.assertEqual(len(wd), 10)
+        del dont_keep
+        self.assertEqual(len(wd), 5)
+
+        # Check the remaining values.
+        self.assertCountEqual(
+            list(wd.values()),
+            list(range(0, 10, 2)))
+        self.assertEqual(
+            [wd[k] for k in keep],
+            list(range(0, 10, 2)))
+
+        # Check the remaining keys.
+        self.assertCountEqual(
+            [id(k) for k in wd.keys()],
+            [id(k) for k in wd])
+        self.assertCountEqual(
+            [id(k) for k in wd.keys()],
+            [id(k) for k in keep])
+
+    def test_weak_keys_values(self):
+        wd = WeakIDDict()
+
+        keep = []
+        dont_keep = []
+        values = list(map(WeakreffableInt, range(10)))
+        for n, i in enumerate(values, 1):
+            key = AllTheSame()
+            if not (i.value % 2):
+                keep.append(key)
+            else:
+                dont_keep.append(key)
+            wd[key] = i
+            del key
+            # No keys or values have been deleted, yet.
+            self.assertEqual(len(wd), n)
+
+        # Delete half of the keys.
+        self.assertEqual(len(wd), 10)
+        del dont_keep
+        self.assertEqual(len(wd), 5)
+
+        # Check the remaining values.
+        self.assertCountEqual(
+            list(wd.values()),
+            list(map(WeakreffableInt, [0, 2, 4, 6, 8])))
+        self.assertEqual(
+            [wd[k] for k in keep],
+            list(map(WeakreffableInt, [0, 2, 4, 6, 8])))
+
+        # Check the remaining keys.
+        self.assertCountEqual(
+            [id(k) for k in wd.keys()],
+            [id(k) for k in wd])
+        self.assertCountEqual(
+            [id(k) for k in wd.keys()],
+            [id(k) for k in keep])
+
+        # Delete the weak values progressively and ensure that the
+        # corresponding entries disappear.
+        del values[0:2]
+        self.assertEqual(len(wd), 4)
+        del values[0:2]
+        self.assertEqual(len(wd), 3)
+        del values[0:2]
+        self.assertEqual(len(wd), 2)
+        del values[0:2]
+        self.assertEqual(len(wd), 1)
+        del values[0:2]
+        self.assertEqual(len(wd), 0)

traits/util/weakiddict.py

@@ -0,0 +1,75 @@
+""" Variants of weak-key dictionaries that are based on object identity.
+
+They will ignore the ``__hash__`` and ``__eq__`` implementations on the
+objects. These are intended for various kinds of caches that map instances of
+classes to other things without keeping those instances alive. Note that
+iteration is not guarded, so if one were iterating over these dictionaries and
+one of the weakrefs got cleaned up, this might modify the structure and break
+the iteration. As this is not a common use for such caches, we have not
+bothered to make these dicts robust to that case.
+"""
+
+import collections
+from weakref import ref
+
+
+def _remover(key_id, id_dict_ref):
+    def callback(wr, id_dict_ref=id_dict_ref):
+        id_dict = id_dict_ref()
+        if id_dict is not None:
+            id_dict.data.pop(key_id, None)
+    return callback
+
+
+class WeakIDDict(collections.MutableMapping):
+    """ A weak-key-value dictionary that uses the id() of the key for
+    comparisons.
+    """
+
+    def __init__(self, dict=None):
+        self.data = {}
+        if dict is not None:
+            self.update(dict)
+
+    def __repr__(self):
+        return '<WeakIDDict at 0x{0:x}>'.format(id(self))
+
+    def __delitem__(self, key):
+        del self.data[id(key)]
+
+    def __getitem__(self, key):
+        return self.data[id(key)][1]()
+
+    def __setitem__(self, key, value):
+        self.data[id(key)] = (
+            ref(key, _remover(id(key), ref(self))),
+            ref(value, _remover(id(key), ref(self))))
+
+    def __len__(self):
+        return len(self.data)
+
+    def __contains__(self, key):
+        return id(key) in self.data
+
+    def __iter__(self):
+        for id_key in self.data:
+            wr_key = self.data[id_key][0]
+            key = wr_key()
+            if key is not None:
+                yield key
+
+
+class WeakIDKeyDict(WeakIDDict):
+    """ A weak-key dictionary that uses the id() of the key for comparisons.
+
+    This differs from `WeakIDDict` in that it does not try to make a weakref to
+    the values.
+    """
+
+    def __getitem__(self, key):
+        return self.data[id(key)][1]
+
+    def __setitem__(self, key, value):
+        self.data[id(key)] = (
+            ref(key, _remover(id(key), ref(self))),
+            value)