Search code examples
iosobjective-cconcurrencyassociated-object

Objective-C runtime associate object with NSMutable dictionary


Reference from this post link I implemented a similar category using the same concept of using NSMutableDictionary to store the information I need. But there is one thing confuses me in the original post

- (NSMutableDictionary *)addl_associatedDictionary
{
     NSMutableDictionary *res = nil;
     @synchronized(self) 
     {
        if (!(res = [self addl_associatedDictionaryPrimitive])) 
        {
           res = [self addl_generateAssociatedDictionary];
        }
     }
    return res;
}

I know @synchronized keyword is a protection for the mutilthread. but as i go through other examples most of then didn't use the protection. so is the protection necessary? also can i use static dispatch_once_t to replace the @synchronized? below is my code snipptes in .m file

@dynamic associatedDict;

-(void)setAssociateValue:(NSMutableDictionary*)dict
{
    objc_setAssociatedObject(self, @selector(associatedDict), dict,   OBJC_ASSOCIATION_RETAIN);
} 

-(id)getAssociateValue
{
    return objc_getAssociatedObject(self, @selector(associatedDict));
}

-(NSMutableDictionary*)associatedDict
{
    NSMutableDictionary* dict=[self getAssociateValue];
    if(!dict)
    {
       dict=[[NSMutableDictionary alloc]init];
       [self setAssociatedDict:dict];
    }
    return dict;
 } 


 -(void)setAssociateDict:(NSMutableDictionary *)associatedDict
{
    [self setAssociatedDict:associatedDict];
}

-(id)associate_getObjectForKey:(NSString*)key
{
    return self.associatedDict[key];
}

-(void)associate_setObject:(id)obj forKey:(NSString*)key
{
   [self.associatedDict setObject:obj forKey:key];
}

Solution

  • Such as it may help to answer the implied concern about locking costs: I notice that you're using OBJC_ASSOCIATION_RETAIN rather than OBJC_ASSOCIATION_RETAIN_NONATOMIC. That would appear to be redundant given your @synchronize in that if you have the latter than you can forego the locking on the former. At the minute you're paying twice for synchronisation. Either pay once or not at all.

    The best overall solution might be:

    NSMutableDictionary *res; // no need to assign to `nil`; it's implied in ARC
    
    // you're using an atomic property, so this is inherently safe
    if (!(res = [self addl_associatedDictionaryPrimitive])) 
    {
        // okay, doesn't exist, but two or more threads may get to
        // here simultaneously so we'll need to synchronise, and...
        @synchronized(self) 
        {
            // ... check again. As someone else may already have proceeded past the
            // outer if and created it while this thread was waiting to
            // enter the synchronised block. Assuming this dictionary
            // is created more rarely than it's accessed, this is no great issue
            if (!(res = [self addl_associatedDictionaryPrimitive])) 
            {
               res = [self addl_generateAssociatedDictionary];
            }
        }
    }
    return res;
    

    ... and stick with OBJC_ASSOCIATION_RETAIN. Also pay attention to CRD's point: mutable dictionaries are not themselves thread safe. So if you actually need thread safety then this doesn't exactly resolve the issue. If you don't need thread safety then switch to OBJC_ASSOCIATION_RETAIN_NONATOMIC and dump the @synchronized.