KVO and XIB file madness

You need a mechanism for the window controller to tell the plugin view controller that the window is closing, that isn't tangled up with deallocation.

Ideally you'd provide standard plugin API for this, because from the window controller's point of view any plugin may have cleanup it wants to do and there should be a formal way of doing that.

Note that you crashed because the plugin was using a reference to the window controller (for its observation) but not retaining ownership. If it had, then you wouldn't have crashed but would have produced a retain cycle. Either way, the solution is the same: have a proper API for shutting down the plugin.

There's already a mechanism: NSWindowWillCloseNotification.

Also, it's documented that AppKit will call -viewWillMoveToWindow: down the window's view hierarchy before deallocating the window.

A notification won't work reliably here because the plugin has an unretained reference to the window controller and there's no guarantee that the notification will arrive before the window controller is deallocated.

viewWillMoveToWindow will work, I suppose, and it's a very useful point of intervention (that I often forget about), but the awkward point is that its timing is that of the window/view hierarchy, not of the window controller/view controller hierarchy. It's not impossible, for example, that a window may outlive its window controller (because, IIRC, windows are actually deallocated by the window server process, separately and later than the app code that closes the window).

There may well be a chain of reasoning that guarantees that viewWillMoveToWindow will be invoked before the window controller is deallocated, but I nevertheless think that it's valuable for a plugin API to have an explicit method that indicates when the plugin should shut down. Not necessary, perhaps, but valuable.

> A notification won't work reliably here because the plugin has an unretained reference to the window controller and there's no guarantee that the notification will arrive before the window controller is deallocated.

You're suggesting that the window controller may be deallocated before the window is closed (i.e. while the window is still open)? That doesn't seem likely and, in any case, the window controller can close its window explicitly to force the ordering.

>> You're suggesting that the window controller may be deallocated before the window is closed?

No, I'm suggesting that notifications may be delivered and/or handled asynchronously wrt posting.

Again, I'm suggesting that the exception was due to a bug (the plugin view controller failed to take ownership of its window controller reference), that fixing it would have led a second bug (a retain cycle), and that some means of telling the plugin to break the cycle is necessary. The dependency is between the view controller and the window controller, not the view and the window, and trying to reason about the window controller deallocation timing from the window behavior is treacherous.

All notification handlers are run to completion before the post operation returns. A handler can, of course, submit an asynchronous task as part of its work, but that's under the control of the handler code. In this case, the plugin would not want to delay/defer the unbinding of the array controller's content, so it just wouldn't.

Thanks for the reply.

"You need a mechanism for the window controller to tell the plugin view controller that the window is closing, that isn't tangled up with deallocation."

Actually, I do have a willLoad / didUnload pair of methods that are defined in the plug-in's super class. The didUnload method is called just before the plug-in is going away, and prior to the dealloc call of the WSSiteWIndowController.

But how do I tell the plug-in to effectively release the bindings that were introduced in the NIB file prior to the deallocation of the WSSiteWindowController going away. The opportunity to do this would be in the didUnload method, but I don't know how to force that disconnection, save for one thing:

I have an IBOutlet from my plug-in's code to the NSArrayController in the plug-in's XIB. I can, in the viewWillAppear method, programatically bind the NSArrayController's content key to the 'agent.weatherProperties' key path of the 'self.controller' object. And in the viewWillDisappear, I can unbind programatically.

I tried it, and it works, but it feels like this is something that I shouldn't have to do in code IF the NIB were to break the connections at the right time (before the dealloc of the WSSiteWindowController).

The trouble is that the status of the NIB isn't relevant. The NIB file is an archive of the various objects that make up that piece of your UI. When loading happens (the process that ends with viewDidLoad being invoked), the objects are unarchived, their inter-relationships (i.e. outlets) are re-established, and they are then independent of the NIB. The NIB no longer needs to be around.

So, if you want to break down the inter-relationships between objects that originated in the NIB, you have to do that programmatically.

