Why does this KVO code fall 100% of the time?

Below is the code below NSKVOUnionSetAndNotify

calling CFDictionaryGetValue

with what appears to be bogus.

It seems to be a race between swizzled code addFoos

/ NSKVOUnionSetAndNotify

and the act of adding and removing KVO observers.

#import <Foundation/Foundation.h>
@interface TestObject : NSObject
@property (readonly) NSSet *foos;
@end

@implementation TestObject {
    NSMutableSet *_internalFoos;
    dispatch_queue_t queue;
    BOOL observed;
}

- (id)init {
    self = [super init];
    _internalFoos = [NSMutableSet set];
    queue = dispatch_queue_create("test", DISPATCH_QUEUE_CONCURRENT);
    return self;
}

- (void)start {
    // Start a bunch of work hitting the unordered collection mutator
    for (int i = 0; i < 10; i++) {
        dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
            while (YES) {
                @autoreleasepool {
                    [self addFoos:[NSSet setWithObject:@(rand() % 100)]];
                }
            }
        });
    }

    // Start work that will constantly observe and unobserve the unordered collection
    [self observe];
}

- (void)observe {
    dispatch_async(dispatch_get_main_queue(), ^{
        observed = YES;
        [self addObserver:self forKeyPath:@"foos" options:0 context:NULL];
    });
}

- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context {
    dispatch_async(dispatch_get_main_queue(), ^{
        if (observed) {
            observed = NO;
            [self removeObserver:self forKeyPath:@"foos"];
            [self observe];
        }
    });
}

// Public unordered collection property
- (NSSet *)foos {
    __block NSSet *result;
    dispatch_sync(queue, ^{
        result = [_internalFoos copy];
    });
    return result;
}

// KVO compliant mutators for unordered collection
- (void)addFoos:(NSSet *)objects {
    dispatch_barrier_sync(queue, ^{
        [_internalFoos unionSet:objects];
    });
}

- (void)removeFoos:(NSSet *)objects {
    dispatch_barrier_sync(queue, ^{
        [_internalFoos minusSet:objects];
    });
}
@end

int main(int argc, const char * argv[]) {
    @autoreleasepool {
        TestObject *t = [[TestObject alloc] init];
        [t start];
        CFRunLoopRunInMode(kCFRunLoopDefaultMode, 10000, false);
    }
    return 0;
}

      

+3


source to share


1 answer


The actual alarm you get is EXC_BAD_ACCESS

when a key value observation dictionary is connected to it. The stack trace looks like this:

* thread #2: tid = 0x1ade39, 0x00007fff92f8e097 libobjc.A.dylib`objc_msgSend + 23, queue = 'com.apple.root.default-priority', stop reason = EXC_BAD_ACCESS (code=1, address=0x18)
    frame #0: 0x00007fff92f8e097 libobjc.A.dylib`objc_msgSend + 23
    frame #1: 0x00007fff8ffe2b11 CoreFoundation`CFDictionaryGetValue + 145
    frame #2: 0x00007fff8dc55750 Foundation`NSKVOUnionSetAndNotify + 147
  * frame #3: 0x0000000100000f85 TestApp`__19-[TestObject start]_block_invoke(.block_descriptor=<unavailable>) + 165 at main.m:34
    frame #4: 0x000000010001832d libdispatch.dylib`_dispatch_call_block_and_release + 12
    frame #5: 0x0000000100014925 libdispatch.dylib`_dispatch_client_callout + 8
    frame #6: 0x0000000100016c3d libdispatch.dylib`_dispatch_root_queue_drain + 601
    frame #7: 0x00000001000182e6 libdispatch.dylib`_dispatch_worker_thread2 + 52
    frame #8: 0x00007fff9291eef8 libsystem_pthread.dylib`_pthread_wqthread + 314
    frame #9: 0x00007fff92921fb9 libsystem_pthread.dylib`start_wqthread + 13

      

If you set a symbolic breakpoint with a symbol NSKVOUnionSetAndNotify

, the debugger will stop where this method is called. The glitch you see is that automatic key value notifications are sent from one thread when your method is called [addFoos:]

, but then a change dictionary from another thread is opened to it. This is encouraged by using the global dispatch queue when calling this method, as this will execute the block on many different threads.

There are many ways to fix this crash, and I will try to walk you through this to give you a clearer picture of what is going on.

In the simplest case, you can fix the error by using the mutable proxy key-value for that key:

