#5532: Fix memory leak when a buddy disappears.

7 files changed, 64 insertions, 53 deletions

diff --git a/lib/sugar/graphics/icon.py b/lib/sugar/graphics/icon.py
index d2d71d6..ac151c3 100644
--- a/lib/sugar/graphics/icon.py
+++ b/lib/sugar/graphics/icon.py
@@ -386,6 +386,11 @@ class CanvasIcon(hippo.CanvasBox, hippo.CanvasItem):
         hippo.CanvasBox.__init__(self, **kwargs)
 
         self._palette = None
+        self.connect('destroy', self.__destroy_cb)
+
+    def __destroy_cb(self, icon):
+        if self._palette is not None:
+            self._palette.destroy()
 
     def do_set_property(self, pspec, value):
         if pspec.name == 'file-name':
diff --git a/lib/sugar/graphics/palette.py b/lib/sugar/graphics/palette.py
index a4cef8f..a24a806 100644
--- a/lib/sugar/graphics/palette.py
+++ b/lib/sugar/graphics/palette.py
@@ -148,6 +148,7 @@ class Palette(gtk.Window):
         # Just assume xthickness and ythickness are the same
         self.set_border_width(self.style.xthickness)
         self.connect('realize', self._realize_cb)
+        self.connect('destroy', self.__destroy_cb)
 
         self.palette_state = self.PRIMARY
 
@@ -217,6 +218,12 @@ class Palette(gtk.Window):
         self._mouse_detector = MouseSpeedDetector(self, 200, 5)
         self._mouse_detector.connect('motion-slow', self._mouse_slow_cb)
 
+    def __destroy_cb(self, palette):        
+        self.set_group_id(None)
+        
+        if self._palette_popup_sid is not None:
+            _palette_observer.disconnect(self._palette_popup_sid)
+        
     def _add_menu(self):
         self._menu_box = gtk.VBox()
         self._secondary_box.pack_start(self._menu_box)
diff --git a/lib/sugar/graphics/palettegroup.py b/lib/sugar/graphics/palettegroup.py
index 40d4ca2..bdae76b 100644
--- a/lib/sugar/graphics/palettegroup.py
+++ b/lib/sugar/graphics/palettegroup.py
@@ -68,6 +68,7 @@ class Group(gobject.GObject):
             palette.disconnect(sid)
 
         self._palettes.remove(palette)
+        del self._sig_ids[palette]
 
     def popdown(self):
         for palette in self._palettes:
diff --git a/lib/sugar/presence/buddy.py b/lib/sugar/presence/buddy.py
index 2748299..1030cfc 100644
--- a/lib/sugar/presence/buddy.py
+++ b/lib/sugar/presence/buddy.py
@@ -78,11 +78,16 @@ class Buddy(gobject.GObject):
 
         bobj = bus.get_object(self._PRESENCE_SERVICE, object_path)
         self._buddy = dbus.Interface(bobj, self._BUDDY_DBUS_INTERFACE)
-        self._buddy.connect_to_signal('IconChanged', self._icon_changed_cb,
-                                      byte_arrays=True)
-        self._buddy.connect_to_signal('JoinedActivity', self._joined_activity_cb)
-        self._buddy.connect_to_signal('LeftActivity', self._left_activity_cb)
-        self._buddy.connect_to_signal('PropertyChanged', self._property_changed_cb)
+
+        self._icon_changed_signal = self._buddy.connect_to_signal(
+                'IconChanged', self._icon_changed_cb, byte_arrays=True)
+        self._joined_activity_signal = self._buddy.connect_to_signal(
+                'JoinedActivity', self._joined_activity_cb)
+        self._left_activity_signal = self._buddy.connect_to_signal(
+                'LeftActivity', self._left_activity_cb)
+        self._property_changed_signal = self._buddy.connect_to_signal(
+                'PropertyChanged', self._property_changed_cb)
+
         self._properties = self._get_properties_helper()
 
         activities = self._buddy.GetJoinedActivities()
@@ -90,6 +95,12 @@ class Buddy(gobject.GObject):
             self._activities[op] = self._ps_new_object(op)
         self._icon = None
 
+    def destroy(self):
+        self._icon_changed_signal.remove()
+        self._joined_activity_signal.remove()
+        self._left_activity_signal.remove()
+        self._property_changed_signal.remove()
+        
     def _get_properties_helper(self):
         """Retrieve the Buddy's property dictionary from the service object
         """
