Merge lp:~therve/storm/mutable-variables-flush-leak into lp:storm

[Thomas Herve]

[Thomas Herve (therve)]

This another corner-case bug when you use ListVariable or PickleVariable, reported by Jürgen Kartnaller. The fix is fairly trivial (if it's the correct one), the test isn't so great though.

[Jamu Kakar (jkakar)]

I can't think of a better way to test this issue. +1!

review: Approve

[Jürgen Kartnaller (jukart)]

This patch is already in production in a web-service for 5 web sites.

review: Approve

=== modified file 'storm/variables.py'
--- storm/variables.py	2009-03-04 16:56:58 +0000
+++ storm/variables.py	2009-10-03 18:25:19 +0000
@@ -550,7 +550,7 @@
         Variable.__init__(self, *args, **kwargs)
         if self.event is not None:
             self.event.hook("start-tracking-changes", self._start_tracking)
-            self.event.hook("object-deleted", self._detect_changes)
+            self.event.hook("object-deleted", self._detect_changes_and_stop)
 
     def _start_tracking(self, obj_info, event_system):
         self._event_system = event_system
@@ -564,6 +564,11 @@
         if (self._checkpoint_state is not Undef and
             self.get_state() != self._checkpoint_state):
             self.event.emit("changed", self, None, self._value, False)
+    
+    def _detect_changes_and_stop(self, obj_info):
+        self._detect_changes(obj_info)
+        if self._event_system is not None:
+            self._stop_tracking(obj_info, self._event_system)
 
     def get(self, default=None, to_db=False):
         if self._event_system is not None:

=== modified file 'tests/store/base.py'
--- tests/store/base.py	2009-07-31 01:53:08 +0000
+++ tests/store/base.py	2009-10-03 18:25:19 +0000
@@ -421,6 +421,32 @@
         self.store.flush()
         self.assertEquals(len(events), 1)
 
+    def test_wb_flush_event_with_deleted_object_before_flush(self):
+        """
+        When an object is deleted before flush and it contains mutable
+        variables, those variables unhook from the global event system to
+        prevent a leak.
+        """
+        class PickleBlob(Blob):
+            bin = Pickle()
+
+        # Disable the cache, which holds strong references.
+        self.get_cache(self.store).set_size(0)
+
+        blob = self.store.get(Blob, 20)
+        blob.bin = "\x80\x02}q\x01U\x01aK\x01s."
+        self.store.flush()
+        del blob
+        gc.collect()
+
+        pickle_blob = self.store.get(PickleBlob, 20)
+        pickle_blob.bin = "foobin"
+        del pickle_blob
+
+        self.store.flush()
+        self.assertEquals(self.store._event._hooks["flush"], set())
+
+
     def test_obj_info_with_deleted_object_with_get(self):
         # Same thing, but using get rather than find.