Re: Memory management of document modal panels


Graham Cox
 

On 26 Jul 2017, at 1:20 pm, Quincey Morris <quinceymorris@rivergatesoftware.com> wrote:

Yes to the release, but not “self”.

The receiver for “beginSheet:completionHandler:” is the window on which the sheet is displayed. It (or perhaps the NSApplication object on its behalf) must retain the completion handler block until after it’s executed, but this has nothing to do with the sheet itself.

Furthermore, nothing in this process knows anything about the *sheet’s* window controller. The only referencing properties in Cocoa are NSWindow’s “windowController” and “delegate”, and they’re both non-owning references.

Therefore, you must retain the sheet’s window controller before invoking “beginSheet:completionHandler:”, and releasing the window controller at the end of the completion handler seems like the only reasonable place to balance the retain.

Other than that, I don’t see any danger of a retain cycle. The sheet’s window controller owns the sheet, and (assuming you capture a reference to the window controller inside the block) the block owns the window controller. But there is no owning reference from the sheet back to the block (it doesn’t know the block exists) or to the window controller (unless you incorrectly subclassed the sheet NSWindow and added one). The chain of references is something like:

NSApp (or originating window) -1-> block -2-> sheet window controller -3-> sheet

until the completion handler is called. After it’s called, the reference -1- is manually broken by the invoker of the completion handler, and so the rest of the chain falls apart.

OK, this agrees with my own reasoning - I retain the controller prior to the beginSheet, and release it in the completion block. But there’s no other funny business - no NSWindow subclass or anything like that anywhere.

But it is [self release], in this case - which is why it’s kinda bothering me. Maybe some code will help:

- (void) beginModalWithParentWindow:(NSWindow*) parent forGridLayer:(DKOSquareAnnotatedGrid*) gridLayer
{
[self retain];

self.grid = gridLayer;
self.gridOriginalSettings = gridLayer.settings;

[parent beginSheet:self.window completionHandler:^(NSModalResponse returnCode)
{
//NSLog(@"ended with %ld", returnCode);

if( returnCode == NSModalResponseCancel )
{
[mGridController revertOrCancel];
}
else if ( returnCode == NSModalResponseOK )
{
NSUndoManager* um = self.grid.undoManager;
[[um prepareWithInvocationTarget:self.grid] applySettings:self.gridOriginalSettings];
[um setActionName:NSLocalizedString(@"Index Grid Settings", nil)];
}

[self release];
}];
}


So this is my wrapper method in my NSWindowController subclass, that is called by the document. In this case the document doesn’t supply the block, it’s just enclosed inside the wrapper. I’m assuming that it’s the parent window that retains this block until it is invoked.

You can ignore the response stuff - what’s important is that final [self release], which balances the [self retain] at the top of the method.

n.b. I can confirm that the window controller is deallocated after dismissing the sheet, so it would seem all is well - there can’t be a (hidden) retain cycle.

So what’s the problem? Well, the problem is that previously I wasn’t doing this retain/release at all. The window controller was getting autoreleased but everything *appeared* to work just fine, so *something* was apparently retaining the window controller while it was displaying the sheet. I just couldn’t figure out what that could be, which is why I started to wonder about whether anybody was. It’s possible that it was working by chance, and was a crash waiting to happen.


—Graham

Join cocoa@apple-dev.groups.io to automatically receive all group messages.