I have a general question about memory management of a panel shown as a sheet.

As a general design pattern, I often have a class method that returns a new instance of a NSWindowController subclass for a given sheet. As per the usual rules, this instance is returned autoreleased.

I also usually provide a method in that class that will run the panel as a sheet on a nominated window, with a completion block. This method is essentially a wrapper around -[NSWindow beginSheet:completionHandler:], often with some additional setup private to my class.

The client of this code is typically my document subclass - in response to a user command, it will request a new instance of the panel via the class method, then run it as a sheet, passing its own -windowForSheet: as the parent.

The question I have is, should the panel controller be retained for the duration, or does the sheet display mechanism take care of that? It’s not clear to me who owns the NSWindowController instance while the sheet is in use. Logically it is the document, but because it just puts up the sheet and passes it a completion block (or not - sometimes the completion block is private to the window controller subclass, hidden within the -beginSheet wrapper), there’s nowhere other than within the completion block itself to release the controller. So should the completion block end up calling [self release] as its last step? What implications would that have for a retain loop caused by referencing self within the completion block?

As you can tell, this is a manual memory management question, but even in the ARC world, it’s not clear how ARC would resolve this question either.

—Graham

On Jul 25, 2017, at 19:43 , Graham Cox <graham@...> wrote:

On 26 Jul 2017, at 1:20 pm, Quincey Morris <quinceymorris@...> 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

On Jul 25, 2017, at 21:06 , Graham Cox <graham@...> wrote:

On Jul 25, 2017, at 21:22 , Quincey Morris <quinceymorris@...> wrote:

On 26 Jul 2017, at 2:22 pm, Quincey Morris <quinceymorris@...> wrote:

If I’m reading the code correctly, the window is deterministically kept alive by the block (which captures “self” strongly). The block is kept alive by whatever object is going to invoke the completion handler (which I suspect is NSApp, because I suspect “beginSheet:" ends up at "-[NSApplication runModal:…]"). You don’t have a problem with the window controller being unretained, only with the possibility of a retain cycle, which was what the earlier reasoning was about. So you’re fine.

There’s still some “magic” about blocks I’m not altogethr clear about. When are blocks, as an actual object, created? At what point do they retain their references?

I ask because I wonder if there’s a chance that just leaving it to the block to do the retaining could lead to a window of opportunity for the object (‘self’) that it’s retaining to be autoreleased from out under it. For example, if the call to -beginSheet ends up with NSApp, will it do so after draining the current autorelease pool?

I imagine the answer’s no, and to be sure doing the extra retains myself makes sure, but I just want to know whether my previous code (with no retain/release at all) is susceptible to a race condition of some sort.

There is something of a code smell at writing “[self retain]”

Definitely, which is why I’m making sure what I’m doing is legit.

—Graham

On Jul 25, 2017, at 22:41 , Graham Cox <graham@...> wrote:

According to Mike Ash
(https://www.mikeash.com/pyblog/friday-qa-2009-08-14-practical-blocks.html
- see Caveats) self should be being retained.

Apple (https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Blocks/Articles/bxVariables.html#//apple_ref/doc/uid/TP40007502-CH6-SW3)
also suggest that if you reference a local variable inside the block,
then self will be retained.

I can't see anywhere where they suggest the behavior changes with the
retention-model being used; just that a block doesn't retain anything
until it is copied and in the case where you pass a block in as a
completion handler, it is that methods responsibility to copy the
block.

Jeff Laing <jefflaing@...>
-------------------------------------------------------------------------------
"... because even if we can't um, if we can't rise to his level, no at
least we can, we can drag him down to ours ..."
-- William Gaddis, "JR".