Document problems with automatic deregistration and "deep" keypaths

[lilyball]

Experimentally, it seems that when using "deep" keypaths (keypaths with multiple components), automatic deregistration doesn't work properly. The problem is that by the time the observed object deallocates (and therefore cleans up the observer), the leaf object has already deallocated, and so it throws an exception when trying to unregister the KVO. This pattern should work if the observed object outlives self (assuming you use observer: self), or if the observed object deallocates prior to the next object in the path, but in the common case the observed object owns the deeper object and so doesn't deallocate in time.

AFAIK the only way to truly fix this is to completely sidestep KVO's built-in handling of "deep" keypaths and rebuild that ourselves, so that way we can attach the dealloc spy to every component of the path. This is a lot of work and would be very tricky to get right so I'm reluctant to do this. Alternatively we could actually isa-swizzle the observed object in order to clean up KVO at the start of dealloc instead of the end, but that's really nasty.

In the meantime, we should document this scenario.

[AlexGingell]

I came here to post an issue about exactly this! Great to see that you guys already recognise the problem.

The nice thing about deep or compound keypaths is that KVO fires if any component of the chain changes, which is why I'm using them in certain cases. Given that I see exceptions when the leaf objects deallocate prior to the observed object (which happens in a percentage of cases), and that your dealloc spies are only placed on observed and observing objects and not each component in the keyPath chain, I am going to rethink and perhaps split my KVO up to observe each object in the chain individually now.

For what it's worth, I only see these exceptions on iOS 10 and prior. I see no crashes at all of this nature on iOS 11 and 12, which would lead me to believe that they changed something under the KVO hood from iOS 11 on.

[lilyball]

Depending on the ownership model in your app, you might get away with simply deregistering manually in deinit/dealloc and not relying on automatic deregistration. Which is to say, if the root object of your key path is something that your object has a strong reference to, manual deregistration in deinit/dealloc ensures you've torn down the KVO while that object (and, presumably, the rest of the objects in the key path) is still alive.

[AlexGingell]

It's a good point, and I actually was doing something like this.

For clarity and by way of example, I have an 'editor' object, with a 'composition' property, which holds a 'currentLayers' array. i.e. the object structure is:editor.composition.currentLayers

I was PMKVObserving the "composition.currentLayers" key path on the editor so that the relevant classes updated whether currentLayers changed, or a new composition was loaded.

I initially assumed that I didn't have to manually deregister as PMKVObserver would take care of it - I had assumed that currentLayers would be the first thing to deallocate. Not so - it seems it was possible for 'composition' to deallocate before 'editor' or 'currentLayers' (clearly some dependence on my overall implementation and perhaps something held a strong reference to currentLayers for example). I saw: "composition was deallocated while key value observers were still registered with the key path 'currentLayers'".

I tried to ensure KVO was removed in dealloc methods but it is not failsafe. From prior discussions with Apple engineers, they recommend deregistering way ahead of dealloc - as soon as you know the object is no longer needed.

Ok I thought. Now when the user exits the editor, it runs a 'willExitEditor' method in which all observing objects are told deregister KVO, before the editor relinquishes its strong reference to its composition. This cleared up the vast majority of exceptions but, surprisingly, on iOS 9 and 10 I still see occurrences of "composition was deallocated while key value observers were still registered with the key path 'currentLayers'". I literally have no idea how that can occur, but as it's limited to iOS 9 and 10 only, something must have changed within KVO to affect this since iOS 11.

To clear up those last issues, I've split up the observation such that I'm PMKVObserving the editor's 'composition' key path, and the composition's 'currentLayers' key path completely separately. My hope is that PMKVObserver's dealloc spies will solve these last exceptions. Clearly it's somehow possible for 'composition' to deallocate while a currentLayers observer is still registered, hopefully the PMKVObserver dealloc spies will mitigate the issue.

I have to say, I really try to avoid KVO because I think it's deeply flawed within iOS, but when I do elect to use it I will only do it through PMKVObserver - I think this is a magical library and appreciate the work you do.

[lilyball]

KVO is a fascinating technology that is also deeply flawed. I'm glad my library is of help, and I'm sorry it can't do more.

Based on what you've described, it certainly sounds to me as though cancelling the KVO from the top of your editor's dealloc/deinit method should work, since it should still be holding the composition, which should still be holding the currentLayers. But of course cleaning it up even earlier would also work, which it sounds like you're doing now.

If you're actually cleaning it up early, then I have no explanation for why iOS 9/10 is giving you that warning. If I were in your shoes I'd verify that the KVO really was cleaned up prior to entering dealloc/deinit.