KVO not working correctly when observing NSProxy objects

[triplef]

Observing a property of an NSProxy object does not work with GNUstep, but works on Apple platforms.

Running runTest below prints the following on Apple platforms:

Changed <TObject: 0x600002d0c010> 'derivedName': Derived MOO
Changed <TObject: 0x600002d0c010> 'name': MOO
Changed <TObject: 0x600002d0c010> 'derivedName': Derived BAH
Changed <TObject: 0x600002d0c010> 'name': BAH

But only this with GNUstep:

Changed <TObject: 0x7828e2a7e8> 'derivedName': Derived BAH
Changed <TObject: 0x7828e2a7e8> 'name': BAH

Reproducer:

@interface TObject : NSObject
@property (copy) NSString *name;
@property (readonly) NSString *derivedName;
@end

@implementation TObject

+ (NSSet<NSString *> *)keyPathsForValuesAffectingDerivedName
{
	return [NSSet setWithObject:@"name"];
}

- (NSString *)derivedName
{
	return [NSString stringWithFormat:@"Derived %@", self.name];
}

@end

@interface TProxy : NSProxy
@property id proxiedObject;
@end

@implementation TProxy

- (instancetype)initWithProxiedObject:(id)proxiedObject
{
	self.proxiedObject = proxiedObject;
	return self;
}

- (void)forwardInvocation:(NSInvocation *)invocation
{
	[invocation invokeWithTarget:_proxiedObject];
}

- (NSMethodSignature *)methodSignatureForSelector:(SEL)sel
{
	return [_proxiedObject methodSignatureForSelector:sel];
}

@end

@interface TDriver: NSObject

@property (strong) TProxy *proxy;
@property (strong) TObject *obj;

- (void)runTest;

@end

@implementation TDriver

- (void)runTest
{
	self.obj = [[TObject alloc] init];
	self.proxy = [[TProxy alloc] initWithProxiedObject:self.obj];
	
	[(TObject *)self.proxy addObserver:self forKeyPath:@"name" options:NSKeyValueObservingOptionNew context:NULL];
	[(TObject *)self.proxy addObserver:self forKeyPath:@"derivedName" options:NSKeyValueObservingOptionNew context:NULL];
	
	// This does not work
	((TObject *)self.proxy).name = @"MOO";
	
	// This works
	self.obj.name = @"BAH";
	
	[(TObject *)self.proxy removeObserver:self forKeyPath:@"name" context:NULL];
	[(TObject *)self.proxy removeObserver:self forKeyPath:@"derivedName" context:NULL];
}

- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context
{
	NSLog(@"Changed %@ '%@': %@", object, keyPath, change[NSKeyValueChangeNewKey]);
}

@end

[triplef]

With our codebase we’re running into another issue that may be related to the one outlined above, where the KVO implementation calls +keyPathsForValuesAffectingValueForKey: on the NSProxy subclass (from _addNestedObserversAndOptionallyDependents()), but since the method is implemented in an NSObject category this will cause an exception.

However, this issue is not reproduced with the code above. I’m guessing it may require a deeper chain of nested observers.

[rfm]

I never use KVO, and am not really in a position to comment on the new libobjc2 based implementation though I can add a fix if you can supply one, but as a temporary workaround I've added a configure option --disable-newkvo to let you easily work with the old implementation.

[triplef]

Thanks @rfm, I appreciate it. To be honest though I’d be surprised if this worked correctly with the old implementation, and in any case the old implementation has too many other limitations to be useable for us at all.

I just wanted to document the issue, @hmelder will take a look. 🙏

[hmelder]

Turns out that the we skip the hidden class when looking up the target IMP in GSFFIInvocation, calling the original IMP instead of the swizzled one.

[hmelder]

libs-base/Source/Additions/GSObjCRuntime.m

