From 52cad43bc3d3126fcf7e08582373c12e88895cd3 Mon Sep 17 00:00:00 2001 From: Florian Apolloner Date: Tue, 7 Jan 2014 01:11:13 +0100 Subject: [PATCH] Fixed removal of signal receivers in Python 3.4 Make use of `weakref.finalize` and `weakref.WeakMethod` on python 3.4. Simplified the removal of receivers, the old function looked overly complicated. Many thanks go to Antoine Pitrou for helping me to debug and explain all the failures I ran into while writing that patch. --- django/dispatch/dispatcher.py | 63 ++++--- django/dispatch/saferef.py | 261 --------------------------- django/dispatch/weakref_backports.py | 62 +++++++ tests/dispatch/tests/test_saferef.py | 76 -------- 4 files changed, 98 insertions(+), 364 deletions(-) delete mode 100644 django/dispatch/saferef.py create mode 100644 django/dispatch/weakref_backports.py delete mode 100644 tests/dispatch/tests/test_saferef.py diff --git a/django/dispatch/dispatcher.py b/django/dispatch/dispatcher.py index d26433a885..7302f6428e 100644 --- a/django/dispatch/dispatcher.py +++ b/django/dispatch/dispatcher.py @@ -1,11 +1,13 @@ -import weakref +import sys import threading +import weakref -from django.dispatch import saferef from django.utils.six.moves import xrange - -WEAKREF_TYPES = (weakref.ReferenceType, saferef.BoundMethodWeakref) +if sys.version_info < (3,4): + from .weakref_backports import WeakMethod +else: + from weakref import WeakMethod def _make_id(target): @@ -57,9 +59,7 @@ class Signal(object): A function or an instance method which is to receive signals. Receivers must be hashable objects. - If weak is True, then receiver must be weak-referencable (more - precisely saferef.safeRef() must be able to create a reference - to the receiver). + If weak is True, then receiver must be weak-referencable. Receivers must be able to accept keyword arguments. @@ -105,20 +105,33 @@ class Signal(object): assert argspec[2] is not None, \ "Signal receivers must accept keyword arguments (**kwargs)." + receiver_id = _make_id(receiver) if dispatch_uid: lookup_key = (dispatch_uid, _make_id(sender)) else: - lookup_key = (_make_id(receiver), _make_id(sender)) + lookup_key = (receiver_id, _make_id(sender)) if weak: - receiver = saferef.safeRef(receiver, onDelete=self._remove_receiver) + ref = weakref.ref + original_receiver = receiver + # Check for bound methods + if hasattr(receiver, '__self__') and hasattr(receiver, '__func__'): + ref = WeakMethod + original_receiver = original_receiver.__self__ + if sys.version_info >= (3, 4): + receiver = ref(receiver) + weakref.finalize(original_receiver, self._remove_receiver, receiver_id=receiver_id) + else: + receiver = ref(receiver, self._remove_receiver) + # Use the id of the weakref, since that's what passed to the weakref callback! + receiver_id = _make_id(receiver) with self.lock: - for r_key, _ in self.receivers: + for r_key, _, _ in self.receivers: if r_key == lookup_key: break else: - self.receivers.append((lookup_key, receiver)) + self.receivers.append((lookup_key, receiver, receiver_id)) self.sender_receivers_cache.clear() def disconnect(self, receiver=None, sender=None, weak=True, dispatch_uid=None): @@ -150,7 +163,7 @@ class Signal(object): with self.lock: for index in xrange(len(self.receivers)): - (r_key, _) = self.receivers[index] + (r_key, _, _) = self.receivers[index] if r_key == lookup_key: del self.receivers[index] break @@ -242,7 +255,7 @@ class Signal(object): with self.lock: senderkey = _make_id(sender) receivers = [] - for (receiverkey, r_senderkey), receiver in self.receivers: + for (receiverkey, r_senderkey), receiver, _ in self.receivers: if r_senderkey == NONE_ID or r_senderkey == senderkey: receivers.append(receiver) if self.use_caching: @@ -253,7 +266,7 @@ class Signal(object): self.sender_receivers_cache[sender] = receivers non_weak_receivers = [] for receiver in receivers: - if isinstance(receiver, WEAKREF_TYPES): + if isinstance(receiver, weakref.ReferenceType): # Dereference the weak reference. receiver = receiver() if receiver is not None: @@ -262,23 +275,19 @@ class Signal(object): non_weak_receivers.append(receiver) return non_weak_receivers - def _remove_receiver(self, receiver): + def _remove_receiver(self, receiver=None, receiver_id=None, _make_id=_make_id): """ Remove dead receivers from connections. - """ + `receiver_id` is used by python 3.4 and up. `receiver` is used in older + versions and is the weakref to the receiver (if the connection was defined + as `weak`). We also need to pass on `_make_id` since the original reference + will be None during module shutdown. + """ with self.lock: - to_remove = [] - for key, connected_receiver in self.receivers: - if connected_receiver == receiver: - to_remove.append(key) - for key in to_remove: - last_idx = len(self.receivers) - 1 - # enumerate in reverse order so that indexes are valid even - # after we delete some items - for idx, (r_key, _) in enumerate(reversed(self.receivers)): - if r_key == key: - del self.receivers[last_idx - idx] + if receiver is not None: + receiver_id = _make_id(receiver) + self.receivers[:] = [val for val in self.receivers if val[2] != receiver_id] self.sender_receivers_cache.clear() diff --git a/django/dispatch/saferef.py b/django/dispatch/saferef.py deleted file mode 100644 index d040ecdbde..0000000000 --- a/django/dispatch/saferef.py +++ /dev/null @@ -1,261 +0,0 @@ -""" -"Safe weakrefs", originally from pyDispatcher. - -Provides a way to safely weakref any function, including bound methods (which -aren't handled by the core weakref module). -""" - -import traceback -import weakref - - -def safeRef(target, onDelete=None): - """Return a *safe* weak reference to a callable target - - target -- the object to be weakly referenced, if it's a - bound method reference, will create a BoundMethodWeakref, - otherwise creates a simple weakref. - onDelete -- if provided, will have a hard reference stored - to the callable to be called after the safe reference - goes out of scope with the reference object, (either a - weakref or a BoundMethodWeakref) as argument. - """ - if hasattr(target, '__self__'): - if target.__self__ is not None: - # Turn a bound method into a BoundMethodWeakref instance. - # Keep track of these instances for lookup by disconnect(). - assert hasattr(target, '__func__'), """safeRef target %r has __self__, but no __func__, don't know how to create reference""" % (target,) - reference = get_bound_method_weakref( - target=target, - onDelete=onDelete - ) - return reference - if callable(onDelete): - return weakref.ref(target, onDelete) - else: - return weakref.ref(target) - - -class BoundMethodWeakref(object): - """'Safe' and reusable weak references to instance methods - - BoundMethodWeakref objects provide a mechanism for - referencing a bound method without requiring that the - method object itself (which is normally a transient - object) is kept alive. Instead, the BoundMethodWeakref - object keeps weak references to both the object and the - function which together define the instance method. - - Attributes: - key -- the identity key for the reference, calculated - by the class's calculateKey method applied to the - target instance method - deletionMethods -- sequence of callable objects taking - single argument, a reference to this object which - will be called when *either* the target object or - target function is garbage collected (i.e. when - this object becomes invalid). These are specified - as the onDelete parameters of safeRef calls. - weakSelf -- weak reference to the target object - weakFunc -- weak reference to the target function - - Class Attributes: - _allInstances -- class attribute pointing to all live - BoundMethodWeakref objects indexed by the class's - calculateKey(target) method applied to the target - objects. This weak value dictionary is used to - short-circuit creation so that multiple references - to the same (object, function) pair produce the - same BoundMethodWeakref instance. - - """ - - _allInstances = weakref.WeakValueDictionary() - - def __new__(cls, target, onDelete=None, *arguments, **named): - """Create new instance or return current instance - - Basically this method of construction allows us to - short-circuit creation of references to already- - referenced instance methods. The key corresponding - to the target is calculated, and if there is already - an existing reference, that is returned, with its - deletionMethods attribute updated. Otherwise the - new instance is created and registered in the table - of already-referenced methods. - """ - key = cls.calculateKey(target) - current = cls._allInstances.get(key) - if current is not None: - current.deletionMethods.append(onDelete) - return current - else: - base = super(BoundMethodWeakref, cls).__new__(cls) - cls._allInstances[key] = base - base.__init__(target, onDelete, *arguments, **named) - return base - - def __init__(self, target, onDelete=None): - """Return a weak-reference-like instance for a bound method - - target -- the instance-method target for the weak - reference, must have __self__ and __func__ attributes - and be reconstructable via: - target.__func__.__get__( target.__self__ ) - which is true of built-in instance methods. - onDelete -- optional callback which will be called - when this weak reference ceases to be valid - (i.e. either the object or the function is garbage - collected). Should take a single argument, - which will be passed a pointer to this object. - """ - def remove(weak, self=self): - """Set self.isDead to true when method or instance is destroyed""" - methods = self.deletionMethods[:] - del self.deletionMethods[:] - try: - del self.__class__._allInstances[self.key] - except KeyError: - pass - for function in methods: - try: - if callable(function): - function(self) - except Exception as e: - try: - traceback.print_exc() - except AttributeError: - print('Exception during saferef %s cleanup function %s: %s' % ( - self, function, e) - ) - self.deletionMethods = [onDelete] - self.key = self.calculateKey(target) - self.weakSelf = weakref.ref(target.__self__, remove) - self.weakFunc = weakref.ref(target.__func__, remove) - self.selfName = str(target.__self__) - self.funcName = str(target.__func__.__name__) - - @classmethod - def calculateKey(cls, target): - """Calculate the reference key for this reference - - Currently this is a two-tuple of the id()'s of the - target object and the target function respectively. - """ - return (id(target.__self__), id(target.__func__)) - - def __str__(self): - """Give a friendly representation of the object""" - return """%s( %s.%s )""" % ( - self.__class__.__name__, - self.selfName, - self.funcName, - ) - - __repr__ = __str__ - - def __hash__(self): - return hash(self.key) - - def __bool__(self): - """Whether we are still a valid reference""" - return self() is not None - - def __nonzero__(self): # Python 2 compatibility - return type(self).__bool__(self) - - def __eq__(self, other): - """Compare with another reference""" - if not isinstance(other, self.__class__): - return self.__class__ == type(other) - return self.key == other.key - - def __call__(self): - """Return a strong reference to the bound method - - If the target cannot be retrieved, then will - return None, otherwise returns a bound instance - method for our object and function. - - Note: - You may call this method any number of times, - as it does not invalidate the reference. - """ - target = self.weakSelf() - if target is not None: - function = self.weakFunc() - if function is not None: - return function.__get__(target) - return None - - -class BoundNonDescriptorMethodWeakref(BoundMethodWeakref): - """A specialized BoundMethodWeakref, for platforms where instance methods - are not descriptors. - - It assumes that the function name and the target attribute name are the - same, instead of assuming that the function is a descriptor. This approach - is equally fast, but not 100% reliable because functions can be stored on an - attribute named differenty than the function's name such as in: - - class A: pass - def foo(self): return "foo" - A.bar = foo - - But this shouldn't be a common use case. So, on platforms where methods - aren't descriptors (such as Jython) this implementation has the advantage - of working in the most cases. - """ - def __init__(self, target, onDelete=None): - """Return a weak-reference-like instance for a bound method - - target -- the instance-method target for the weak - reference, must have __self__ and __func__ attributes - and be reconstructable via: - target.__func__.__get__( target.__self__ ) - which is true of built-in instance methods. - onDelete -- optional callback which will be called - when this weak reference ceases to be valid - (i.e. either the object or the function is garbage - collected). Should take a single argument, - which will be passed a pointer to this object. - """ - assert getattr(target.__self__, target.__name__) == target, \ - ("method %s isn't available as the attribute %s of %s" % - (target, target.__name__, target.__self__)) - super(BoundNonDescriptorMethodWeakref, self).__init__(target, onDelete) - - def __call__(self): - """Return a strong reference to the bound method - - If the target cannot be retrieved, then will - return None, otherwise returns a bound instance - method for our object and function. - - Note: - You may call this method any number of times, - as it does not invalidate the reference. - """ - target = self.weakSelf() - if target is not None: - function = self.weakFunc() - if function is not None: - # Using partial() would be another option, but it erases the - # "signature" of the function. That is, after a function is - # curried, the inspect module can't be used to determine how - # many arguments the function expects, nor what keyword - # arguments it supports, and pydispatcher needs this - # information. - return getattr(target, function.__name__) - return None - - -def get_bound_method_weakref(target, onDelete): - """Instantiates the appropiate BoundMethodWeakRef, depending on the details of - the underlying class method implementation""" - if hasattr(target, '__get__'): - # target method is a descriptor, so the default implementation works: - return BoundMethodWeakref(target=target, onDelete=onDelete) - else: - # no luck, use the alternative implementation: - return BoundNonDescriptorMethodWeakref(target=target, onDelete=onDelete) diff --git a/django/dispatch/weakref_backports.py b/django/dispatch/weakref_backports.py new file mode 100644 index 0000000000..a5cc82bf10 --- /dev/null +++ b/django/dispatch/weakref_backports.py @@ -0,0 +1,62 @@ +""" +weakref_backports is a partial backport of the weakref module for python +versions below 3.4. + +TODO: LICENSE! +""" +from weakref import ref + + +class WeakMethod(ref): + """ + A custom `weakref.ref` subclass which simulates a weak reference to + a bound method, working around the lifetime problem of bound methods. + """ + + __slots__ = "_func_ref", "_meth_type", "_alive", "__weakref__" + + def __new__(cls, meth, callback=None): + try: + obj = meth.__self__ + func = meth.__func__ + except AttributeError: + raise TypeError("argument should be a bound method, not {}" + .format(type(meth))) + def _cb(arg): + # The self-weakref trick is needed to avoid creating a reference + # cycle. + self = self_wr() + if self._alive: + self._alive = False + if callback is not None: + callback(self) + self = ref.__new__(cls, obj, _cb) + self._func_ref = ref(func, _cb) + self._meth_type = type(meth) + self._alive = True + self_wr = ref(self) + return self + + def __call__(self): + obj = super(WeakMethod, self).__call__() + func = self._func_ref() + if obj is None or func is None: + return None + return self._meth_type(func, obj) + + def __eq__(self, other): + if isinstance(other, WeakMethod): + if not self._alive or not other._alive: + return self is other + return ref.__eq__(self, other) and self._func_ref == other._func_ref + return False + + def __ne__(self, other): + if isinstance(other, WeakMethod): + if not self._alive or not other._alive: + return self is not other + return ref.__ne__(self, other) or self._func_ref != other._func_ref + return True + + __hash__ = ref.__hash__ + diff --git a/tests/dispatch/tests/test_saferef.py b/tests/dispatch/tests/test_saferef.py deleted file mode 100644 index 6da756362a..0000000000 --- a/tests/dispatch/tests/test_saferef.py +++ /dev/null @@ -1,76 +0,0 @@ -import unittest - -from django.dispatch.saferef import safeRef -from django.utils.six.moves import xrange - - -class Test1(object): - def x(self): - pass - - -def test2(obj): - pass - - -class Test2(object): - def __call__(self, obj): - pass - - -class SaferefTests(unittest.TestCase): - def setUp(self): - ts = [] - ss = [] - for x in xrange(5000): - t = Test1() - ts.append(t) - s = safeRef(t.x, self._closure) - ss.append(s) - ts.append(test2) - ss.append(safeRef(test2, self._closure)) - for x in xrange(30): - t = Test2() - ts.append(t) - s = safeRef(t, self._closure) - ss.append(s) - self.ts = ts - self.ss = ss - self.closureCount = 0 - - def tearDown(self): - del self.ts - del self.ss - - def testIn(self): - """Test the "in" operator for safe references (cmp)""" - for t in self.ts[:50]: - self.assertTrue(safeRef(t.x) in self.ss) - - def testValid(self): - """Test that the references are valid (return instance methods)""" - for s in self.ss: - self.assertTrue(s()) - - def testShortCircuit(self): - """Test that creation short-circuits to reuse existing references""" - sd = {} - for s in self.ss: - sd[s] = 1 - for t in self.ts: - if hasattr(t, 'x'): - self.assertTrue(safeRef(t.x) in sd) - else: - self.assertTrue(safeRef(t) in sd) - - def testRepresentation(self): - """Test that the reference object's representation works - - XXX Doesn't currently check the results, just that no error - is raised - """ - repr(self.ss[-1]) - - def _closure(self, ref): - """Dumb utility mechanism to increment deletion counter""" - self.closureCount += 1