Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collision with KVO #70

Open
kzaher opened this issue Nov 29, 2015 · 1 comment
Open

Collision with KVO #70

kzaher opened this issue Nov 29, 2015 · 1 comment

Comments

@kzaher
Copy link

kzaher commented Nov 29, 2015

Hi,

Thnx for this library :)

I have a question about potential issue that I've uncovered while trying to implement something similar in one of my own library.

BaseClass *baseClass = [[BaseClass alloc] init];
[baseClass addObserver:self forKeyPath:@"property" options:0 context:nil];
SubClass *subclass = [[SubClass alloc] init];
[subclass addObserver:self forKeyPath:@"property" options:0 context:nil];

[baseClass aspect_hookSelector:@selector(method) withOptions:0 usingBlock:^(id instance) {
    NSLog(@"class");
} error:nil];

[subclass aspect_hookSelector:@selector(method) withOptions:0 usingBlock:^(id instance) {
    NSLog(@"subclass");
} error:nil];

// Commenting these 2 lines make it to work again because it looks like wrong class is being swizzled.
// KVO will swap `object_getClass` to original after all observers have been removed {
[baseClass removeObserver:self forKeyPath:@"property"];
[subclass removeObserver:self forKeyPath:@"property"];
// }

[subclass method];

... where ...

@interface BaseClass : NSObject

@property (nonatomic, copy) NSString *property;

-(void)method;

@end

@interface SubClass : BaseClass
@end

@implementation BaseClass

-(void)method {
    NSLog(@"called base");
}

@end

@implementation SubClass

-(void)method {
    NSLog(@"called sub");
    [super method];
}

@end

It looks to me that the problem is here

static Class aspect_hookClass(NSObject *self, NSError **error) {
    NSCParameterAssert(self);
    Class statedClass = self.class;
    Class baseClass = object_getClass(self);
    NSString *className = NSStringFromClass(baseClass);

    // Already subclassed
    if ([className hasSuffix:AspectsSubclassSuffix]) {
        return baseClass;

        // We swizzle a class object, not a single object.
    }else if (class_isMetaClass(baseClass)) {
        return aspect_swizzleClassInPlace((Class)self);
        // Probably a KVO'ed class. Swizzle in place. Also swizzle meta classes in place.
    }else if (statedClass != baseClass) {
        return aspect_swizzleClassInPlace(statedClass); // <-- I believe this should be used
        //return aspect_swizzleClassInPlace(baseClass);
    }

If I understand this code correctly, if target object is already being swizzled (like with KVO), the "acted" class should be used because otherwise class returned with object_getClass could be swapped and changed again, thus killing installed observer (like it happens in that example that I've pasted).

It also seems to me that it should be

Class baseClass = self.class;
Class subclass = object_getClass(self);

... instead of ..

Class statedClass = self.class;
Class baseClass = object_getClass(self);

Is this a bug or am I doing something wrong?

Kruno

@doggy
Copy link

doggy commented Mar 3, 2017

Class statedClass = self.class;
Class actualClass = object_getClass(self);

It should be named as above I think.

P.S. You can take a look at PR #115 if you need KVO coexistence.

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

No branches or pull requests

2 participants