Skip to content
This repository has been archived by the owner on Mar 3, 2020. It is now read-only.

Automatic removal of observation for not retained observed objects. #94

Open
nlutsenko opened this issue Mar 16, 2016 · 11 comments
Open

Comments

@nlutsenko
Copy link
Contributor

This is a follow-up issue for #48 for discussions and actual implementation of automatic removal of any observation registered via KVOController when the observed objects are not retained in any way.
The story here is that unless you let KVOController retain the objects - all of the observation becomes as fragile as it is with vanilla Cocoa KVO APIs.

Taking into account that we all would love to observe things in a "weak" way - there are 2 ways one could implement such a thing:

  • Swizzle dealloc and unregister registered observers or KVOControllers there. Sounds like less than ideal solution, but this will work just as fine. It feels very very fragile, but is a supported use case for ObjC runtime.
  • Use a separate instance of a custom class to box the original object into a property and store all of the observation info inside of it instead of a custom map table inside KVOController. As well as add all of the KVO observation with extra prefix path component whenever KVOController is registering an observer. Then store this instance via associated objects on the observed object.
    This will allow both weak reference to the original object, as well as to know the moment where we need to unregister KVO, as associated objects are cleaned up automatically on dealloc.

Do you have an idea/suggestion/comment? Great! Please post it here!
Do you want to take a chance on implementing this? Amazing! Pull requests are very very welcome, and I'll do my best to review/guide/help/drive any implementation of this.

@k06a
Copy link

k06a commented Mar 16, 2016

Swizzling dealloc sounds horrible, lets try to box observable object and hope KVO will handle whole keypath mutations including objects deallocation :)

@joerick
Copy link

joerick commented Mar 16, 2016

Hey @nlutsenko, I've come here from #67.

I'd feel a bit nervous about the swizzling of dealloc too, especially as ARC does some strange stuff there as well (i.e. ARC automatically calls [super dealloc] somehow). Still, might work.

Your second point sounds great, but I'm not 100% sure how it would work. Specifically doing the removeObserver:forKeyPath: call at the right time, just before dealloc.

I tried a proof-of-concept here: https://gist.github.com/joerick/a0b87607e69071fa6a3c

In my example the observer knows the person dealloc'd because the observer's dealloc gets called. Here we're relying on

  • an implementation detail that associated objects are released before the KVO exception is raised
  • there's only one strong reference to the observer

I'm not 100% confident these are reliable assumptions, the second feels a bit race-conditiony.

Any other ideas of how to call the -removeObserver:forKeyPath: at the right time?

@nlutsenko
Copy link
Contributor Author

That proof of concept actually looks fine, since the assumptions here are correct. I would expect associated objects being cleared before KVO is checked on dealloc, since it literally needs to deconstruct all dependents (all ivars/instance objects) before being able to check for KVO.

One strong reference to the observer - this is expected and is required for this to work.

@k06a
Copy link

k06a commented Mar 17, 2016

Looks like using long keypaths to KVO-observing creates chain on observers and also manages of automagically unsubscribing objects in this chain while chain mutations like this:

  1. Assume we observe self for keypath viewModel.bestFriend.name:
    • KVO makes self.viewModel.bestFriend object to observe keypath name
    • KVO makes self.viewModel object to observe keypath bestFriend
    • KVO makes self object to observe keypath viewModel
  2. Then we mutate our chain like this self.viewModel = model;:
    • KVO makes old self.viewModel.bestFriend to automagically unobserve keypath name
    • KVO makes old self.viewModel to automagically unobserve keypath bestFriend
    • KVO makes new self.viewModel.bestFriend to observe keypath name
    • KVO makes new self.viewModel object to observe keypath bestFriend

And idea is to make something like this:

[self.KVOControllerNonRetaining observe:self
                                keyPath:@"viewModel.title"
                                options:(NSKeyValueObservingOptionInitial|NSKeyValueObservingOptionNew)
                                  block:^(id observer, id object, NSDictionary<NSString *,id> * change)
{
    //
}];

=>

[self.KVOControllerNonRetaining observe:self.KVOControllerNonRetaining
                                keyPath:@"observer.viewModel.title"
                                options:(NSKeyValueObservingOptionInitial|NSKeyValueObservingOptionNew)
                                  block:^(id observer, id object, NSDictionary<NSString *,id> * change)
{
    //
}];

But looks like exactly this not works because of weak in keypath chain.

@k06a
Copy link

k06a commented Mar 17, 2016

Here is good answer, explaining KVO keypath observing chain implementation: http://stackoverflow.com/a/16533561/440168

@joerick
Copy link

joerick commented Mar 17, 2016

@k06a, that's a great idea! So we can avoid the dealloc stuff entirely, if we observe across a weak property and we get a KVO notification nil, we test the property- if it's nil, the object's gone.

@arjunaud
Copy link

Can any one please tell whether this enhancement is completed or still under development?

@k06a
Copy link

k06a commented Sep 4, 2016

Recently tried different type of automatic unobserving KVO:

Swizzled addObserver:forKeyPath:options:context:, removeObserver:forKeyPath:context: and removeObserver:forKeyPath: to remember observing keypath/target/context in associated object. And this associated object should unobserve original object on dealloc. The key moment of this trick is not to store original object pointer as weak because it is already nilled in associated object's dealloc, assign works fine. Full code is here: https://github.com/ML-Works/KVO-MVVM/blob/master/Pod/Classes/KVO-MVVM-Private.m

I don't really like swizzling those 3 methods, but this allowed to forget about unobserving and allowed to observe weaks in the middle of keypaths. Before supporting weaks in keypath we used asserts to check keypath for weak properties, look at CheckClassKeyPathForWeaks function: https://github.com/ML-Works/KVO-MVVM/blob/0.3.1/Pod/Classes/NSObject%2BMVVMKVOPrivate.m#L17 I think this may be useful if not to support weaks in keypaths in KVOController.

UPDATE: Updated urls

@SolaWing
Copy link

SolaWing commented Sep 9, 2016

Just point out: associated objects are cleaned up automatically on NSObject's dealloc. It's fine for single key. but for key path, it's a little later.
Suppose Observing UIView's layer.bounds key path, when UIView's associated cleaned up in NSObejct's dealloc, layer already deallocd in UIView's dealloc, and will crash.
So if use associated objects version, we need to create a associated object for each object in key paths. This also sounds horrible.

@k06a
Copy link

k06a commented Sep 9, 2016

@SolaWing exactly. Associated object for each object in keyPath works, but horrors a little bit :)

@k06a
Copy link

k06a commented Sep 22, 2016

@nlutsenko what do you think?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants