Am 02.03.2010 09:24, schrieb Wolfgang Lux:
Author: fredkiefer
Date: Sun Feb 8 13:54:21 2009
New Revision: 27812
URL: http://svn.gna.org/viewcvs/gnustep?rev=27812&view=rev
Log:
Use KVC call setValue:forKey: to establish the outlet connection.
This will result in ivars being properly retained.
Modified:
libs/gui/trunk/ChangeLog
libs/gui/trunk/Source/NSBundleAdditions.m
it's been a while since you've committed this change, but I just got
round yesterday evening to track down a major space leak to exactly this
change: Windows (and apparently other entities) loaded from a nib file
never get released and deallocated. As an example, try this with Ink:
- Open the memory panel from the info panel.
- Open a new document window.
- Update the memory panel. The panel now shows one (new) NSWindow, one
(new) Document, etc.
- Close the document window.
- Update the memory panel. The Document and NSWindowController instance
are gone, the NSWindow is still alive.
If you revert your change, the NSWindow and its views are properly
released.
At first, I was assuming that your change is correct and the nib loading
code was improperly retaining the top-level entities. However, the
longer I think about it the more I am convinced that it is your change
that is wrong. In fact we do *not* want to retain ivars in general. It
is well documented by Apple that only ivars that reference top-level
entities of a nib are owned by the receiver (and this is already handled
properly by the nib loading code), other ivars are not owned. An example
for this is the tv attribute of Ink's Document objects. This attribute
must not be retained when the connection is instantiated. Otherwise, the
text view would not be released even when the window were released and
deallocated when its document is closed. [1]
Wolfgang
[1] You can trivially test this without reverting your change by adding
the statement [[aController window] autorelease] to the Document
-windowControllerDidLoadNib: method. Now the window is deallocated when
it is closed, but its text view still remains alive.
Thank you for looking into this change. My main problem with it is that
it is already a year ago that I made it, so my memory is getting week on
this.
As far as I remember this change was about replacing the usage of
GNUstep base private functions with a proper official interface. Looking
at the diff this seems to be the case. Instead of having
NSString *selName;
SEL sel;
selName = [NSString stringWithFormat: @"address@hidden@:",
[[_tag substringToIndex: 1] uppercaseString],
[_tag substringFromIndex: 1]];
sel = NSSelectorFromString(selName);
if (sel && [_src respondsToSelector: sel])
{
[_src performSelector: sel withObject: _dst];
}
else
{
const char *nam = [_tag cString];
const char *type;
unsigned int size;
int offset;
/*
* Use the GNUstep additional function to set the instance
* variable directly.
* FIXME - need some way to do this for libFoundation and
* Foundation based systems.
*/
if (GSObjCFindVariable(_src, nam, &type, &size, &offset))
{
GSObjCSetVariable(_src, offset, size, (void*)&_dst);
}
}
we now have
[_src setValue: _dst forKey: _tag];
Even the old code was calling setter methods instead of setting an ivar
directly so retains could happen there as well. The main difference is
in the case where we have an object ivar and there is no accessor
method. There the old code would just set the ivar to the value and the
new code now does a retain in GSObjCSetVal().
I see this as a general problem of KVC, as long as GNUstep doesn't
supports ways to tell the compiler which ivars need to be retained when
setting their value, we need another mechanism to get this correct. The
only thing I can think of at the moment is to add a setter method for
these cases that wont use ASSIGN.
I also found some discussion on the dev mailing list triggered by change
r27706. I already recommended the same solution at that time, but never
implemented it myself :-(
_______________________________________________
Gnustep-dev mailing list
address@hidden
http://lists.gnu.org/mailman/listinfo/gnustep-dev