However, that is not your situation. The troublesome part of your scenario is the relationship between the plugin view controller and the window controller, which is something to establish separately from the NIB. (You said the window controller passes a reference to itself to the plugin.) Because you established this manually, you need to get rid of it manually. You have several choices about how to do so, including at least:

1. In your plugin's didUnload, you can set its reference to the window controller to nil. That will (presumably, since I don't know the exact bindings you're using) propagate to the UI bindings, if it's done KVO compliantly.

2. As in Ken's advice, watch for the NSWindowWillCloseNotification, or override viewWillMoveToWindow: and check for a nil window.

3. Remove the bindings manually.

My problem with #2 is not that it won't work (Ken says they will, and he's always right about this sort of thing), but that they tie the "shutdown" logic to the behavior of the window and view, which I think is a design mistake. For example, suppose that you wanted to add and later remove a second instance of the view managed by the plugin class. If you're tying your shutdown logic to the window closing, you won't have a way of removing the added view properly.

#1 seems to me to have the advantage of not involving the window or view at all, not directly. The only issue there is that your plugin view controller and view need to be able to tolerate a binding that resolves to a nil object (not a problem generally, because the UI is likely already hidden when this happens, and standard controls will cope, but you need to be sure it won't crash) and doesn't do something expensive when the binding changes to nil (such as trying to retrieve megabytes of data from disk or a server).

Quincey,

#1 was the hint I needed! Setting the plug-in's controller property to nil was the key to addressing the issue, and doing so at the willUnload method in the base class did the trick (which means subclasses of my plug-in architecture should always called the super implementation to ensure this code will execute).

Thanks for your help on this! I very much appreciate it.

Actually, I spoke too soon. I am still getting the exceptions.

I think the issue may be how I am specifying the binding in IB. I cannot attach a screenshot to my post here, so I'll describe what I have:

In the XIB file for my plug-in, I have an array controller object whose "Controller Content" section (in the bindings tab) has a field, "Content Array." That field has the "Binds to" checked and "File's Owner" selected. In the Model Key Path field, I have:

self.controller.agent.weatherProperties

The plug-in, when loaded and instantiated by the WSSiteWindowController, gets passed a reference to self.controller. The plug-in's init method keeps the reference to self.controller around in a weak instance variable.

Just setting self.controller to nil in the plug-in's willUnload method is not enought to break the binding.

Other plugins have similar connections to the WSSiteController. For example, another plug-in I have contains about 5 text fields which are disabled and enabled depending upon the state of a flag in WSSiteWindowController. Those five text fields' Availability / Enabled bindings are bound to self.controller.agent.isRunning (a Boolean). So I'm using bindings a LOT in the various plug-ins' XIB files.

Am I misusing bindings in this particular scenario? I would hate to have to manually bind/unbind things in code. It seems so counterintuitive to do that.

First, a key path should generally not include "self.". It's just redundant. (In rare cases, the whole key path may be just "self", but never as part of a longer key path.)

You said earlier that PropertiesPlugIn is a subclass of NSViewController and the NIB is loaded implicitly when you ask for its view property. So, the NIB's owner — the object which will take the place of the File's Owner placeholder — is the view controller itself. OK, so far, so good.

One problem is that your controller property is zeroing weak. Zeroing weak properties are not KVO-compliant and thus not bindings-compliant. That's because they can change (by getting zeroed when the referent is deallocated) without issuing KVO change notifications. You can make it unsafe-unretained, but then, of course, you have additional reason to be careful to clear it manually at the appropriate time.

Next, how do you clear it? You've described this as an instance variable, not a property. You need to change it in a KVO-compliant manner, which generally means you have to invoke its setter rather than just directly modifying the instance variable that backs the property.

What language are you using, Objective-C or Swift? If Swift, then I guess there's no distinction between instance variables and properties. However, you need to mark the property with the "dynamic" keyword for KVO to work.

Apologies for being late on replying. I've been out of town on business.

To your points:

- I'm using Objective-C.

- Good point about self being in the key path. I've taken it out.

- Yes, PropertiesPlugIn is the File's Owner of the NIB that it loads.