Lines 568 to 617 in 9cdb4f9

	GSGetMethod(Class cls, SEL sel,
	BOOL searchInstanceMethods,
	BOOL searchSuperClasses)
	{
	if (cls == 0 || sel == 0)
	{
	return 0;
	}
	if (searchSuperClasses == NO)
	{
	unsigned int count;
	Method method = NULL;
	Method *methods;
	if (searchInstanceMethods == NO)
	{
	methods = class_copyMethodList(object_getClass(cls), &count);
	}
	else
	{
	methods = class_copyMethodList(cls, &count);
	}
	if (methods != NULL)
	{
	unsigned int index = 0;
	while ((method = methods[index++]) != NULL)
	{
	if (sel_isEqual(sel, method_getName(method)))
	{
	break;
	}
	}
	free(methods);
	}
	return method;
	}
	else
	{
	if (searchInstanceMethods == NO)
	{
	return class_getClassMethod(cls, sel);
	}
	else
	{
	return class_getInstanceMethod(cls, sel);
	}
	}
	}

object_getClass and class_getInstanceMethod skip the hidden classes.

[hmelder]

Works when using objc_msg_lookup in -[GSFFIInvocation invokeWithTarget:] instead of GSGetMethod:

2024-12-02 14:21:21.701 a.out[73446:73446] After adding observer: obj isa=0xaaab18e288c0
2024-12-02 14:21:21.701 a.out[73446:73446] forwardInvocation: Selector: setName: target: 0xaaab18c4fd38
2024-12-02 14:21:21.701 a.out[73446:73446] Looking up instance method for cls=0xaaaae56e0238 sel=setName:
2024-12-02 14:21:21.701 a.out[73446:73446] -[GSFFIInvocation invokeWithTarget:] imp = 0xffff9eeea4dc impOrg = 0xaaaae56c25a0
_addNestedObserversAndOptionallyDependents dependents=0



2024-12-02 14:21:21.702 a.out[73446:73446] Changed <TObject: 0xaaab18c44938> 'derivedName': Derived MOO
_addNestedObserversAndOptionallyDependents dependents=0
2024-12-02 14:21:21.702 a.out[73446:73446] Changed <TObject: 0xaaab18c44938> 'name': MOO



2024-12-02 14:21:21.702 a.out[73446:73446] forwardInvocation: Selector: removeObserver:forKeyPath:context: target: 0xaaab18c4fd38
2024-12-02 14:21:21.702 a.out[73446:73446] Looking up instance method for cls=0xaaaae56e0238 sel=removeObserver:forKeyPath:context:
2024-12-02 14:21:21.702 a.out[73446:73446] -[GSFFIInvocation invokeWithTarget:] imp = 0xffff9eee616c impOrg = 0xffff9eee616c
2024-12-02 14:21:21.702 a.out[73446:73446] forwardInvocation: Selector: removeObserver:forKeyPath:context: target: 0xaaab18c4fd38
2024-12-02 14:21:21.702 a.out[73446:73446] Looking up instance method for cls=0xaaaae56e0238 sel=removeObserver:forKeyPath:context:
2024-12-02 14:21:21.702 a.out[73446:73446] -[GSFFIInvocation invokeWithTarget:] imp = 0xffff9eee616c impOrg = 0xffff9eee616c
hmelder@gnustep:~$

[hmelder]

See #473

[hmelder]

Turns out that when you observe a property from a NSProxy-based object in a nested keypath, i.e. proxy.<KEY>, the KVO machinery fetches the new value with [proxy valueForKey: ] and checks if there are other keys with [proxy.class keyPathsForValuesAffectingValueForKey:].

As keyPathsForValuesAffectingValueForKey: is a class method, the receiver passed to objc_msgSend is not an object but the metaclass of proxy. This leads to a forwarding chain:

1. objc_msgSend calls SlowMsgLookup
2. The inlined objc_msg_lookup_internal cannot find the selector in the dtable
3. objc_msg_lookup_internal calls the objc_proxy_lookup hook first (gnustep-base implements this in gs_objc_proxy_lookup)
4. objc_proxy_lookup returns nil
5. objc_msg_lookup_internal then proceeds and calls __objc_msg_forward2 (hooked to gs_objc_msg_forward2)
6. gs_objc_msg_forward2 extracts type information from the selector and creates an NSInvocation object
7. gs_objc_msg_forward2 calls [recv forwardInvocation:] with the invocation object as argument
8. Remember that recv is actually a metaclass. The message send machinery ones again tries to find an IMP, goes up the super class, and calls -[NSProxy forwardInvocation:] (not 100% sure why it calls an instance method on a metaclass)
9. Boom!

