diff options
| author | Simon Charette <charette.s@gmail.com> | 2025-01-23 00:55:17 -0500 |
|---|---|---|
| committer | Mariusz Felisiak <felisiak.mariusz@gmail.com> | 2025-04-23 13:09:46 +0100 |
| commit | 760121dcb1837fdee6ac7f4e2412c0d539fbff7a (patch) | |
| tree | 4694aaa3ea905b0138633fa6f21122e0e91524c3 /django/dispatch | |
| parent | 19067fe85a32a51703b7ed29626a30e00899007f (diff) | |
Fixed #35801 -- Prevented collision of senders with non-overlapping lifetimes.
As documented, the id() function can return the same value for distinct
objects with non-overlapping lifetimes which can result in signals being
sent to the wrong receivers if two distinct senders happen to have a
colliding id() value.
Since reproduction of the issue requires memory constrained
circumstances where the same exact id() is reused for two senders of the
same signal the test opt to simulate the collision by systematically
making the same id for Sender instances.
Note that we explicitly avoid keeping a strong reference to senders that
cannot be weakly referenced as that would unexpectedly prevent them from
being garbage collected. This means that id(sender) collisions could
still occur for such objects but Django itself doesn't make use of them.
Thanks Sjoerd Job Postmus for the reduced test case and Mariusz for the
review.
Co-authored-by: And Clover <and@doxdesk.com>
Diffstat (limited to 'django/dispatch')
| -rw-r--r-- | django/dispatch/dispatcher.py | 61 |
1 files changed, 40 insertions, 21 deletions
diff --git a/django/dispatch/dispatcher.py b/django/dispatch/dispatcher.py index fe0e1fa599..4b962ce524 100644 --- a/django/dispatch/dispatcher.py +++ b/django/dispatch/dispatcher.py @@ -28,8 +28,10 @@ class Signal: Internal attributes: - receivers - { receiverkey (id) : weakref(receiver) } + receivers: + [((id(receiver), id(sender)), ref(receiver), ref(sender), is_async)] + sender_receivers_cache: + WeakKeyDictionary[sender, list[receiver]] """ def __init__(self, use_caching=False): @@ -108,12 +110,23 @@ class Signal: ref = weakref.WeakMethod receiver_object = receiver.__self__ receiver = ref(receiver) - weakref.finalize(receiver_object, self._remove_receiver) + weakref.finalize(receiver_object, self._flag_dead_receivers) + + # Keep a weakref to sender if possible to ensure associated receivers + # are cleared if it gets garbage collected. This ensures there is no + # id(sender) collisions for distinct senders with non-overlapping + # lifetimes. + sender_ref = None + if sender is not None: + try: + sender_ref = weakref.ref(sender, self._flag_dead_receivers) + except TypeError: + pass with self.lock: self._clear_dead_receivers() - if not any(r_key == lookup_key for r_key, _, _ in self.receivers): - self.receivers.append((lookup_key, receiver, is_async)) + if not any(r_key == lookup_key for r_key, _, _, _ in self.receivers): + self.receivers.append((lookup_key, receiver, sender_ref, is_async)) self.sender_receivers_cache.clear() def disconnect(self, receiver=None, sender=None, dispatch_uid=None): @@ -410,7 +423,10 @@ class Signal: self.receivers = [ r for r in self.receivers - if not (isinstance(r[1], weakref.ReferenceType) and r[1]() is None) + if ( + not (isinstance(r[1], weakref.ReferenceType) and r[1]() is None) + and not (r[2] is not None and r[2]() is None) + ) ] def _live_receivers(self, sender): @@ -432,9 +448,14 @@ class Signal: self._clear_dead_receivers() senderkey = _make_id(sender) receivers = [] - for (_receiverkey, r_senderkey), receiver, is_async in self.receivers: + for ( + (_receiverkey, r_senderkey), + receiver, + sender_ref, + is_async, + ) in self.receivers: if r_senderkey == NONE_ID or r_senderkey == senderkey: - receivers.append((receiver, is_async)) + receivers.append((receiver, sender_ref, is_async)) if self.use_caching: if not receivers: self.sender_receivers_cache[sender] = NO_RECEIVERS @@ -443,27 +464,25 @@ class Signal: self.sender_receivers_cache[sender] = receivers non_weak_sync_receivers = [] non_weak_async_receivers = [] - for receiver, is_async in receivers: + for receiver, sender_ref, is_async in receivers: + # Skip if the receiver/sender is a dead weakref if isinstance(receiver, weakref.ReferenceType): - # Dereference the weak reference. receiver = receiver() - if receiver is not None: - if is_async: - non_weak_async_receivers.append(receiver) - else: - non_weak_sync_receivers.append(receiver) + if receiver is None: + continue + if sender_ref is not None and sender_ref() is None: + continue + if is_async: + non_weak_async_receivers.append(receiver) else: - if is_async: - non_weak_async_receivers.append(receiver) - else: - non_weak_sync_receivers.append(receiver) + non_weak_sync_receivers.append(receiver) return non_weak_sync_receivers, non_weak_async_receivers - def _remove_receiver(self, receiver=None): + def _flag_dead_receivers(self, reference=None): # Mark that the self.receivers list has dead weakrefs. If so, we will # clean those up in connect, disconnect and _live_receivers while # holding self.lock. Note that doing the cleanup here isn't a good - # idea, _remove_receiver() will be called as side effect of garbage + # idea, _flag_dead_receivers() will be called as side effect of garbage # collection, and so the call can happen while we are already holding # self.lock. self._dead_receivers = True |
