44

I have a question on thread safety while using NSMutableDictionary.

The main thread is reading data from NSMutableDictionary where:

  • key is NSString
  • value is UIImage

An asynchronous thread is writing data to above dictionary (using NSOperationQueue)

How do I make the above dictionary thread safe?

Should I make the NSMutableDictionary property atomic? Or do I need to make any additional changes?

@property(retain) NSMutableDictionary *dicNamesWithPhotos;

Stunner
  • 12,025
  • 12
  • 86
  • 145
Raj
  • 636
  • 1
  • 8
  • 12
  • 2
    I'm no expert on multithreading, but I do know that the "atomic" flag (the default for @synthesize'd accessors) makes no guarantees of thread safety. I did think the same thing when I first read about it, though. – Joshua Nozzi Dec 31 '09 at 19:43

6 Answers6

78

NSMutableDictionary isn't designed to be thread-safe data structure, and simply marking the property as atomic, doesn't ensure that the underlying data operations are actually performed atomically (in a safe manner).

To ensure that each operation is done in a safe manner, you would need to guard each operation on the dictionary with a lock:

// in initialization
self.dictionary = [[NSMutableDictionary alloc] init];
// create a lock object for the dictionary
self.dictionary_lock = [[NSLock alloc] init];


// at every access or modification:
[object.dictionary_lock lock];
[object.dictionary setObject:image forKey:name];
[object.dictionary_lock unlock];

You should consider rolling your own NSDictionary that simply delegates calls to NSMutableDictionary while holding a lock:

@interface SafeMutableDictionary : NSMutableDictionary
{
    NSLock *lock;
    NSMutableDictionary *underlyingDictionary;
}

@end

@implementation SafeMutableDictionary

- (id)init
{
    if (self = [super init]) {
        lock = [[NSLock alloc] init];
        underlyingDictionary = [[NSMutableDictionary alloc] init];
    }
    return self;
}

- (void) dealloc
{
   [lock_ release];
   [underlyingDictionary release];
   [super dealloc];
}

// forward all the calls with the lock held
- (retval_t) forward: (SEL) sel : (arglist_t) args
{
    [lock lock];
    @try {
        return [underlyingDictionary performv:sel : args];
    }
    @finally {
        [lock unlock];
    }
}

@end

Please note that because each operation requires waiting for the lock and holding it, it's not quite scalable, but it might be good enough in your case.

If you want to use a proper threaded library, you can use TransactionKit library as they have TKMutableDictionary which is a multi-threaded safe library. I personally haven't used it, and it seems that it's a work in progress library, but you might want to give it a try.

Stavash
  • 14,244
  • 5
  • 52
  • 80
notnoop
  • 58,763
  • 21
  • 123
  • 144
  • 2
    This looks like a great method but I can't get it to compile. I'm getting `"expected ')' before 'retval_t'"` on the line `- (retval_t) forward: (SEL) sel : (arglist_t) args` Any ideas? – Michael Waterfall Apr 23 '10 at 16:01
  • 5
    Fabulous answer. Now obsolete. Use a queue instead. I have a dead simple serialized dictionary somewhere. I should post it. Message forwarding is slow and fragile. – bbum Aug 09 '11 at 14:46
  • 6
    Follow-up to bbum's comment. There's a useful write-up on using GCD concurrent queues with NSMutableDictionary on Mike Ash's blog here: http://mikeash.com/pyblog/friday-qa-2011-10-14-whats-new-in-gcd.html – stephent Mar 04 '12 at 18:45
  • @bbum did you ever manage to post a example of your serialised dictionary? – Paul.s Aug 29 '13 at 08:21
  • 3
    How about using @sychronized to make it safe? – allenlinli Oct 07 '14 at 04:27
  • I agree with @AllenLin's comment, I find `@synchronized` to be more convenient than trying to explicitly lock/unlock. – aroth May 15 '15 at 05:41
  • Agreed with @allenlinli, this is essentially what `@synchronized` does for you for free since we are just locking access to the dictionary and not discerning if it's a get/set operation. – Joe Tam Jan 14 '17 at 18:42
6

Nowadays you'd probably go for @synchronized(object) instead.