NSMutableSet *someSet = [self mutableSetValueForKey:@"foos"];
[someSet unionSet:[NSSet setWithObject:@(rand() % 100)]];

      

This will stop this disaster. What's going on here? When called mutableSetValueForKey:

, the result is a proxy object that forwards messages to your KVC-compliant accessors for the "foos" key. The author object does not actually completely match the required pattern for a KVC-compliant property of this type. If other KVC accessors are passed for this key, they can go through non-thread safe accessors provided by Foundation, which can cause this to fail over and over again. We'll look at how to fix this for now.

The crash is triggered by automatic KVO change notifications crossing streams. Automatic KVO notifications work by swizzling classes and methods at runtime. You can read a more detailed explanation here and here . KVC accessors are essentially wrapped at runtime using KVO methods. This actually crashes in the original application. This is the pasted KVO code disassembled from Foundation:

int _NSKVOUnionSetAndNotify(int arg0, int arg1, int arg2) {
    r4 = object_getIndexedIvars(object_getClass(arg0));
    OSSpinLockLock(_NSKVONotifyingInfoPropertyKeysSpinLock);
    r6 = CFDictionaryGetValue(*(r4 + 0xc), arg1);
    OSSpinLockUnlock(_NSKVONotifyingInfoPropertyKeysSpinLock);
    var_0 = arg2;
    [arg0 willChangeValueForKey:r6 withSetMutation:0x1 usingObjects:STK-1];
    r0 = *r4;
    r0 = class_getInstanceMethod(r0, arg1);
    method_invoke(arg0, r0);
    var_0 = arg2;
    r0 = [arg0 didChangeValueForKey:r6 withSetMutation:0x1 usingObjects:STK-1];
    Pop();
    Pop();
    Pop();
    return r0;
}

      

As you can see, this completes the KVC-enabled accessor with willChangeValueForKey:withSetMutation:usingObjects:

and didChangeValueForKey: withSetMutation:usingObjects:

. These are the methods for sending KVO notifications. KVO will insert this wrapper at runtime if the object has chosen to automatically notify the observer of the key value. In between these calls, you can see class_getInstanceMethod

. This links to the KVC compatible accessory and then calls it. In the case of the source code, this is being run from within NSSet unionSet:

, which was threading and causing a crash when accessing the pluggable dictionary.

Automatic notifications are sent by the thread on which the change occurred and are intended to be received on the same thread. That's what Teh IntarWebs is, there is a lot of bad or misleading information about KVO. Not all objects and not all properties issue automatic KVO notifications, and in your classes you can control what to do and not to do. From the Key Values ​​Programming Guide: Automatic Change Notification :

NSObject provides a basic implementation of automatic key change notification. Key-value change notification informs observers of changes made using key-compatible as well as key-value encoding methods. Automatic notification is also supported by collection objects returned, for example mutableArrayValueForKey:

This can cause all NSObject descendants to automatically issue automatic notifications. This is not the case - perhaps the framework classes do not or do not implement special behavior. Examples are the main data. From the Master Data Programming Guide :

NSManagedObject disables automatic key value change (KVO) notifications for simulated properties, and primitive accessors do not invoke access and change notification methods. For unmoded properties in OS X v10.4 Core Data also disables automatic KVO; on OS X v10.5 and later, Core Data adopts NSObjects behavior.

As a developer, you can be sure that the notice of observers with automatic key value enabled or disabled for a particular property, to implement a method with the correct naming convention +automaticallyNotifiesObserversOf<Key>

. When this method returns NO, no automatic key value notifications are issued for this property. When automatic change notifications are disabled, KVO should also not check the accessor method at runtime, as this is done primarily to support automatic change notifications. For example:

+ (BOOL) automaticallyNotifiesObserversOfFoos {
    return NO;
}

      

In a comment, the author stated that the reason he used dispatch_barrier_sync

for his accessor methods is that if he didn't, the KVO notifications would appear before the changes take place. If automatic notifications are turned off for a property, you still have the option to send these notifications manually. This is done using the willChangeValueForKey:

and methods didChangeValueForKey:

. This not only gives you control over when these notifications are sent (if at all), but to which stream. Automatic change notifications, remember, are sent and received on the stream where the change occurred. For example, if you only wanted change notifications to occur on the main queue, you could do so with recursive decomposition:

