Fixes for memory management ... to help the menus dealloc properly (/dealloc

at all)


git-svn-id: svn+ssh://svn.gna.org/svn/gnustep/libs/gui/trunk@12700 72102866-910b-0410-8b05-ffd578937521

Source/NSMenuItemCell.m

@@ -54,7 +54,7 @@ static NSImage	*arrowImageH = nil;
 {
   if (self == [NSMenuItemCell class])
     {
-      [self setVersion: 1];
+      [self setVersion: 2];
       colorClass = [NSColor class];
       arrowImage = [[NSImage imageNamed: @"common_3DArrowRight"] copy];
       arrowImageH = [[NSImage imageNamed: @"common_3DArrowRightH"] copy];
@@ -76,9 +76,7 @@ static NSImage	*arrowImageH = nil;
 
 - (void) dealloc
 {
-  RELEASE(_menuItem);
-  RELEASE(_menuView);
-
+  RELEASE (_menuItem);
   [super dealloc];
 }
 
@@ -95,7 +93,7 @@ static NSImage	*arrowImageH = nil;
 
 - (void) setMenuItem:(NSMenuItem *)item
 {
-  ASSIGN(_menuItem, item);
+  ASSIGN (_menuItem, item);
 }
 
 - (NSMenuItem *) menuItem
@@ -105,7 +103,8 @@ static NSImage	*arrowImageH = nil;
 
 - (void) setMenuView:(NSMenuView *)menuView
 {
-  ASSIGN(_menuView, menuView);
+  /* The menu view is retaining us, we should not retain it.  */
+  _menuView = menuView;
   if ([[_menuView menu] _ownedByPopUp])
     {
       _mcell_belongs_to_popupbutton = YES;
@@ -646,28 +645,37 @@ static NSImage	*arrowImageH = nil;
 
   if (_menuItem)
     c->_menuItem = [_menuItem copyWithZone: zone];
-  c->_menuView = RETAIN(_menuView);
+
+  /* We do not copy _menuView, because _menuView owns the old cell,
+     but not the new one!  _menuView knows nothing about c.  If we copy
+     the pointer to _menuView into c, then that pointer might become
+     invalid at any point in time (it never becomes invalid for the original
+     cell because _menuView will call [originalCell setMenuView: nil]
+     when it's being deallocated.  But it will not do the same for c, because
+     it doesn't even know that c exists!)  */
+  c->_menuView = nil;
 
   return c;
 }
 
-//
-// NSCoding protocol
-//
+/*
+ * NSCoding protocol
+ *
+ * Normally unused since the NSMenu encodes/decodes the NSMenuItems, but
+ * not the NSMenuItemCells.
+ */
 - (void) encodeWithCoder: (NSCoder*)aCoder
 {
   [super encodeWithCoder: aCoder];
 
   [aCoder encodeConditionalObject: _menuItem];
-  [aCoder encodeConditionalObject: _menuView];
 }
 
 - (id) initWithCoder: (NSCoder*)aDecoder
 {
   [super initWithCoder: aDecoder];
 
-  _menuItem = [aDecoder decodeObject];
-  _menuView = [aDecoder decodeObject];
+  ASSIGN (_menuItem, [aDecoder decodeObject]);
 
   _needs_sizing = YES;
 

Source/NSMenuView.m

@@ -95,8 +95,12 @@
 - (void) dealloc
 {
   RELEASE(_font);
+
+  /* Clean the pointer to us stored into the _itemCells.  */
+  [_itemCells makeObjectsPerformSelector: @selector(setMenuView:)
+	      withObject: nil];
+
   RELEASE(_itemCells);
-  TEST_RELEASE(_menu);
 
   [super dealloc];
 }
@@ -111,10 +115,11 @@
   if (_menu != nil)
     {
       // Remove this menu view from the old menu list of observers.
-      [theCenter removeObserver: self name: nil object: _menu];
+      [theCenter removeObserver: self  name: nil  object: _menu];
     }
 
-  ASSIGN(_menu, menu);
+  /* menu is retaining us, so we should not be retaining menu.  */
+  _menu = menu;
   _items_link = [_menu itemArray];
 
   // Add this menu view to the menu's list of observers.
@@ -1056,6 +1061,9 @@
 
 /*
  * NSCoding Protocol
+ *
+ * Normally unused because NSMenu does not encode its NSMenuView since
+ * NSMenuView is considered a platform specific way of rendering the menu.
  */
 - (void) encodeWithCoder: (NSCoder*)encoder
 {
@@ -1073,6 +1081,10 @@
   self = [super initWithCoder: decoder];
 
   [decoder decodeValueOfObjCType: @encode(id) at: &_itemCells];
+  
+  [_itemCells makeObjectsPerformSelector: @selector(setMenuView:)
+	      withObject: self];
+
   [decoder decodeValueOfObjCType: @encode(id) at: &_font];
   [decoder decodeValueOfObjCType: @encode(BOOL) at: &_horizontal];
   [decoder decodeValueOfObjCType: @encode(float) at: &_horizontalEdgePad];