Here a minimal reproducable:

#import <Foundation/Foundation.h>


@interface Proxy : NSProxy

@property (nonatomic, strong) id object;

- (instancetype) initWithObject:(id)object;

- (void)forwardInvocation:(NSInvocation *)invocation;

- (NSMethodSignature *)methodSignatureForSelector:(SEL)sel;

@end

@implementation Proxy

+ (NSSet *) keyPathsForValuesAffectingValueForKey: (NSString *)key {
	NSLog(@"keyPathsForValuesAffectingValueForKey called on proxy");
	return [NSSet set];
}

- (instancetype) initWithObject:(id)object {
	_object = object;
	return self;
}

- (void)forwardInvocation:(NSInvocation *)anInvocation
{
    [anInvocation setTarget:_object];
    [anInvocation invoke];
    return;
}

- (NSMethodSignature *)methodSignatureForSelector:(SEL)aSelector
{
    return [_object methodSignatureForSelector:aSelector];
}

@end

@interface Observee : NSObject

@property (nonatomic, strong) NSString *firstName;
@property (nonatomic, strong) NSString *lastName;

- (NSString *) name;

@end

@implementation Observee

+ (NSSet *)keyPathsForValuesAffectingName {
	return [NSSet setWithArray: @[ @"firstName", @"lastName"  ]];
}


- (NSString *) name {
  return [NSString stringWithFormat: @"%@ %@", _firstName, _lastName];
}

@end


@interface Encapsulation : NSObject

@property (nonatomic, strong) Proxy *proxy;

- (instancetype) init;


@end

@implementation Encapsulation
- (instancetype) init {
	self = [super init];
	if (self) {
		_proxy = [[Proxy alloc] initWithObject: [Observee new]];
	}

	return self;
}
@end

@interface Observer : NSObject
@end

@implementation Observer

- (void)observeValueForKeyPath:(NSString *)keyPath
                      ofObject:(id)object
                        change:(NSDictionary *)change
                       context:(void *)context
{
	NSLog(@"%@ - object: %@ change: %@", keyPath, object, change);
}

@end

int main(void) {
	Observer *o = [Observer new];
	Encapsulation *e = [Encapsulation new];

	[e addObserver: o forKeyPath: @"proxy.name" options: 0 context: NULL];
	[e addObserver: o forKeyPath: @"proxy.firstName" options: 0 context: NULL];

	[(Observee *)e.proxy setFirstName: @"Hans"];
	[(Observee *)e.proxy setLastName: @"Acker"];

        [e removeObserver: o forKeyPath: @"proxy.name"];
        [e removeObserver: o forKeyPath: @"proxy.firstName"];


	return 0;
}

Backtrace in LLDB:

