Mon 12 Jan 2009 11:28:10 AM UTC, comment #6:
Greg,
I think your change is just equivalent to mine, as far as the retain count is concerned. But not copying something is always great.
The problem with your changes is that they als result in the the same problems that I had after mine. When you start up Bean, create a new document, close that and again start a new document. Now closing that from the close icon in the window decoration will result in the following backtrace (windows decoration drawn by GNUstep):
0xb7761734 in objc_msg_lookup () from /usr/lib/libobjc.so.2
(gdb) bt
#0 0xb7761734 in objc_msg_lookup () from /usr/lib/libobjc.so.2
#1 0x08084476 in -[MyDocument windowResignedMain:] (self=0x86e7f98,
_cmd=0x80a3818, sender=0x82d2128) at MyDocument.m:7870
#2 0xb791319d in -[NSNotificationCenter _postAndRelease:] (
self=0x80cfec8, _cmd=0xb7b28548, notification=0x82d2128)
at NSNotificationCenter.m:1070
#3 0xb7913ab2 in -[NSNotificationCenter postNotificationName:object:userInfo:] (self=0x80cfec8, _cmd=0xb7b28550, name=0xb7f202c0, object=0x8508558,
info=0x0) at NSNotificationCenter.m:1129
#4 0xb7913969 in -[NSNotificationCenter postNotificationName:object:] (
self=0x80cfec8, _cmd=0xb7f122a0, name=0xb7f202c0, object=0x8508558)
at NSNotificationCenter.m:1109
#5 0xb7dbe111 in -[NSWindow resignMainWindow] (self=0x8508558,
_cmd=0xb7f11e38) at NSWindow.m:1775
#6 0xb7db895c in -[NSWindow(GNUstepPrivate) _lossOfKeyOrMainWindow] (
self=0x8508558, _cmd=0xb7f12320) at NSWindow.m:313
#7 0xb7dbd8bb in -[NSWindow orderWindow:relativeTo:] (self=0x8508558,
_cmd=0xb7f12318, place=NSWindowOut, otherWin=0) at NSWindow.m:1653
#8 0xb7dbd810 in -[NSWindow orderOut:] (self=0x8508558, _cmd=0xb7f124b0,
sender=0x8508558) at NSWindow.m:1613
#9 0xb7dc1b45 in -[NSWindow close] (self=0x8508558, _cmd=0xb7f12528)
at NSWindow.m:2644
#10 0xb7dc245a in -[NSWindow performClose:] (self=0x8508558,
_cmd=0xb7f12580, sender=0x8508850) at NSWindow.m:2824
#11 0xb7a147df in L10 ()
from /usr/GNUstep/System/Library/Libraries/libgnustep-base.so.1.19
#12 0x08508558 in ?? ()
#13 0xb79d4e1f in GSFFCallInvokeWithTargetAndImp (_inv=0x81ecaf0,
anObject=0x8508558, imp=0xb7dc223a <-[NSWindow performClose:]>)
at GSFFCallInvocation.m:709
#14 0xb79d51e8 in -[GSFFCallInvocation invokeWithTarget:] (
self=0x81ecaf0, _cmd=0xb7e78a70, anObject=0x8508558)
at GSFFCallInvocation.m:779
#15 0xb7c30e72 in -[NSApplication sendAction:to:from:] (self=0x81e2b28,
_cmd=0xb7e9a270, aSelector=0xb7f12580, aTarget=0x8508558,
sender=0x8508850) at NSApplication.m:2034
#16 0xb7c94a3b in -[NSControl sendAction:to:] (self=0x8508850,
_cmd=0xb7e8c098, theAction=0xb7f12580, theTarget=0x8508558)
at NSControl.m:730
#17 0xb7c6c090 in -[NSCell trackMouse:inRect:ofView:untilMouseUp:] (
self=0x8508900, _cmd=0xb7e9a318, theEvent=0x851a868, cellFrame=
{origin = {x = 0, y = 0}, size = {width = 15, height = 15}},
controlView=0x8508850, flag=0 '\000') at NSCell.m:1693
#18 0xb7c952e1 in -[NSControl mouseDown:] (self=0x8508850,
_cmd=0xb7f12758, theEvent=0x839b868) at NSControl.m:886
#19 0xb7dc4dfe in -[NSWindow sendEvent:] (self=0x8508558,
_cmd=0xb7e78a10, theEvent=0x839b868) at NSWindow.m:3560
#20 0xb7c3086b in -[NSApplication sendEvent:] (self=0x81e2b28,
_cmd=0xb7e78950, theEvent=0x839b868) at NSApplication.m:1915
#21 0xb7c2f32d in -[NSApplication run] (self=0x81e2b28, _cmd=0xb7e6e020)
at NSApplication.m:1415
#22 0xb7c1114d in NSApplicationMain (argc=1, argv=0xbf966914)
at Functions.m:74
#23 0x08094576 in main (argc=Cannot access memory at address 0xe0075
) at main.m:13
But the original problem is definitily gone, so I will close this bug report.
|
Mon 12 Jan 2009 01:52:57 AM UTC, comment #5:
Fred,
This change...
"ASSIGN(_connections, [[coder decodeObjectForKey: @"NSConnections"] mutableCopy]);
replacing it with
_connections = [[coder decodeObjectForKey: @"NSConnections"] mutableCopy];"
...is incorrect, I believe. I changed this to:
ASSIGN(_connections, (NSMutableArray *)
[coder decodeObjectForKey: @"NSConnections"])
and it seems to work fine.
In the dealloc the retain count for _connections is 2 only because the NSEnumerator which was used to iterate over it in the awakeWithContext: method has not yet been released and dealloc'd.
After this change the NSIBOutletConnectors are being freed. I have attached a snapshop of Bean's memory panel.
I am still testing this. I committed all of the changes discused above, plus the one you suggested in nibInstatiateWithInstantiator:.
I am going to continue to document the code and should be done with all of that by tonight.
Thanks, GC
(file #17250)
|
Sun 11 Jan 2009 10:09:39 PM UTC, comment #3:
With re-work I just meant that my current bug fixing approach to the problem may not be sufficient, but perhaps I just gave up one patch too early?
You see, I am very sure that both my patches are correct, but currently things get worse when applying them. So perhaps to whole concept in this file is wrong.
What we need to do here, as far as I understand it is
- Code complex NIB files correctly
- Allow for the process to replace objects while instantiating them (Normally via awakeFromNib)
- Don't leak any memory.
The current implemantion does great on the first two items, it falls short on the third one. This may well be because the second should be implemented differently. But this is hard to tell without an outlined concept for the current implementation. Either that concept could be wrong or just the implementation.
|
Fri 09 Jan 2009 06:40:08 PM UTC, comment #1:
Finding the root cause was easy, the following line in GSNibLoading.m was leaking the copied array
ASSIGN(_connections, [[coder decodeObjectForKey: @"NSConnections"] mutableCopy]);
replacing it with
_connections = [[coder decodeObjectForKey: @"NSConnections"] mutableCopy];
solves this. But now there are problems with the freed up objects. One of these problems I could trace down to this method:
@implementation NSNibConnector (NibCompatibility)
- (void) instantiateWithInstantiator: (id<GSInstantiator>)instantiator
{
_src = [instantiator instantiateObject: _src];
_dst = [instantiator instantiateObject: _dst];
}
@end
When the instantiated object is different from the original one, then one is leaked whereas the other has no retain. Replacing this with the following solves the issue:
[self setSource: [instantiator instantiateObject: _src]];
[self setDestination: [instantiator instantiateObject: _dst]];
Still there are many more places where now already freed objects get released. All of this was hidden by the original memory leak.
At this point I gave up, my impression is that currently the code in GSNibLoading needs a full rework. And with the current lack of documentation in that class it would take me to long to understand the different concepts used here.
|