Objective-C - Optimizing this singleton pattern?

Question

I've found that singleton pattern on the net. It seems to me it has many things that can be optimized.

-In sharedMySingleton method, no need to call a retain ? I'm not sure...
-If not, why is there a retain in allocWithZone ?
-what is the use of @synchronized. The NSAssert make think that the block can be called many times, so if yes, there should be some more code to release previous memory, or exit the block clearly without just NSAsserting, and if no, why is there this NSAssert ?
-the chain beetween sharedMySingleton and alloc seems strange. I'd wrote myself something like :

+(MySingleton*)sharedMySingleton
{
    @synchronized([MySingleton class])
    {
        if (_sharedMySingleton == nil) _sharedMySingleton = [[self alloc] init];
        return _sharedMySingleton;
    }

    return nil;
}

+(id)alloc
{
    @synchronized([MySingleton class])
    {
        return [super alloc];
    }

    return nil;
} 

Singleton pattern

#import "MySingleton.h"

@implementation MySingleton

// ##########################################################################################################
// ######################################## SINGLETON PART ##################################################
// ##########################################################################################################
static MySingleton* _sharedMySingleton = nil;

// =================================================================================================
+(MySingleton*)sharedMySingleton
// =================================================================================================
{
    @synchronized([MySingleton class])
    {
        if (_sharedMySingleton == nil) [[self alloc] init];
        return _sharedMySingleton;
    }

    return nil;
}

// =================================================================================================
+(id)alloc
// =================================================================================================
{
    @synchronized([MySingleton class])
    {
        NSAssert(_sharedMySingleton == nil, @"Attempted to allocate a second instance of a singleton.");
        _sharedMySingleton = [super alloc];
        return _sharedMySingleton;
    }

    return nil;
} 

+ (id)allocWithZone:(NSZone *)zone  { return [[self sharedMySingleton] retain]; }
- (id)copyWithZone:(NSZone *)zone   { return self; }
- (id)retain                        { return self; }
- (NSUInteger)retainCount           { return NSUIntegerMax;  /* denotes an object that cannot be released */}
- (oneway void)release              { /* do nothing */ }
- (id)autorelease                   { return self; }

// ##########################################################################################################
// ##########################################################################################################
// ##########################################################################################################

// =================================================================================================
-(id)init 
// =================================================================================================
{   
    if (!(self = [super init])) return nil;

    return self;
}

// =================================================================================================
-(void) dealloc
// =================================================================================================
{
    [super dealloc];
}

// =================================================================================================
-(void)test 
// =================================================================================================
{
    NSLog(@"Hello World!");
}

@end

Answer 1

You shouldn't use this pattern at all (it's for a very special case of Singleton that you almost never need, and even in that case you generally shouldn't use it).

There are many good patterns discussed at What should my Objective-C singleton look like?, but most of them are outdated since the release of GCD. In modern versions of Mac and iOS, you should use the following pattern, given by Colin Barrett in the linked question:

+ (MyFoo *)sharedFoo
{
    static dispatch_once_t once;
    static MyFoo *sharedFoo;
    dispatch_once(&once, ^{ sharedFoo = [[self alloc] init]; });
    return sharedFoo;
}

I only copy it here rather than marking the question duplicate because the old question's highest-rated answers are out-dated.

Answer 2

In sharedMySingleton method, no need to call a retain?

alloc returns a new instance with a reference count of one, so no retain is necessary.

If not, why is there a retain in allocWithZone?

According the rules, when you call allocWithZone, you own the reference, therefore allocWithZone much increase the reference count for you. But this implementation of allocWithZone is returning the singleton instance that was already created and owned by someone else (the sharedMySingleton method). So the sharedMySingleton method creates the object with alloc, therefore it becomes an owner. And then you get the same instance through allocWithZone, therefore you become a second owner of the same instance. So the retain count must go up since there are two owners now. That is why allocWithZone needs to retain.

What is the use of @synchronized?

@synchronized allows code to be called simultaneously by multiple threads. If you will never call sharedMySingleton from more than one thread, then it is not necessary and you may omit it.

The NSAssert make think that the block can be called many times, so if yes, there should be some more code to release previous memory, or exit the block clearly without just NSAsserting, and if no, why is there this NSAssert?

Because the class is intended to be a singleton, alloc should only be called once. The NSAssert() terminates the program if alloc is called more than once. Since NSAssert() terminates the program, when alloc is called a second time, no memory management is necessary.

Answer 3

No need to call retain because there is an alloc. Call retain on top of that would cause a memory leak. There is retain on allocWithZone because it's a singleton, so we don't want to create two different instances of the class. Rather allocating a new instance we augment the retain count of the singleton instance. Why is that? Probably to prevent somebody not aware of the singleton type of the class. If he calls allocWithZone and then release the instance, everything still work fine, and he actually accessed the shared singleton instance.

@synchronized is used to prevent two calls from two different threads to enter in the if statement at the same time. So the code is thread-safe.

The NSSAssert is probably to make the app 'crash' if two instances of the singleton are ever created. It's 'just-to-be-sure' code, also called defensive programming.

Concerning the chain beetween sharedMySingleton and alloc, I think it's just fine.