- (void)addFoos:(NSSet *)objects {
    dispatch_async(dispatch_get_main_queue(), ^{
        [self willChangeValueForKey:@"foos"];
        dispatch_barrier_sync(queue, ^{
            [_internalFoos unionSet:objects];
            dispatch_async(dispatch_get_main_queue(), ^{
                [self didChangeValueForKey:@"foos"];
            });
        });
    });
}

      

The original class in the author's question was forcing the KVO watch to start and stop on the main queue, which appears to be an attempt to emit notifications on the main queue. The example above shows a solution that not only addresses this issue, but also ensures that KVO notifications are sent correctly before and after data changes.

In the example above, I modified the author's original method as an illustrative example - this class is still not KVC compliant for the "foos" key. To be compatible with key values, an object must first meet the key value encoding requirements. To solve this problem, first create the correct keyword-encoded adapters for the unordered volatile collection :

Immutable: countOfFoos

enumeratorOfFoos

memberOfFoos:



Mutable: addFoosObject:

removeFoosObject:

This is just a bare minimum, there are additional techniques that can be implemented for performance or data integrity purposes.

The original application used a parallel queue and dispatch_barrier_sync

. This was dangerous for many reasons. The approach recommended by the Concurrency Programming Guide is to use a sequential queue. This ensures that only one thing can touch the protected resource at a time, and that is from a consistent context. For example, two of the above methods would look like this:

- (NSUInteger)countOfFoos {
    __block NSUInteger  result  = 0;
    dispatch_sync([self serialQueue], ^{
        result = [[self internalFoos] count];
    });
    return result;
}

- (void) addFoosObject:(id)object {
    id addedObject = [object copy];
    dispatch_async([self serialQueue], ^{
        [[self internalFoos] addObject:addedObject];
    });
}

      

Note that in this example and the following, I am not including manual KVO change notifications for brevity and clarity. If you want manual change notifications to be sent, this code should be added to these methods, like you saw in the previous example.

Unlike using dispatch_barrier_sync

with a parallel queue, this will not lead to dead ends.

This is how you get deadlocks

WWDC 2011 Session 210 Mastering the Grand Central Dispatch showed the correct use of the Dispatch Barrier API to implement read / write locks for using a parallel queue. This will be implemented as follows:

- (id) memberOfFoos:(id)object {
    __block id  result  = nil;
    dispatch_sync([self concurrentQueue], ^{
        result = [[self internalFoos] member:object];
    });
    return result;
}

- (void) addFoosObject:(id)object {
    id addedObject = [object copy];
    dispatch_barrier_async([self concurrentQueue], ^{
        [[self internalFoos] addObject:addedObject];
    });
}

      

Note that for a write operation, the dispatch barrier is opened asynchronously while the read operation uses dispatch_sync

. The original application used dispatch_barrier_sync

to read and write, which, according to the author, was done to control when sending automatic change notifications. Using manual shift notifications will address this issue (again, not shown in this example for brevity and clarity).

There are still problems with the original KVO implementation. It does not use a pointer context

to determine the ownership of the observation. This is a recommended practice and can use a pointer to self

as a value. The value must have the same address as the object used to add and remove the observer:

[self addObserver:self forKeyPath:@"foos" options:NSKeyValueObservingOptionNew context:(void *)self];

- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context {
    if (context == (__bridge void *)self){
        // check the key path, etc.
    } else {
        [super observeValueForKeyPath:keyPath ofObject:object change:change context:context];
    }
}

      

In the NSKeyValueObserving.h header:

You should use -removeObserver: forKeyPath: context: instead of -removeObserver: forKeyPath: whenever possible, because it allows you to specify your intent more precisely. When the same observer registers for the same key path multiple times, but with different context pointers each time, -removeObserver: forKeyPath: must guess the context pointer when deciding what to delete, and may be wrong.

If you are interested in further understanding the application and implementation of Key Value Observing, I suggest the video KVO is considered Awesome

In short:

• Implement the required key value encoding access pattern (unordered volatile collection)

• Secure these accessories (using serial queue with dispatch_sync

/ dispatch_async

or parallel queue with dispatch_sync

/ dispatch_barrier_async

)

• Decide if you want automatic KVO notifications or not, enter automaticallyNotifiesObserversOfFoos

accordingly

• Correctly add change notifications manually to accessor methods.

• Make sure the code that accesses your property does so through the correct KVC accessor methods (i.e. mutableSetValueForKey:

)

+6


source







All Articles