(lldb) bt
* thread #1, name = 'a.out', stop reason = signal SIGABRT
  * frame #0: 0x0000fffff7706898 libc.so.6`__pthread_kill_implementation(threadid=281474835498848, signo=6, no_tid=<unavailable>) at pthread_kill.c:44:76
    frame #1: 0x0000fffff76b683c libc.so.6`__GI_raise(sig=6) at raise.c:26:13
    frame #2: 0x0000fffff76a1a80 libc.so.6`__GI_abort at abort.c:79:7
    frame #3: 0x0000fffff791521c libobjc.so.4.6`objc_exception_throw(object=0x0000aaaaaaed5768) at eh_personality.c:261:2
    frame #4: 0x0000fffff7bd7018 libgnustep-base.so.1.30`-[NSException raise](self=0x0000aaaaaaed5768, _cmd="\x80") at NSException.m:1610:3
    frame #5: 0x0000fffff7bd674c libgnustep-base.so.1.30`+[NSException raise:format:arguments:](self=0x0000fffff7f32090, _cmd=">\U00000005\U00000001", name=0x0000fffff7f8df28, format=0x0000fffff7f94148, argList=(__stack = 0x0000ffffffffe810, __gr_top = 0x0000ffffffffe790, __vr_top = 0x0000ffffffffe770, __gr_offs = -32, __vr_offs = -128)) at NSException.m:1489:3
    frame #6: 0x0000fffff7bd66bc libgnustep-base.so.1.30`+[NSException raise:format:](self=0x0000fffff7f32090, _cmd="T", name=0x0000fffff7f8df28, format=0x0000fffff7f94148) at NSException.m:1474:3
    frame #7: 0x0000fffff7c6f6f0 libgnustep-base.so.1.30`-[NSProxy forwardInvocation:](self=0x0000aaaaaaac0210, _cmd="`\v\U00000001", anInvocation=0x0000aaaaaaecfa98) at NSProxy.m:283:3
    frame #8: 0x0000fffff7d27404 libgnustep-base.so.1.30`GSFFIInvocationCallback(cif=0x0000aaaaaaeb7630, retp=0x0000ffffffffeae0, args=0x0000ffffffffe940, user=0x0000aaaaaaecfa08) at GSFFIInvocation.m:610:3
    frame #9: 0x0000fffff711643c libffi.so.8`ffi_closure_SYSV_inner(cif=0x0000aaaaaaeb7630, fun=<unavailable>, user_data=<unavailable>, context=<unavailable>, stack=0x0000ffffffffeb20, rvalue=0x0000fffff71168b8, struct_rvalue=<unavailable>) at ffi.c:1123:3
    frame #10: 0x0000fffff71168b8 libffi.so.8`ffi_closure_SYSV at sysv.S:377
    frame #11: 0x0000fffff7d09714 libgnustep-base.so.1.30`_NSKVOEnsureKeyWillNotify(object=0x0000aaaaaacf8ff8, key=0x0000aaaaaad14a48) at NSKVOSwizzling.m:678:8
    frame #12: 0x0000fffff7d08d7c libgnustep-base.so.1.30`_addKeyObserver(keyObserver=0x0000aaaaaad6c138) at NSKVOSupport.m:425:3
    frame #13: 0x0000fffff7d06500 libgnustep-base.so.1.30`_addKeypathObserver(object=0x0000aaaaaacf8ff8, keypath=0x0000aaaaaad14a48, keyPathObserver=0x0000aaaaaad00628, affectedObservers=0x0000000000000000) at NSKVOSupport.m:456:7
    frame #14: 0x0000fffff7d08b48 libgnustep-base.so.1.30`_addNestedObserversAndOptionallyDependents(keyObserver=0x0000aaaaaad13678, dependents=true) at NSKVOSupport.m:397:11
    frame #15: 0x0000fffff7d064f8 libgnustep-base.so.1.30`_addKeypathObserver(object=0x0000aaaaaacebbb8, keypath=0x0000aaaaaaac0920, keyPathObserver=0x0000aaaaaad00628, affectedObservers=0x0000000000000000) at NSKVOSupport.m:455:7
    frame #16: 0x0000fffff7d06338 libgnustep-base.so.1.30`-[NSObject(self=0x0000aaaaaacebbb8, _cmd="\xc6\U00000012\U00000001", observer=0x0000aaaaaace90c8, keyPath=0x0000aaaaaaac0920, options=0, context=0x0000000000000000) addObserver:forKeyPath:options:context:] at NSKVOSupport.m:716:7
    frame #17: 0x0000aaaaaaaa2804 a.out`main at main.m:103:2
    frame #18: 0x0000fffff76a2294 libc.so.6`__libc_start_call_main(main=(a.out`main at main.m:99), argc=1, argv=0x0000fffffffff2a8) at libc_start_call_main.h:58:16
    frame #19: 0x0000fffff76a2378 libc.so.6`__libc_start_main_impl(main=(a.out`main at main.m:99), argc=1, argv=0x0000fffffffff2a8, init=<unavailable>, fini=<unavailable>, rtld_fini=<unavailable>, stack_end=<unavailable>) at libc-start.c:360:3
    frame #20: 0x0000aaaaaaaa21f0 a.out`_start + 48
(lldb)

Here an example where the msg send machinery calls the instance method with metaclass as receiver:

#import <Foundation/Foundation.h>

@interface Proxy : NSProxy
@end

@implementation Proxy
- (void)forwardInvocation:(NSInvocation *)anInvocation
{
    [anInvocation setTarget:nil];
    [anInvocation invoke];
    return;
}
@end

int main(void) {
	[Proxy forwardInvocation: nil];
	return 0;
};