Reduce sender/receiver bookeeping accumulation to only leave empty sets behind.bugfix/bookeeping-leaks

2 files changed, 46 insertions, 29 deletions

diff --git a/blinker/base.py b/blinker/base.py
index 675c2e0..0577b65 100644
--- a/blinker/base.py
+++ b/blinker/base.py
@@ -329,21 +329,12 @@ class Signal(object):
     def _disconnect(self, receiver_id, sender_id):
         if sender_id == ANY_ID:
             if self._by_receiver.pop(receiver_id, False):
-                empty_buckets = []
-                for key, bucket in self._by_sender.items():
+                for bucket in self._by_sender.values():
                     bucket.discard(receiver_id)
-                    if not bucket:
-                        empty_buckets.append(key)
-                for key in empty_buckets:
-                    del self._by_sender[key]
             self.receivers.pop(receiver_id, None)
         else:
             self._by_sender[sender_id].discard(receiver_id)
-            if not self._by_sender[sender_id]:
-                del self._by_sender[sender_id]
             self._by_receiver[receiver_id].discard(sender_id)
-            if not self._by_receiver[receiver_id]:
-                del self._by_receiver[receiver_id]
 
     def _cleanup_receiver(self, receiver_ref):
         """Disconnect a receiver from all senders."""
@@ -356,8 +347,32 @@ class Signal(object):
         self._weak_senders.pop(sender_id, None)
         for receiver_id in self._by_sender.pop(sender_id, ()):
             self._by_receiver[receiver_id].discard(sender_id)
-            if not self._by_receiver[receiver_id]:
-                del self._by_receiver[receiver_id]
+
+    def _cleanup_bookkeeping(self):
+        """Prune unused sender/receiver bookeeping. Not threadsafe.
+
+        Connecting & disconnecting leave behind a small amount of bookeeping
+        for the receiver and sender values. Typical workloads using Blinker,
+        for example in most web apps, Flask, CLI scripts, etc., are not
+        adversely affected by this bookkeeping.
+
+        With a long-running Python process performing dynamic signal routing
+        with high volume- e.g. connecting to function closures, "senders" are
+        all unique object instances, and doing all of this over and over- you
+        may see memory usage will grow due to extraneous bookeeping. (Visible
+        in memory profiing as an increasing number of empty sets.)
+
+        This method will prune that bookeeping away, with the caveat that such
+        pruning is not threadsafe. The risk is that cleanup of a fully
+        disconnected receiver/sender pair occurs while another thread is
+        connecting that same pair. If you are in the highly dynamic, unique
+        receiver/sender situation that has lead you to this method, that
+        failure mode is perhaps not a big deal for you.
+        """
+        for mapping in (self._by_sender, self._by_receiver):
+            for _id, bucket in list(mapping.items()):
+                if not bucket:
+                    mapping.pop(_id, None)
 
     def _clear_state(self):
         """Throw away all signal state.  Useful for unit tests."""
diff --git a/tests/test_signals.py b/tests/test_signals.py
index e4fc24d..a1172ed 100644
--- a/tests/test_signals.py
+++ b/tests/test_signals.py
@@ -180,9 +180,14 @@ def test_signal_signals_weak_sender():
     assert len(sentinel) == 1
 
 
-def test_memory_leaks():
+def test_empty_bucket_growth():
     sentinel = Sentinel()
     sig = blinker.Signal()
+    senders = lambda: (len(sig._by_sender),
+                       sum(len(i) for i in sig._by_sender.values()))
+    receivers = lambda: (len(sig._by_receiver),
+                         sum(len(i) for i in sig._by_receiver.values()))
+
     receiver1 = sentinel.make_receiver('receiver1')
     receiver2 = sentinel.make_receiver('receiver2')
 
@@ -193,25 +198,22 @@ def test_memory_leaks():
     sig.connect(receiver1, sender=sender)
     sig.connect(receiver2, sender=sender)
 
-    assert len(sig._by_sender) == 1
-    assert len(sig._by_receiver) == 2
-    receivers_bucket = list(sig._by_sender.values())[0]
-    assert len(receivers_bucket) == 2
-    senders_buckets = list(sig._by_receiver.values())
-    assert len(senders_buckets[0]) == 1
-    assert len(senders_buckets[1]) == 1
+    assert senders() == (1, 2)
+    assert receivers() == (2, 2)
 
     sig.disconnect(receiver1, sender=sender)
-    assert len(sig._by_sender) == 1
-    assert len(sig._by_receiver) == 1
-    receivers_bucket = list(sig._by_sender.values())[0]
-    assert len(receivers_bucket) == 1
-    senders_bucket = list(sig._by_receiver.values())[0]
-    assert len(senders_bucket) == 1
+
+    assert senders() == (1, 1)
+    assert receivers() == (2, 1)
 
     sig.disconnect(receiver2, sender=sender)
-    assert len(sig._by_sender) == 0
-    assert len(sig._by_receiver) == 0
+
+    assert senders() == (1, 0)
+    assert receivers() == (2, 0)
+
+    sig._cleanup_bookkeeping()
+    assert senders() == (0, 0)
+    assert receivers() == (0, 0)
 
 
 def test_meta_connect_failure():
@@ -228,7 +230,7 @@ def test_meta_connect_failure():
     assert_raises(TypeError, sig.connect, receiver)
     assert not sig.receivers
     assert not sig._by_receiver
-    assert not sig._by_sender
+    assert sig._by_sender == {blinker.base.ANY_ID: set()}
 
     blinker.receiver_connected._clear_state()