NSMutableDictionary Not Threadsafe: So?

Question

I've had a very rarely occurring bug where a key in an NSMutableDictionary instance ends up pointing to the wrong object (another object in the same dictionary).

Could "key confusion" be the result of a race condition?

Here's the code (I've put in new thread safety since):

- (NSNumber*) makeHashFromResizedImage:(UIImage*)original newSize:(CGSize)newSize {
    int retVal = original.hash + newSize.width * 2 + newSize.height * 4;
    return [NSNumber numberWithInteger:retVal];
}

- (UIImage *)resizeImage:(UIImage*)image newSize:(CGSize)newSize {
    NSNumber *key = [self makeHashFromResizedImage:image newSize:newSize];

    if ([self.resizedImages objectForKey:key]) {
        return [self.resizedImages objectForKey:key];
    }

    UIImage *newImage = {{image-resizing-code-here}};

    [self.resizedImages setObject:newImage forKey:key];

    return newImage;
}

Note: {{image-resizing-code-here}} was removed for brevity. I'm beginning to suspect my makeHashFromResizedImage as it probably doesn't work as expected. Going to Unit Test it now.

Please show your code. (and yes, [NSDictionary isn't thread-safe](http://stackoverflow.com/questions/1986736/nsmutabledictionary-thread-safety).) — kennytm, Jan 17 '12 at 18:04
@KennyTM I was going to say my code is not relevant, but surely it is relevant and I'm probably looking at the wrong thing. My `makeHashFromResizedImage` looks like it might be having float-and-int problems, too, for instance. Code is posted. — Dan Rosenstark, Jan 17 '12 at 19:05
If you are worried about making NSDictionary thread safe, look at this question [here](http://stackoverflow.com/questions/8763028/how-can-i-make-every-message-an-object-receives-thread-safe) — Richard J. Ross III, Jan 17 '12 at 19:14
Thanks @RichardJ.RossIII I'm not worried, and in fact I don't tink it's a problem in this context. I'm beginning to think that my problem is much simpler: I think I've screwed up `makeHashFromResizedImage`. Checking that now. — Dan Rosenstark, Jan 17 '12 at 19:21

score 2 · Answer 1 · answered Jan 17 '12 at 18:40

2

As Rob said; you need to provide exclusion yourself.

Yes, "key confusion" could be a result of a race condition, but it is exceedingly unlikely. More likely that you would end up with intermittent crashes. But, sure, misplaced values could happen because of corruption and undefined behavior.

answered Jan 17 '12 at 18:40

bbum

162,346
23
271
359

Thanks. I'm probably not looking at the right problem. I updated the question with some code. – Dan Rosenstark Jan 17 '12 at 19:05

kennytm · Accepted Answer · 2012-01-17T19:30:46.287

2

This:

if ([self.resizedImages objectForKey:key]) {
    return [self.resizedImages objectForKey:key];
}

If another thread called setObject:forKey: with that key the result of the 2nd line will be different from the first line. Why not just store it in a variable? At least the thing you check and the thing you return will be the same.

UIImage* image = [[[[self.resizedImages objectForKey:key] retain] autorelease];
if (image) {
    return image;
}

(The retain + autorelease is just to retain image won't be -dealloc-ed when this thread is running.)

If you have to ensure that the key corresponding to that image will not be overwritten before the function returns, you'll need to use a lock, as others answered.

Edit: Perhaps you should use an NSMutableSet of the UIImage instead. Especially, don't use a hash value as a key! A hash value does not need to be different when the two objects are not the same. Create a new class X which contains a CGSize and UIImage property, and use a NSMutableSet of X, if the same UIImage can be added multiple times.

edited Jan 17 '12 at 19:30

answered Jan 17 '12 at 19:12

kennytm

510,854
105
1,084
1,005

Thanks! First off, this is all under ARC, just to simplify. Secondly, I don't mind if somebody else subs the object for my key, because it SHOULD be pointing to the same image, resized the same way. Now I'm beginning to suspect my hash function, which looks quite wrong. – Dan Rosenstark Jan 17 '12 at 19:20
@Yar: If it's in ARC then just leave off the `retain` and `autorelease` because a __strong variable should have done that for you. – kennytm Jan 17 '12 at 19:25
Thanks @KennyTM, I will try a custom key class that takes a `CGSize` and a `UIImage`, carefully overriding `isEqual:` – Dan Rosenstark Jan 17 '12 at 19:54

score -1 · Answer 3 · answered Jan 17 '12 at 18:16

-1

NSMutableDictionary is not thread safe. It is up to you to provide locking.

answered Jan 17 '12 at 18:16

Rob Napier

286,113
34
456
610

Are you answering the title to the question, or the question? – Dan Rosenstark Jan 17 '12 at 19:06

NSMutableDictionary Not Threadsafe: So?

3 Answers3