KVO deadlock (seemingly internal global mutex being held, and attempting to swap with lock of object being observed or mutated)

KVODeadlock.m

@interface NSObject ()
- (void)_addObserver:(id)arg1 forProperty:(id)arg2 options:(unsigned int)arg3 context:(void*)arg4;
- (void)_changeValueForKey:(id)arg1 key:(id)arg2 key:(id)arg3 usingBlock:(id /* block */)arg4;
- (void)_changeValueForKey:(id)arg1 usingBlock:(id /* block */)arg2;
- (void)_changeValueForKeys:(id*)arg1 count:(unsigned int)arg2 maybeOldValuesDict:(id)arg3 usingBlock:(id /* block */)arg4;
- (void)_didChangeValuesForKeys:(id)arg1;
- (void)_notifyObserversForKeyPath:(id)arg1 change:(id)arg2;
- (void)_notifyObserversOfChangeFromValuesForKeys:(id)arg1 toValuesForKeys:(id)arg2;
- (void)_willChangeValuesForKeys:(id)arg1;
@end

@interface KVOSafeObject : NSObject @end

@implementation KVOSafeObject

- (void)_changeValueForKeys:(id *)arg1 count:(unsigned int)arg2 maybeOldValuesDict:(id)arg3 usingBlock:(id)arg4 {
    @synchronized (self) {
        [super _changeValueForKeys:arg1 count:arg2 maybeOldValuesDict:arg3 usingBlock:arg4];
    }
}

- (void)_addObserver:(id)arg1 forProperty:(id)arg2 options:(unsigned int)arg3 context:(void*)arg4 {
    @synchronized(self) {
        [super _addObserver:arg1 forProperty:arg2 options:arg3 context:arg4];
    }
}
- (void)_changeValueForKey:(id)arg1 key:(id)arg2 key:(id)arg3 usingBlock:(id /* block */)arg4 {
    @synchronized(self) {
        [super _changeValueForKey:arg1 key:arg2 key:arg3 usingBlock:arg4];
    }
}

- (void)_changeValueForKey:(id)arg1 usingBlock:(id)arg2 {
    @synchronized(self) {
        [super _changeValueForKey:arg1 usingBlock:arg2];
    }
}

- (void)_willChangeValuesForKeys:(id)arg1 {
    @synchronized(self) {
        [super _willChangeValuesForKeys:arg1];
    }
}

- (void)_didChangeValuesForKeys:(id)arg1 {
    @synchronized(self) {
        [super _didChangeValuesForKeys:arg1];
    }
}

- (void)removeObserver:(NSObject *)observer forKeyPath:(NSString *)keyPath {
    @synchronized(self) {
        [super removeObserver:observer forKeyPath:keyPath];
    }
}

- (void)removeObserver:(NSObject *)observer forKeyPath:(NSString *)keyPath context:(nullable void *)context {
    @synchronized(self) {
        [super removeObserver:observer forKeyPath:keyPath context:context];
    }
}

- (void)addObserver:(NSObject *)observer forKeyPath:(NSString *)keyPath options:(NSKeyValueObservingOptions)options context:(nullable void *)context {
    @synchronized (self) {
        [super addObserver:observer forKeyPath:keyPath options:options context:context];
    }
}

@end

@interface RandomTestObject : KVOSafeObject
@property NSString *field;
@end

@implementation RandomTestObject @end

@interface NestedTestObject2 : KVOSafeObject
@property NSString *nestedField;
@end

@implementation NestedTestObject2 @end

@interface NestedTestObject : KVOSafeObject
@property NSString *nestedField;
@property NestedTestObject2 *nestedObject;
@end

@implementation NestedTestObject @end

@interface TestObject : KVOSafeObject
@property NestedTestObject *nestedObject;
@end

@implementation TestObject @end

@interface TestObserver : NSObject
@property TestObject *object;
@property RandomTestObject *randomObject;
@end

@implementation TestObserver

- (instancetype)init {
    self = [super init];

    _object = [TestObject new];

    // Dummy observer for safety
    [_object addObserver:self
              forKeyPath:@"nestedObject.nestedField"
                 options:(NSKeyValueObservingOptionInitial | NSKeyValueObservingOptionNew | NSKeyValueObservingOptionOld)
                 context:observerContext];

    // Object to observe/modify that isnt in the frequent add/remove graph
    _randomObject = [RandomTestObject new];

    [_randomObject addObserver:self
                    forKeyPath:@"field"
                       options:(NSKeyValueObservingOptionInitial | NSKeyValueObservingOptionNew | NSKeyValueObservingOptionOld)
                       context:observerContext];

    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0.5 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
        [self observe];
    });

    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
        [self set];
        [self set2];
    });

    return self;
}

- (void)set2 {
    @synchronized (self.object.nestedObject) {
        self.randomObject.field = [NSString new];
//        self.object.nestedObject.nestedObject = [NestedTestObject2 new];
    }

    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
        [self set2];
    });
}

- (void)set {
    @synchronized (self.object) {
        self.object.nestedObject = [NestedTestObject new];
    }

    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
        [self set];
    });
}

-(void)observe {
    [self.object addObserver:self
                  forKeyPath:@"nestedObject.nestedObject.nestedField"
                     options:(NSKeyValueObservingOptionInitial | NSKeyValueObservingOptionNew | NSKeyValueObservingOptionOld)
                     context:observerContext];

    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0), ^{
        [self.object removeObserver:self forKeyPath:@"nestedObject.nestedObject.nestedField" context:observerContext];

        dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0), ^{
            [self observe];
        });
    });
}

static void *observerContext = &observerContext;

- (void)observeValueForKeyPath:(NSString *)keyPath
                      ofObject:(id)object
                        change:(NSDictionary<NSKeyValueChangeKey,id> *)change
                       context:(void *)context {

    if (context != observerContext) {
        return [super observeValueForKeyPath:keyPath ofObject:object change:change context:context];
    }

    NSLog(@"Changed");
}

@end

As observation addition/removal recurses top->down, and mutation/notification recurses bottom->up, I'm not totally sure the absence of these internal locks would actually solve anything right now – I think with anything in the graph locking for whatever reason, there's always the chance that observer addition will recurse at the right time such that object A is locked, object B is about to be locked, but object B (or somewhere under it) locks to mutate on another thread, and starts locking upwards for notification eventually needing A.

A heinous hack (on top of an already heinous hack) of simplifying the KVOSafeObject implementation to the below, with the addition of more eagerly locking down the graph when adding/removing the observers, is solving this for me.

@implementation KVOSafeObject

- (void)_changeValueForKeys:(id *)arg1 count:(unsigned int)arg2 maybeOldValuesDict:(id)arg3 usingBlock:(id)arg4 {
    @synchronized (self) {
        [super _changeValueForKeys:arg1 count:arg2 maybeOldValuesDict:arg3 usingBlock:arg4];
    }
}

- (void)removeObserver:(NSObject *)observer forKeyPath:(NSString *)keyPath {
    @synchronized(self) {
        @synchronized ([self valueForKeyPath:[keyPath componentsSeparatedByString:@"."][0]]) {
            [super removeObserver:observer forKeyPath:keyPath];
        }
    }
}

- (void)addObserver:(NSObject *)observer
         forKeyPath:(NSString *)keyPath
            options:(NSKeyValueObservingOptions)options
            context:(nullable void *)context {

    @synchronized (self) {
        @synchronized ([self valueForKeyPath:[keyPath componentsSeparatedByString:@"."][0]]) {
            [super addObserver:observer forKeyPath:keyPath options:options context:context];
        }
    }
}

@end