diff --git a/lib/sugar/presence/presenceservice.py b/lib/sugar/presence/presenceservice.py
index 97640da..cb47a3a 100644
--- a/lib/sugar/presence/presenceservice.py
+++ b/lib/sugar/presence/presenceservice.py
@@ -156,10 +156,10 @@ class PresenceService(gobject.GObject):
         
         returns presence Buddy or Activity representation
         """
-        _logger.debug('Creating proxy for %s', object_path)
         obj = None
         try:
             obj = self._objcache[object_path]
+            _logger.debug('Reused proxy %r', obj)
         except KeyError:
             if object_path.startswith(self._PS_BUDDY_OP):
                 obj = Buddy(self._bus, self._new_object,
@@ -175,7 +175,7 @@ class PresenceService(gobject.GObject):
             else:
                 raise RuntimeError("Unknown object type")
             self._objcache[object_path] = obj
-        _logger.debug('Proxy is %r', obj)
+            _logger.debug('Created proxy %r', obj)
         return obj
 
     def _have_object(self, object_path):
@@ -200,7 +200,17 @@ class PresenceService(gobject.GObject):
         # Don't try to create a new object here if needed; it will probably
         # fail anyway because the object has already been destroyed in the PS
         if self._have_object(object_path):
-            self.emit('buddy-disappeared', self._new_object(object_path))
+            obj = self._objcache[object_path]
+            self.emit('buddy-disappeared', obj)
+
+            # We cannot maintain the object in the cache because that would keep
+            # a lot of objects from being collected. That includes UI objects
+            # due to signals using strong references.
+            # If we want to cache some despite the memory usage increase,
+            # we could use a LRU cache limited to some value.
+            del self._objcache[object_path]
+            obj.destroy()
+            
         return False
 
     def _buddy_disappeared_cb(self, object_path):
diff --git a/shell/view/BuddyMenu.py b/shell/view/BuddyMenu.py
index a31f623..7f2501d 100644
--- a/shell/view/BuddyMenu.py
+++ b/shell/view/BuddyMenu.py
@@ -16,7 +16,6 @@
 from gettext import gettext as _
 import logging
 
-#import gtk
 import gobject
 import hippo
 
@@ -31,55 +30,31 @@ class BuddyMenu(Palette):
         self._shell = shell
 
         Palette.__init__(self, buddy.get_nick())
-
-# FIXME: re-enable when buddy avatars are re-enabled
-#        pixbuf = None
-#        try:
-#            pixbuf = self._get_buddy_icon_pixbuf()
-#        except gobject.GError, e:
-#            pass
-#        if pixbuf:
-#            scaled_pixbuf = pixbuf.scale_simple(units.grid_to_pixels(1),
-#                                                units.grid_to_pixels(1),
-#                                                gtk.gdk.INTERP_BILINEAR)
-#            del pixbuf
-#            image = gtk.Image()
-#            image.set_from_pixbuf(scaled_pixbuf)
-#            self.set_content(image)
-#            image.show()
+        self._active_activity_changed_hid = None
+        self.connect('destroy', self.__destroy_cb)
 
         self._buddy.connect('icon-changed', self._buddy_icon_changed_cb)
         self._buddy.connect('nick-changed', self._buddy_nick_changed_cb)
 
-        owner = shell.get_model().get_owner()
+        owner = self._get_shell_model().get_owner()
         if buddy.get_nick() != owner.get_nick():
             self._add_items()
 
-# FIXME: re-enable when buddy avatars are re-enabled
-#    def _get_buddy_icon_pixbuf(self):
-#        buddy_object = self._buddy.get_buddy()
-#        if not buddy_object:
-#            return None
-#
-#        icon_data = buddy_object.props.icon
-#        if not icon_data:
-#            return None
-#        pbl = gtk.gdk.PixbufLoader()
-#        pbl.write(icon_data)
-#        pixbuf = None
-#        try:
-#            pbl.close()
-#            pixbuf = pbl.get_pixbuf()
-#        except gobject.GError:
-#            pass
-#        del pbl
-#        return pixbuf
+    def _get_shell_model(self):
+        return self._shell.get_model()
+
+    def _get_home_model(self):
+        return self._get_shell_model().get_home()
+
+    def __destroy_cb(self, menu):
+        if self._active_activity_changed_hid is not None:
+            home_model = self._get_home_model()
+            home_model.disconnect(self._active_activity_changed_hid)
 
     def _add_items(self):
-        shell_model = self._shell.get_model()
         pservice = presenceservice.get_instance()
 
-        friends = shell_model.get_friends()
+        friends = self._get_shell_model().get_friends()
         if friends.has_buddy(self._buddy):
             menu_item = MenuItem(_('Remove friend'), 'list-remove')
             menu_item.connect('activate', self._remove_friend_cb)
@@ -94,9 +69,9 @@ class BuddyMenu(Palette):
         self._invite_menu.connect('activate', self._invite_friend_cb)
         self.menu.append(self._invite_menu)
         
-        home_model = shell_model.get_home()
-        home_model.connect('active-activity-changed',
-                           self._cur_activity_changed_cb)
+        home_model = self._get_home_model()
+        self._active_activity_changed_hid = home_model.connect(
+                'active-activity-changed', self._cur_activity_changed_cb)
         activity = home_model.get_active_activity()
         self._update_invite_menu(activity)
             
@@ -125,11 +100,11 @@ class BuddyMenu(Palette):
         self.set_primary_text(nick)
 
     def _make_friend_cb(self, menuitem):
-        friends = self._shell.get_model().get_friends()
+        friends = self._get_shell_model().get_friends()
         friends.make_friend(self._buddy)    
 
     def _remove_friend_cb(self, menuitem):
-        friends = self._shell.get_model().get_friends()
+        friends = self._get_shell_model().get_friends()
         friends.remove(self._buddy)
 
     def _invite_friend_cb(self, menuitem):
diff --git a/shell/view/home/MeshBox.py b/shell/view/home/MeshBox.py
index ee8599c..ca9c703 100644
--- a/shell/view/home/MeshBox.py
+++ b/shell/view/home/MeshBox.py
@@ -317,8 +317,8 @@ class ActivityView(hippo.CanvasBox):
 
     def remove_buddy_icon(self, key):
         icon = self._icons[key]
-        self.remove(icon)
         del self._icons[key]
+        icon.destroy()
 
     def _clicked_cb(self, item):
         bundle_id = self._model.get_bundle_id()
@@ -535,6 +535,7 @@ class MeshBox(hippo.CanvasBox):
         icon = self._buddies[buddy_model.get_key()]
         self._layout.remove(icon)
         del self._buddies[buddy_model.get_key()]
+        icon.destroy()
 
     def _remove_buddy(self, buddy_model):
         key = buddy_model.get_key()
@@ -575,6 +576,7 @@ class MeshBox(hippo.CanvasBox):
         icon = self._activities[activity_model.get_id()]
         self._layout.remove(icon)
         del self._activities[activity_model.get_id()]
+        icon.destroy()
 
     def _add_access_point(self, ap_model):
         meshdev = self._model.get_mesh()