...
@synchronized(dictionary) {
    [dictionary setObject:image forKey:name];
}
...
@synchronized(dictionary) {
    [dictionary objectForKey:key];
}
...
@synchronized(dictionary) {
    [dictionary removeObjectForKey:key];
}

No need for the NSLock object any more

P.Melch
  • 8,066
  • 43
  • 40
2

after a little bit of research I want to share with you this article :

Using collection classes safely with multithreaded applications http://developer.apple.com/library/mac/#technotes/tn2002/tn2059.html

It looks like notnoop's answer may not be a solution after all. From threading perspective it is ok, but there are some critical subtleties. I will not post here a solution but I guess that there is a good one in this article.

Hertzel Guinness
  • 5,912
  • 3
  • 38
  • 43
Adrian
  • 1,595
  • 1
  • 19
  • 21
  • 1
    +1 for noticing that locking isn't enough in this case. I was bitten by this once as well, the `[[[dict objectForKey:key] retain] autorelease]` "trick" really is necessary in a multithreaded environment. – DarkDust May 24 '11 at 08:38
  • 4
    That link is now broken, and the technote is from 2002. You might be better off with https://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/Multithreading/ThreadSafetySummary/ThreadSafetySummary.html. – Clay Bridges Aug 22 '12 at 16:19
  • -1 For what is almost (but not quite, thus avoiding flagging,) a link only answer. – ArtOfWarfare Aug 17 '14 at 15:38
1

I have two options to using nsmutabledictionary.

One is:

NSLock* lock = [[NSLock alloc] init];
[lock lock];
[object.dictionary setObject:image forKey:name];
[lock unlock];

Two is:

//Let's assume var image, name are setup properly
dispatch_async(dispatch_get_main_queue(), 
^{ 
        [object.dictionary setObject:image forKey:name];
});

I dont know why some people want to overwrite setting and getting of mutabledictionary.

Roman Marusyk
  • 23,328
  • 24
  • 73
  • 116
gigir
  • 132
  • 1
  • 10
1

Even the answer is correct, there is an elegant and different solution:

- (id)init {
self = [super init];
if (self != nil) {
    NSString *label = [NSString stringWithFormat:@"%@.isolation.%p", [self class], self];
    self.isolationQueue = dispatch_queue_create([label UTF8String], NULL);

    label = [NSString stringWithFormat:@"%@.work.%p", [self class], self];
    self.workQueue = dispatch_queue_create([label UTF8String], NULL);
}
return self;
}
//Setter, write into NSMutableDictionary
- (void)setCount:(NSUInteger)count forKey:(NSString *)key {
key = [key copy];
dispatch_async(self.isolationQueue, ^(){
    if (count == 0) {
        [self.counts removeObjectForKey:key];
    } else {
        self.counts[key] = @(count);
    }
});
}
//Getter, read from NSMutableDictionary
- (NSUInteger)countForKey:(NSString *)key {
__block NSUInteger count;
dispatch_sync(self.isolationQueue, ^(){
    NSNumber *n = self.counts[key];
    count = [n unsignedIntegerValue];
});
return count;
}

The copy is important when using thread unsafe objects, with this you could avoid the possible error because of unintended release of the variable. No need for thread safe entities.

If more queue would like to use the NSMutableDictionary declare a private queue and change the setter to:

self.isolationQueue = dispatch_queue_create([label UTF8String], DISPATCH_QUEUE_CONCURRENT);

- (void)setCount:(NSUInteger)count forKey:(NSString *)key {
key = [key copy];
dispatch_barrier_async(self.isolationQueue, ^(){
    if (count == 0) {
        [self.counts removeObjectForKey:key];
    } else {
        self.counts[key] = @(count);
    }
});
}

IMPORTANT!

You have to set an own private queue without it the dispatch_barrier_sync is just a simple dispatch_sync

Detailed explanation is in this marvelous blog article.

BootMaker
  • 1,639
  • 1
  • 22
  • 24
1

In some cases you might use the NSCache class. The documentation claims that it's thread safe:

You can add, remove, and query items in the cache from different threads without having to lock the cache yourself.

Here is article that describes quite useful tricks related to NSCache

Konstantin Nikolskii
  • 1,075
  • 1
  • 12
  • 17