Thread Safty Deja Vu (retain/autorelease or lock/unlock?)
Thread Safty Deja Vu (retain/autorelease or lock/unlock?)
- Subject: Thread Safty Deja Vu (retain/autorelease or lock/unlock?)
- From: Brant Vasilieff <email@hidden>
- Date: Mon, 23 Jul 2001 00:13:18 -0700
I recently discovered a problem with my threaded code. Basically, it
happens, when an accesser returns an object that is then released by a
second thread before it can be used. The solution is giving me that
deja vu feeling, but I can't find where I saw it before.
To start with, consider the very simple widget class:
@implementation Widget : NSObject
{
NSString* name;
}
- (NSString*)name;
- (void)setName:(NSString*)inName;
@end
@implementation Widget
- (NSString*)name
{
return name;
}
- (void)setName:(NSString*)inName
{
id oldName = name;
name = [inName retain];
[oldName release];
}
@end
Also assume that a static variable sharedWidget has already been created
before these two threads execute.
- (void)thread1
{
NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
{
NSString* name = [sharedWidget name]; // 1
...
NSSize size = [name sizeWithAttributes:nil]; // 2
}
[pool release];
}
- (void)thread2
{
NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
{
[sharedWidget setName:@"A New Name"];
}
[pool release];
}
What can happen, is that calling name on the sharedWidget in thread1
returns the old name of the widget. Then thread2 executes releasing the
old name. When thread1 continues at step 2, it attempts to get the text
size of a released string and fails.
So to prevent this, I temporarily changed the fetch to:
NSString* name = [[[sharedWidget name] retain] autorelease];
This fixes the problem, locally, but now my questions. I have two
approaches. Either change all accesser methods to return an
autoreleased object within the scope of the current threads autorelease
pool, or lock the widget when using it.
So in solution 1 the name accesser would look like:
#if !defined ThreadSafeReturn
#define ThreadSafeReturn(value) \
return [[value retain] autorelease];
#endif
- (NSString*)name
{
ThreadSafeReturn( name );
}
Solution 2 would add an NSLock to the Widget class, so my threads would
look like:
- (void)thread1
{
NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
[sharedWidget lock];
{
NSString* name = [sharedWidget name]; // 1
...
NSSize size = [name sizeWithAttributes:nil]; // 2
}
[sharedWidget unlock];
[pool release];
}
- (void)thread2
{
NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
{
[sharedWidget lock];
[sharedWidget setName:@"A New Name"];
[sharedWidget unlock];
}
[pool release];
}
Any comments on which approach would be more efficient?
I've always tried to keep locks down to a minimum. The reason there's
no lock within setName, is that, it was OK to get an old value for the
name, as a notification to redraw the window would be sent soon after.
Solution1 seems to be the most convenient, and easiest of the two to
implement without introducing deadlock situations.
Also, I'm curious what the overhead of including an NSLock object in
each widget would entail. Is it just additional memory? Does each
NSLock maintain it's own queue of threads to signal when it's unlocked?
If I had thousands of widgets, is there going to a performance penalty
beyond those due to the extra memory footprint?
TIA,
Brant