views:

716

answers:

3

I'm creating a KVC/KVO-compliant mutable array on one of my objects the recommended way:

@interface Factory {
    NSMutableArray *widgets;
}
- (NSArray *)widgets;
- (void)insertObject:(id)obj inWidgetsAtIndex:(NSUInteger)idx;
- (void)removeObjectFromWidgetsAtIndex:(NSUInteger)idx;
@end

Clearly this is a tricky thread-safety issue. In the insert and remove methods I'm locking around array access to prevent concurrent modification, as recommended.

My question is, what is the proper way to implement the widgets accessor? Here's my implementation:

- (NSArray *)widgets {
    [widgetLock lock];
    NSArray *a = [[widgets copy] autorelease];
    [widgetLock unlock];
    return a;
}

Is it threadsafe?

A: 

You will need locking on all reading and writing methods. If your insert and remove are also locking (like you said) then the accessor method should be fine like that.

tcurdt
+2  A: 

Your widgets accessor should be fine, although you should be aware that none of the objects in that array are locked. So, you could run into problems trying to concurrently run code like

[[[myFactory widgets] objectAtIndex:7] setName:@"mildred"];

and

[myTextField setStringValue:[[[myFactory widgets] objectAtIndex:7] name]]; // mildred? or something else?

Since the objects in your array are not locked, you could run into race conditions or readers/writers-type problems. Isn't multithreading a joy?

On a different note, for KVC-compliance, I'd advise implementing objectInWidgetsAtIndex: and countOfWidgets instead of a widgets accessor. Remember, KVC models relationships, not array properties. So you would call something like [myFactory mutableArrayValueForKey:@"widgets"] to get an array representing the widgets property.

Alex
+1  A: 

Rather than creating your own lock, you could also use the locking built into the language:

i.e

- (NSArray *)widgets {
    @synchronized(widgets)
    {
        NSArray *a = [[widgets copy] autorelease];
        return a;
    }
}

and use similar locking in all other methods that access widgets. (The parameter widgets passed into @synchronized refers to the instance variable, not the method.)

alex's comment about access to contained objects still apply.

Matt Gallagher