gnustep-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Question about memory management


From: David Chisnall
Subject: Re: Question about memory management
Date: Fri, 7 Jun 2013 09:26:03 +0100

On 7 Jun 2013, at 07:29, Maxthon Chan <address@hidden> wrote:

> Just asking, with ARC, is this a good choice on implementing singleton?

No, it's not thread-safe.

> //Singleton.h
> #import <Foundation/Foundation.h>
> @interface Singleton : NSObject
> + (instancetype)defaultSingleton;
> // …
> @end
> extern Singleton *DefaultSingleton // Of course this is optional
> 
> // Singleton.m
> Singleton *DefaultSingleton

This should be declared static, as the variable should not be exposed outside 
of the Singleton.m compilation unit.

> @implementation Singleton
> + (instancetype)defaultSingleton
> {
>       if (!DefaultSingleton)
>               DefaultSingleton = [[self alloc] init];

If two threads call this method at once, then both will enter the body of this 
if statement, both will allocate instances of the singleton and one version 
will leak.

The correct way of implementing a singleton is to create it in the +initialize 
method, which is guaranteed to be thread-safe.  My preferred pattern is:

+ (void)initialize
{
        [[self alloc] init];
}
+ allocWithZone: (NSZone*)aZone
{
        if (DefaultSingleton != nil)
        {
                [NSException raise: NSInvalidArgumentException format: 
@"Attempted to create multiple instances of singleton %@", self];
        }
        DefaultSingleton = [super allocWithZone: aZone];
        NSAssert(nil != DefaultSingleton, @"Allocation of singleton instance of 
%@ failed!", self);
        return DefaultSingleton;
}

Calling the constructor in +initialize means that the first time ANY message is 
sent to this class, the singleton will be created and no other thread will be 
allowed to send messages to this class until the singleton is fully initialised.

The nil check in +allocWithZone: is safe, because the variable will be set with 
a lock held and can then be safely queried because it will never transition 
from non-nil to nil.  This exception prevents people from calling +alloc on the 
object.

Assigning the value to DefaultSingleton in +allocWithZone: instead of 
+initialize means that calls to things from the -init method don't have to have 
special cases to check the presence of the singleton.  

For completeness, you may also include NSCoding methods that ensure that the 
singleton is correctly created when serialising / deserialising.  It would be 
nice if there were an NSSingleton protocol that included a +sharedInstance 
method so that the serialiser code could know to always replace any references 
to the class with the shared instance, but there isn't.  

David




reply via email to

[Prev in Thread] Current Thread [Next in Thread]