2

I was wondering if the following is the best way to read and make a copy an object that may be locked by another thread?

-(NSObject*) getCopyOfActiveObjectsOfType:(Class) objectClass
{
    NSMutableArray* copy = [NSMutableArray arrayWithArray:[self.activeObjects objectForKey:objectClass]];

    return copy;
}

I have several methods like the following that lock the object in question to add or remove objects to the array.

-(void) addObject:(NSObject *)object toPoolOfObjects:(NSMutableDictionary*) objects
{
    Class objectClass = [object class];
    @synchronized (objects)
    {
        NSMutableArray* objectArray = [objects objectForKey:objectClass];

        if(objectArray == nil)
        {
            objectArray = [[[NSMutableArray alloc] init] autorelease];
            [objects setObject:objectArray forKey:objectClass];
        }
        [objectArray addObject:object];
    }

}
xcoder
  • 967
  • 2
  • 11
  • 24
  • 4
    Isn't the point of locking exactly *not* to do this? – omz Sep 05 '11 at 14:29
  • 2
    Btw the @synchronized directive only has an effect when you use it everywhere. It does not prevent you from accessing the object outside of a @synchronized block, so you *could* access `objects` however you like, but you really shouldn't. – omz Sep 05 '11 at 14:39
  • Well I'm trying to do something similar to dirty reads, and non-repeatable reads like in databases: http://en.wikipedia.org/wiki/Isolation_(database_systems)#Dirty_readsI need synchronization for certain blocks of code, which need to be consistent, but in the first method, as I only need to read the data, and don't mind if it is inconsistent, I need a way to do that. The first method should not be blocked by the locking operations of other methods locking the object, as I need the first method to be fast. If the other methods are slow because of locking, that is ok. – xcoder Sep 06 '11 at 01:26
  • Also, I guess that since the first method is returning a copy (its only reading), it should not addict the second method. – xcoder Sep 06 '11 at 01:49
  • So what happens when another thread removes all objects from the array while you're in the middle of making a copy? Your copying thread ends up with dangling pointers and your code will crash. At that point you will probably start caring about consistency. In most cases this won't happen but when it does, it's unpredictable and very hard to debug. – omz Sep 06 '11 at 08:27
  • In my scenario, I don't think there should be dangling pointers, since the objects are retained in a third array. What do you think? I'm trying to determine if an exception will be thrown because the array is being modified while the array is being copied. – xcoder Sep 07 '11 at 02:04
  • http://en.wikipedia.org/wiki/Read/write_lock_pattern And http://cocoaheads.byu.edu/wiki/locks ;-) – eSniff Nov 04 '11 at 01:18

1 Answers1

0

Using the @synchronize method or equivalent in alternate languages:

  • if your instance is accessed from multiple threads and it is not immutable, you will require synchronization blocks around all methods accessing state that can be be mutated
    • your object is immutable if none of the accessors / methods change the internal state of the object.

You are using standard NSMutableArray / NSMutableDictionary primitives for state, however these are not thread safe and therefore require locks when shared across threads. Without knowing the access vs update frequency for your API and it's consumers, it is difficult to recommend a specific solution.

If your API is mostly reads, you could get away with the following solution, which implements a copy on write. Though as always, you must profile to determine if the solution meets your performance requirements.

- (NSArray *)getActiveObjectsOfType:(Class)objectClass {
    @synchronize(self.activeObjects) {
        return [self.activeObjects objectForKey:objectClass];
    }
}

- (void)addObject:(NSObject *)object {
    Class objectClass = [object class];

    // synchronize access to activeObjects
    @synchronize(self.activeObjects) {
        NSArray *existingArray = [[[self.activeObjects objectForKey:objectClass] retain] autorelease];

        NSMutableArray *newArray;
        if (existingArray) {
            newArray = [NSMutableArray arrayWithArray:existingArray];
        } else {
            newArray = [NSMutableArray array];
        }

        [newArray addObject:object];

        [self.activeObjects setObject:newArray forKey:objectClass];
    }
}

What is important to note with this solution is the NSArray returned by getActiveObjectsOfType is immutable.

Stuart Carnie
  • 5,458
  • 1
  • 29
  • 27
  • To be completely safe, I think you need to wrap the first assignment of `existingArray` with a `[[ retain] autorelease];`. See http://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/Multithreading/ThreadSafety/ThreadSafety.html#//apple_ref/doc/uid/10000057i-CH8-SW8 – NSGod Nov 15 '11 at 18:15
  • Yes, you are correct. When we exit the first synchronization block, another thread (calling `addObject:`) could free `existingArray` in the second `@synchronize` block. – Stuart Carnie Nov 15 '11 at 20:24
  • I see that your suggestion might solve my initial problem, but also creates another - it looks possible that if two threads call addObject, depending on the order of execution of the threads, one of the objects being added may be "lost", since a copy could be made before the object is added by the one thread. Following which the other thread would add its own object, and then replace the array in the second synchronized block. When the other thread replaces the array in the second synchronized block, the object that was added in the other thread (but not copied over) would be lost. – xcoder Nov 30 '11 at 05:52
  • Premature optimization is the root of all evil… You are correct, and the entire block should be in a `@synchronize` block. Editing… – Stuart Carnie Dec 07 '11 at 19:38