Re: Proper way to create a singleton without @synchronized ?
Re: Proper way to create a singleton without @synchronized ?
- Subject: Re: Proper way to create a singleton without @synchronized ?
- From: Kyle Sluder <email@hidden>
- Date: Sat, 16 Apr 2011 20:31:50 -0700
On Sat, Apr 16, 2011 at 8:04 PM, WT <email@hidden> wrote:
> From the testing I've done - creating several threads, each invoking either [MySingleton sharedInstance] or [[MySingleton alloc] init] after a randomly selected sleep time, and running the test anew several times - it appears to behave correctly, that is, the same instance is used in every thread, it's always properly initialized (all "instances" have the same value for someInteger), and each of the three dispatch_once() blocks is executed exactly once, no matter how many threads were created.
Do you really need a singleton, or just a default instance? For
example, NSFileManager has a default instance, but you can still
alloc/init your own if you want.
If all you need is a shared instance, then this will work just dandy
for any class:
+ (id)sharedInstance {
static id sharedInstance;
static dispatch_once_t once;
dispatch_once(&once, ^{ sharedInstance = [[self alloc] init]; });
return sharedInstance;
}
As for your code:
> // MySingleton.h:
>
> #import <Foundation/Foundation.h>
>
> @interface MySingleton: NSObject
> {
> NSUInteger someInteger_;
> }
>
> @property (readwrite, nonatomic, assign) NSUInteger someInteger;
>
> + (MySingleton*) sharedInstance;
>
> @end
>
> // MySingleton.m:
>
> #import <dispatch/dispatch.h>
> #import "MySingleton.h"
> #import <stdlib.h>
> #import <time.h>
>
> static MySingleton* stSharedInstance = nil;
>
> static dispatch_once_t stSharedInstanceInvoked;
> static dispatch_once_t stAllocWithZoneInvoked;
> static dispatch_once_t stInitInvoked;
All of these could be made static variables within the method body,
increasing readability and reducing pollution of the global scope
without affecting storage longevity.
>
> @implementation MySingleton
>
> @synthesize someInteger = someInteger_;
>
>
> + (MySingleton*) sharedInstance;
> {
> dispatch_once(&stSharedInstanceInvoked,
> ^{
> NSLog(@"+sharedInstance block entered");
>
> // Assignment done in +allocWithZone:.
> // The autorelease message is sent to prevent XCode's
> // static analyzer from issuing a warning about a possible
> // memory leak. There is no leak, since the singleton is
> // not meant to be deallocated, and the autorelease message
> // does nothing, as per its overridden implementation below.
Rather than relying on -allocWithZone: to return the shared instance,
why not assign to stSharedInstance here?
>
> [[[[self class] alloc] init] autorelease];
You are in a class method. self == [self class], so no need to call +class here.
>
> NSLog(@"+sharedInstance block exited");
> });
>
> return stSharedInstance;
> }
>
>
> + (id) allocWithZone: (NSZone*) zone;
> {
> dispatch_once(&stAllocWithZoneInvoked,
> ^{
> NSLog(@"+allocWithZone: block entered");
>
> stSharedInstance = [super allocWithZone: zone];
>
> NSLog(@"+allocWithZone: block exited");
> });
>
> return stSharedInstance;
Of course this means you can't subclass this class.
> }
>
>
> - (id) init;
> {
> __block id tempSelf = self;
Because the block is not going to be copied, you do not need to worry
about creating a __block version of self here. And since this is a
singleton, even if the block *were* copied (and thus self retained),
you still wouldn't care.
>
> dispatch_once(&stInitInvoked,
> ^{
> NSLog(@"-init block entered");
>
> tempSelf = [super init];
>
> if (tempSelf)
> {
> someInteger_ = random() % 1000;
> NSLog(@"Instance initialized");
> }
>
> NSLog(@"-init block exited");
> });
>
> self = tempSelf;
> return self;
> }
>
>
> - (id) copyWithZone: (NSZone*) zone;
> {
> return self;
By virtue of this implementation, you can't implement NSCopying in a
subclass. But you can't subclass this class anyway because of
+allocWithZone:.
> }
>
>
> - (id) retain;
> {
> return self;
> }
>
>
> - (unsigned) retainCount;
> {
> return UINT_MAX; // Denotes an object that cannot be released.
Why bother? All this will do is mask any bugs where your singleton disappears.
> }
>
>
> - (void) release;
> {
> /* do nothing */
Don't do this. As above, you're just masking cases where you
accidentally over-release your singleton.
> }
>
>
> - (id) autorelease;
> {
> return self;
> }
Look, you're going to great lengths to ensure you never create more
than one instance of this class. Is any of it really necessary?
--Kyle Sluder
_______________________________________________
Cocoa-dev mailing list (email@hidden)
Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com
Help/Unsubscribe/Update your Subscription:
This email sent to email@hidden