- The declaration of the 'controller' property is as follows:

@property (weak) WSSiteWindowController *controller;

Every subclass of WSPlugIn (which itself is a sublcass of NSViewcontroller) inherits this property. (It is not an instance variable as I first indicated, so I apologize for the misrepresentation.)

It is weak because if it were strong, a retain cycle would ensue (the WSSiteWindowController object holds a strong reference to a WSPlugInManager, which itself holds a strong reference to an NSArray of WSPlugIns, of which PropertiesPlugIn is one).

It may help if I outline the relationships here:

- WSSiteWindowController creates an WSPlugInManager (and has a strong reference to it) and passes 'self' to it at init time.

- WSPlugInManager holds onto the WSSiteWindowController reference as a weak reference to WSSiteWindowController.

- WSPlugInManager loads one or more WSPlugIn subclassed bundles and keeps them in the NSArray (strongly reference), and at init time passes 'self.controller' (the WSSiteWindowController reference) to the plug-in. The plug-in keeps around that weak reference to the WSSiteWindowController.

So at this point, WSSiteWindowController is referenced weakly by both the WSPlugInManager that it created, and by the WSPlugIn object that was created by the WSPlugInManager.

I'm thinking it is possible to change the weak WSSiteWindowController references in both the WSPlugInManager and WSPlugIn objects to strong, so long as when the WSSiteWindowController's window closes, a mechanism is put into place to properly tear down the plug-ins AND the WSPlugInManager, correct?

>> It is weak because if it were strong, a retain cycle would ensue

To reiterate:

1. It is a bug to make it weak (because that leads to a crash, which is the horse you rode in on).

2. It is a bug to make it strong without a mechanism to nil out the relationship when the window is closing (because that leads to a retain cycle).

3. A better design would be to make it neither strong nor weak. I mean, remove knowledge of the window controller from the plugin system. "agent.weatherProperties" sounds like pure data model to me, so the MVC relationship you want is "model.agent.weatherProperties", not "controller.agent.weatherProperties".

Another way of putting is, instead of the window controller passing "self" into your plugin system, it should pass "self.model", where "model" is a custom object that has all of the properties needed by plugins that are currently accessed via the window controller.

Although "controller" vs. "model" may seem like a distinction without a difference, the point is that exposing the window controller to the plugins allows them to abuse the window controller, with potentially fatal side effects. (For example, what if a plugin itself decides to store a weak reference to "pluginManager.controller" — for "safety" because it creates no retain cycles — and make bindings through it?)

Quincey,

Ok, I understand what you are saying, and it makes sense. The reason I pass the reference to the window controller is because it holds references to several objects of importance to plugins:

1) There is an agent plug-in that is a property of WSSiteWindowController that plug-ins need to interact with.

2) In some cases, it is desirable for a plug-in to reference the plug-in manager (WSPlugInManager), which is a property of WSSiteWindowController

3) In another case, a plug-in uses the beginSheet:completionHandler: method to display a drop down sheet. The window object (again, referenced from the WSSiteWindowController object) is readily available there.

Your advice is to create a new object that holds references to the things I need (in this case, the WSPlugInManager) and let the plug-ins interact with and bind to objects from that model.

I'll play with this idea and see where I land. Thansk for the advice.

Case 1: Pass the agent to the plugin as a separate parameter. The fact that the window controller is the ultimate source seems irrelevant.

Case 2: Pass the plugin manager to the plugin as a separate parameter. There's still a slight code smell here, because it relies on the plugin to pass the plugin manager up to its superclass init, and not store it separately (because of the danger of a retain cycle in code you don't control). One alternative is to pass an opaque "context" parameter instead, which as a matter of private implementation is (or is an object containing a reference to) the plugin manager, which is then exposed as a superclass property after the superclass init.

Case 3: The plugin can find its window via pluginViewController.view.window. Alternatively, provide a "windowForSheet" method in the plugin manager, which may safely have a private, weak reference to the window controller, if you want to arrange things that way.