Singletons in Cocoa: Doing them wrong

2009-06-17 15:19:51 -08:00

UPDATE 2010-09-04: I wrote this post about an older version of Apple’s singleton example. The current version is much better: It still doesn’t handle over-releases the way I would like, but there’s nothing objectively wrong with it. As such, there’s nothing wrong with using Apple’s current example. You can continue reading this post to see my criticism of Apple’s old singleton example and my own implementation.


A singleton is a simple concept: It’s an object that you can only have one of. Being that simple, you may think it’s hard to get wrong.

If you do, I welcome you to the wonderful world of programming.

The Cocoa Fundamentals Guide is necessary reading for all Cocoa programmers. Even so, nobody’s perfect, and the CFG’s authors are no exception.

On Twitter last night, a mistake of theirs came to light when someone named Nick tweeted about how hard it is to make a singleton in Cocoa. Nick cited an example from the CFG; Chris Hanson responded that “that example does WAY too much work”, and Bill Bumgarner added that the example is “ridiculous”. They‘re were both right.

Doing it wrong.

The example, in order to “ensure singleton behavior”, overrides four basic Foundation methods:

  • retain
  • release
  • autorelease
  • retainCount

In the override implementations, retain, release, and autorelease are all no-ops. release does exactly nothing; the other two do nothing but return self. The override implementation of retainCount unconditionally returns UINT_MAX.

Before I proceed any further, I’ll give you a caution about singletons. You generally do not need to make your object a singleton at all. Even if your application has a single-window interface, with only one of each controller, you probably don’t need to enforce that. Just don’t create more than one. Then, if you do create more than one, you have a bug.

Let’s digress for a moment. Imagine if you allowed it. Imagine that you allow, say, your app delegate to create more than one of your root controller object. Your app delegate will then proceed to set up both twin objects, and they both respond to such things as notifications.

Bad, right? Now let’s say you “fix” this by making your root controller a singleton.

Your app delegate is still trying to create two root controllers. It’s still trying to set up two root controllers, and as a result of the two set-up messages, the controller is signed up twice for notifications.

But now you really only have one root controller. Your app delegate creates the first one, then gets it again when it tries to create the second. All according to plan so far. Then your app delegate sets up the first object—and then it sets it up again, again thinking that it’s talking to a second object. Even worse, because the object is now signed up twice for notifications (once per set-up message), every notification response happens twice.

You now have only one root controller, but you didn’t fix the bug, which wasn’t in the controller at all, but in the app delegate. To fix the bug, you must fix the app delegate; you don’t need a singleton for this at all.

OK, digression over. Singletons are bad. Avoid them. If you have a lot of them, rip most of them out. (Dave Dribin has bookmarked a lot of other good cases against singletons, and BJ Homer points out that they aren’t all bad. More on BJ’s post later.)

Back to the singleton at hand.

First, let’s look at retainCount. A comment explains that UINT_MAX “denotes an object that cannot be released”, implying that the framework itself considers UINT_MAX a special value.

This is actually correct, although I originally thought (and wrote, in an earlier version of this post) that it was bogus. The documentation for retainCount explicitly says that objects whose release method does nothing should return UINT_MAX.

Next on the hit list: autorelease. This is just mostly pointless. autorelease means nothing more than “send this a release message later”. If release does nothing, then autorelease becomes “do nothing later”. All this override does is move the nothing up to now. A performance optimization, perhaps, but I’d say it’s premature.

And now we come to the real villains: retain and release.

First off, you shouldn’t retain a singleton anyway. I can’t imagine why you would do this, except incidentally, by making it a timer target or something.

But even if you do think of a reason to retain a singleton, you still need to balance retain and release. If you retain without releasing afterward, you are under-releasing the object. If you release without retaining previously, you are over-releasing the object. These two statements have no exceptions.

If you break retain and release, then you’re able to over-retain or over-release (or even both!) the object with impunity. Your app no longer crashes, but you didn’t really fix the problem; you’re just getting away with it. You’re still trying to over-retain and/or over-release an object.

The Cocoa Fundamentals Guide’s primary audience is new Cocoa developers. Those who have never used a retain-counting system before may very well over-release the singleton, and Apple’s example singleton implementation hides that bug from them. That’s bad; every Cocoa programmer should know how to recognize an over-release crash, and breaking retain and release denies the reader an opportunity to learn that. I’ve filed a documentation bug report about the example.

Also, a rebuttal

BJ Homer responds at a different angle to last night’s flurry of tweets.

First, though, we need to get a definition straight. A singleton is a class of which there should only be one instance in any given process. There are actually very few singleton classes in the Cocoa framework including NSDocumentController and NSFontManager. You cannot create more than one of these objects; …

This is true. A singleton object is an object that your app can only have one of. There are a couple of different ways to do this:

… if you try to call [[NSDocumentController alloc] init], you’ll get back the exact same object as you do when you call [NSDocument sharedDocumentController], no matter how many times you call alloc and init. NSApplication is arguably a singleton as well; you can alloc another one, but you’ll get an assertion failure when you call init.

Another way is to simply implement the sharedFramistan method, and leave allocWithZone: and init alone. I suspect that this is common, since it’s the easiest way, but BJ is right that it isn’t a true singleton, since it allows multiple instances.

Where BJ goes wrong is in his defense of the retain and release overrides:

Consider the following example:

MyFloozit *floozit1 = [[MyFloozit alloc] init];
[floozit1 doSomething];
[floozit1 release];

MyFloozit *floozit2 = [[MyFloozit alloc] init]; // MyFloozit is a singleton, so this should be the same object as floozit1
[floozit2 doSomething]; // CRASH HERE
[floozit2 release];

We’ll leave aside the problem that you probably should be using sharedFloozit to obtain the singleton instance of MyFloozit.

When floozit1 is set, a new MyFloozit is allocated, and a static MyFloozit pointer is set. When floozit1 is released, that static pointer is still pointing to the old instance. As a result, when we try to set floozit2 (or when anyone else tries to call [MyFloozit sharedFloozit]), we get back a pointer to that same instance. The one that has been dealloc-ed.

BJ is missing something else the CFG says, which he even quoted later on:

Situations could arise where you want a singleton instance (created and controlled by the class factory method) but also have the ability to create other instances as needed through allocation and initialization. …

(Emphasis added by me.)

A singleton object owns itself. As such, it retains itself. As such, it should never die, because it always has at least one owner—and that’s without breaking retain and release. If it does die, it’s because something over-released it; later, something that was using it will crash, which lets you know that you have a bug. This is a good thing, because now you can fix the bug.

Each of the release messages in BJ’s example balances an alloc message above it. That alloc message may actually return an existing object, but we’re expecting to own a new object. Therefore, the singleton’s allocWithZone: should implicitly retain the existing object.

There is no good reason to override retain and release. Don’t do it. This also goes for autorelease. And, since you never override release, you also never need to override retainCount.

Doing it right

So, having thoroughly dismantled the [old] Apple documentation’s poor implementation of a singleton, let’s look at the correct way to do it.

Let’s go through the requirements:

  • The One True Instance is the only instance. (If you deliberately allow multiple instances, I call this a “multipleton”. I’ll leave that as an exercise for the reader, and concentrate on the true singleton here.)
  • There is a sharedFramistan method. It tests whether the One True Instance exists; if not, it creates the object and remembers it in file-scope static storage. Then it returns the One True Instance.
  • We’ll allow going through alloc and init, and return the same instance. We’ll do this in allocWithZone:, as Apple did. We’ll also need to make sure init doesn’t do its work twice on the same object.

[UPDATE 2010-09-04: I used to have the code inline in the post here; I have since moved it to a Mercurial repository on Bitbucket. You can download PRHEmptySingleton.h and .m from there.]

This is actually even more complex than Apple’s example, but it passes all six of these tests:

  1. sharedFramistan always returns the same object.
  2. alloc/init always produce the same object (which, itself, is the same object as sharedFramistan).
  3. alloc/init will not return an object confused by multiple init messages.
  4. Over-releasing causes a crash.
  5. Keeping alloc/allocWithZone:/retain balanced with release never causes a crash.
  6. If [super init] returns a different object, alloc/init won’t break.

Apple’s [old] example fails test 4. BJ doesn’t show an implementation for his example, but according to his own description of its behavior, it fails test 5. My version passes all six.

Take-aways

Hiding bugs is bad. Even worse is giving code that can hide a bug to new Cocoa programmers who could really use practice in detecting and fixing that kind of bug.

If you really need to implement a singleton, there is a right way to do it. The way currently shown in the Cocoa Fundamentals Guide isn’t it.

Don’t change the behavior of retain, release, retainCount, or autorelease. Ever.

UPDATE 2009-09-04: Removed hasInited instance variable after Christiaan Hofman pointed out its redundancy in the comments.

UPDATE 2009-09-19: Moved assignment of the sharedInstance static variable from +initialize to -init, as suggested by Uli Kusterer.

UPDATE 2010-09-04: Replaced inline code sample with links to the Mercurial repository for the sample.

34 Responses to “Singletons in Cocoa: Doing them wrong”

  1. Colin Barrett Says:

    For the sake of argument, what’s wrong with this?

    @interface PRHEmptySingleton: NSObject
    {
    }
    + (id)sharedFramistan;
    @end
    
    static PRHEmptySingleton *sharedInstance = nil;
    
    @implementation PRHEmptySingleton
    
    + (id) sharedFramistan {
        if (!sharedInstance) {
            sharedInstance = [[self alloc] init];
        }
        return sharedInstance;
    }
    
    - (id) init {
        if ((self = [super init])) {
            // Init here
        }
        return self;
    }
    
    @end
    
  2. Kevin Ballard Says:

    Your singleton implementation isn’t thread-safe, btw.

  3. Peter Hosey Says:

    Colin: Doesn’t trap alloc/init, so you can create multiple instances. If done intentionally, that’s a multipleton. If done inadvertently, it’s only loosely a singleton.

    Kevin Ballard: True. I don’t think about threading unless I have to. That’s rare, thanks to the run loop and (in 10.5.7 and later) NSOperation. I think thread safety would only require synchronization in initialize, but I haven’t tested that theory.

  4. Patrick Burleson Says:

    According to the docs for NSObject, there’s this comment in +(void)initialize:

    The runtime sends the initialize message to classes in a thread-safe manner.

    So I think Peter’s implementation is thread safe.

    For reference: http://developer.apple.com/documentation/Cocoa/Reference/Foundation/Classes/NSObject_Class/Reference/Reference.html#//apple_ref/doc/uid/20000050-initialize

  5. Patrick Burleson Says:

    Peter, thanks for making this post. By making the changes outlined here, I was able to finally get rid of a Static Analyzer memory leak warning in a Singleton I had from some third party code. All seems to be working well.

  6. Peter Hosey Says:

    Patrick Burleson: That’s interesting, because the example I gave still doesn’t release the singleton object. Other objects are free to release it, but the singleton itself never releases itself. If the analyzer is smart enough to recognize this as a singleton and not a leak, I’m impressed.

  7. Patrick Burleson Says:

    Peter: I’m not sure if it’s a bug in the analyzer or not, but the Singleton example provided by the Apple docs triggers a leak warning in the +shared method. Your version does not. I’m hoping it’s just the analyzer being super smart. :-)

  8. Peter Hosey Says:

    Yeah, I just ran the analyzer on my test app (which is fuller than the simplified example I presented, mainly having a string property and some test cases in the app delegate), and the only bug it reported is the intentional over-release among the test cases. Interesting.

  9. Cédric Luthi Says:

    What about raising exceptions in retain/release/autorelease and in alloc/init the second time they are called ? The exception would say something like Framistan is a singleton and must be accessed through the sharedFramistan class method.

    Also interesting:

    printf("framistan (shared) %p\n", [PRHEmptySingleton sharedFramistan]);
    printf("framistan (alloc/init) %p\n", [[PRHEmptySingleton alloc] init]);
    printf("workspace (shared) %p\n", [NSWorkspace sharedWorkspace]);
    printf("workspace (alloc/init) %p\n", [[NSWorkspace alloc] init]);

    will output:

    framistan (shared) 0x109250
    framistan (alloc/init) 0x109250
    workspace (shared) 0x109920
    workspace (alloc/init) 0x109980

    So it seems to me that NSWorkspace, and actually other Cocoa singleton classes, are implemented as what you called a multipleton.

  10. Peter Hosey Says:

    Cédric Luthi: Throwing an exception in allocWithZone: or init would work. Doing so in retain and release would prevent you from setting the object as a timer target, since the timer will retain it.

  11. BJ Homer Says:

    Well reasoned. I accept your recommendations.

  12. Jonathan Wight Says:

    \”That\’s rare, thanks to the run loop and NSOperation\”

    Huh? What part of NSOperation protects you from threads?

    Also I\’m with Colin. The simple singleton is fine. Doing strange stuff in alloc, init and dealloc is wankery that serves little to no purpose.

  13. Peter Hosey Says:

    Jonathan Wight: You don’t have to use threads with NSOperation. The one idea I had for an app that would use it would have used subprocesses instead. (I never went through with it, though. I don’t even remember now what it was.)

  14. Jonathan Wight Says:

    I suppose you don’t have to use threads with NSOp. But really, 99% of the time, you will.

  15. GR Says:

    When does the static sharedFramistan get deallocated?
    If it has retained other objects, can I just implement a dealloc method to release those objects?

  16. Peter Hosey Says:

    GR: As shown, it doesn’t. If that matters to you, you could observe for NSApplicationWillTerminateNotification on the local notification center, and release the singleton object (and set the variable to nil) in your notification handler.

    As for dealloc, yes, with one caveat: If you don’t release the singleton in a NSApplicationWillTerminateNotification handler, the class owns its instance until the process exits, so the instance never formally dies, so its dealloc never gets called. Only if you do implement that NSApplicationWillTerminateNotification handler does dealloc get called (as usual).

  17. Sean Says:

    Apple’s example is also not correct in 64 bit. Their UINT_MAX should be NSUIntegerMax.

  18. Peter Hosey Says:

    Sean: Not according to the NSObject documentation. It explicitly says UINT_MAX.

    It’s possible that they’re both wrong. You should file a documentation bug.

  19. Sean Says:

    Peter: I’m pretty sure the docs are wrong, many of them are still not fully updated for 64 bit. A similar example: NSNotFound. The docs still show 0x7fffffff, but the .h changed from INT_MAX to NSIntegerMax. And sample code gets way less love than the docs.

  20. Christiaan Hofman Says:

    You could make it even simpler. The hasInited ivar is totally unnecessary, you can just as well check whether sharedInstance is nil. Think about it.

    Also what I miss in your discussion is the reason why you need to override init. The reason is that when you need to instantiate an instance in a NIB, the NIB loader will always use alloc/init, it won’t be using your custom class factory method. Of course in code you should always use the class factory method.

  21. Peter Hosey Says:

    You could make it even simpler. The hasInited ivar is totally unnecessary, you can just as well check whether sharedInstance is nil. Think about it.

    Think about this: Where is sharedInstance assigned?

  22. Christiaan Hofman Says:

    Peter: Well, it’s assigned in +initialize. This is the first thing that happens, but the assignment takes place after the initial call to -init (because a fully initialized instance is assigned). This means that the first instance that’s allocated and initialized is guaranteed to be the one that will be assigned to sharedInstance, while it is also guaranteed that sharedInstance is assigned whenever another explicit call to +sharedInstance, +allocWithZone: or -init is executed. You think about that.

  23. Peter Hosey Says:

    Christiaan Hofman: Ah, I see now. You’re right, and I apologize.

    To keep your comments in context, here’s a record of what the code said:

    + (void) initialize {
        if (!sharedInstance)
            sharedInstance = [[self alloc] init];
    }
    
    - (id) init {
        if (!hasInited) {
            if ((self = [super init])) {
                //Initialize the instance here.
    
                hasInited = YES;
            }
        }
    
        return self;
    }

    hasInited was an instance variable.

  24. Uli Kusterer Says:

    Just thought I’d mention that Peter has pointed out this posting in a comment to my article on Defensive coding in Objective C and we’re working out a few more details of this code there. The curious among the uninterested might wanna check that out.

  25. Uli Kusterer Says:

    Peter, one more suggestion: One of the uses for Singletons is that you can have a project-specific subclass of a singleton (if you don’t need that, why bother with a specially-implemented signleton? Why not just use a class with class methods?).

    E.g. I have an “alert manager” singleton that my code uses to show error messages. One app has a fullscreen mode where I want to display custom alerts that can be controlled with the Apple Remote, while it is in fullscreen mode. So I create a fullscreen alert manager subclass that knows the specific fullscreen stuff for that one app. All code just asks for [MyAlertManager sharedManager]. In the main() function of the fullscreen app, I do a [MyFullscreenAwareAlertManager sharedManager]. Since this is called first, the alert manager singleton is actually an instance of the subclass. All calls to the superclass in shared code modules now transparently end up with the subclass instead Shared code doesn’t need to be aware of fullscreen support, no ugly ifdefs, no unnecessary dependencies on the fullscreen display code of one app in other apps.

    For this to work, I can’t use the +initialize method you have here (because the superclass’s initialize always gets called before the subclass), instead, I’m doing that work in my sharedManager method. I use [self class] so the subclass automatically inits a subclass instance.

    The only disadvantage of this technique is if other code accidentally refers to a third subclass of the base class first, my calls to the second subclass will go to the wrong kind of object.

  26. Peter Hosey Says:

    I want to display custom alerts that can be controlled with the Apple Remote, while [the app] is in fullscreen mode.

    This implies that it can be switched in and out of full-screen mode. Is that correct?

  27. Christiaan Hofman Says:

    Peter, you can still improve on your code, because it now does not properly follow the memory management rules locally. As a result, Clang will give warnings. In +initialize you have a retained object without assignment, so to follow the rules you should add an extra autorelease. In -init you have two assignments that are not properly retained. So the assignment of sharedInstance should read sharedInstance = [self retain];, as this is the place where the class will take ownership of this instance. Also, the opposite assignment later on should read self = [sharedInstance retain];, the extra retain essentially balances the release in the line above it. Of course this code path should actually never be called.

    And the code in +initialize could easily go into +sharedInstance for lazy evaluation.

  28. Peter Hosey Says:

    In -init you have two assignments that are not properly retained.

    More like properly not retained. If the caller goes directly to alloc/init instead of going through sharedInstance, retaining the receiver in init will over-retain it (implicit retain by alloc + explicit retain in init). Thus, explicitly retaining in init is not an option if I want to maintain compatibility with the alloc/init path. Without explicitly retaining in init, an autorelease in sharedInstance would probably be the final release and would cause the object’s death.

    I’m not sure what to do about the Clang warnings. Second static variable, perhaps? (Using the first one in both places would probably beget a dead store warning.)

  29. Christiaan Hofman Says:

    I disagree completely. You should look at memory management rules locally, as Clang also does, and then everything works out naturally an correctly.

    The point is that in sharedInstance = self, the sharedInstance is assigned and ownership is passed to the class, which means that the instance must be explicitly retained. In your code this extra retain count comes from the line in +initialize, which is wrong, because that’s not the point where ownership is passed (the shared instance is assigned). The line [[self alloc] init] has a net retain count of 1, but there’s no assignment, so nobody taking ownership. Therefore it goes against the memory management rules. To make this more obvious, think of introducing a setter method +setSharedInstance: to assign the shared instance; the implementation of that method should contain a retain as is normal for a setter. Just do the count when you follow my recommendations (all three of them).

    As for the line self = sharedInstance, remember that self should have a retain count of one coming from the creation independent of what else happens, that is, not counting the global retain count of 1 for the shared instance. This means that it needs one extra retain. Think about when somehow the code would go through this line as it is now: in that case the retain count for sharedInstance would be the same, even though an alloc/init is used to get it, so a following release on the returned object should be valid. Let me rephrase this. Using [[[PRHEmptySingleton alloc] init] release] should be OK. However, if for some reason this would use the last part (for the sake of the argument), then this would lead to the retain count of sharedInstance to be reduced by one, and it would become invalid.

    In short, this is the correct code.

    + (void) initialize {
        if (!sharedInstance) {
            // Nobody takes ownership here, so retain count should be balanced to 0
            [[[self alloc] init] release];
        }
    }
    
    + (id) sharedFramistan {
        return sharedInstance;
    }
    
    + (id) allocWithZone:(NSZone *)zone {
        // We should have one more overall retain because it's alloc
        return [sharedInstance retain] ?: [super allocWithZone:zone];
    }
    
    - (id) init {
        if (!sharedInstance) {
            if ((self = [super init])) {
                //Initialize the instance here.
            }
            // The class takes ownership over sharedInstance here, so retain explicitly, think of a setter, think of replacing it with
            // [[self class] setSharedInstance:self];
            sharedInstance = [self retain];
        } else if (self != sharedInstance) {
            // We reduce overall a retain count here...
            [self release];
            // ...so we need to balance with an extra retain count here. We should be allowed to follow this method with a further retain.
            self = [sharedInstance retain];
        }
    
        return self;
    }
  30. Peter Hosey Says:

    I see your meaning now. Thanks. I think I’ll put this whole thing in an Xcode project in version control to handle changes like this in the future (and make adoption easier). I’ll probably do this before CocoaHeads this week.

  31. Kendall Helmstetter Gelner Says:

    I have to disagree on the whole retain/release thing.

    Your whole argument about leaving it out is that you are “hiding bugs”. But if you override retain/release, by definition it is not a bug to call release on that object, you have changed the meaning for the class in a way that is compatible with the understanding the developer would have about how retain/release work.

    Instead what you are doing, is making your application less robust and purposefully introducing a single point of failure that will probably lead to a crash. You are trying to make the code outside your class a little better, at the cost of potential frustration to the application owner who should at all times be paramount in your thoughts as an application developer.

    The trouble is you think you are protecting bad coders from themselves but instead you are spreading code with a very nasty subtle bug that might well be overlooked by many programmers. What happens if in testing they only make use of the singleton once, and then call release? Everything works of course because the invalid reference is never used. But then the application reaches the real world, and the infrequently used singleton is finally accessed twice… you have not helped the poor developer at all, you have introduced a crashing bug to thousands of users and forced the developer to learn all about desymbolication.

    In fact you aren’t even putting just bad coders in peril. Lets say you get someone who knows memory management pretty well but forgets they are using a singleton class. So, they alloc/init and then when done they call release. Again, the running code comes back sometime later to the same point and calls alloc/init again….

    They are screwed, even though they followed the correct memory management rules. Because you did not override release, the shared instance variable you check in init is not nil, but is now invalid! So your init code proceeds to return the now invalid instance, and crashing ensues. What did they do to deserve this cruel trick? They were again decent developers following the memory management rules, the only crime was that they did not read the source for the class they tried to use.

    At the very minimum, you should override release to throw an exception in debug mode only and not actually release the object. Then the application will work, but any accidental or misguided release calls to the singleton will flag the error to the developer, not the application owner. This gives you the effect of developer guidance you were seeking without peril to the continued operation of the application.

    I would argue if you are going to override release you may as well do retain as well to match, to keep the retain count at the value you would expect a singleton to have. But that part I am far more indifferent to since it doesn’t really affect execution the way mishandling release can.

  32. Peter Hosey Says:

    Kendall Helmstetter Gelner:

    Lets say you get someone who knows memory management pretty well but forgets they are using a singleton class. So, they alloc/init and then when done they call release. Again, the running code comes back sometime later to the same point and calls alloc/init again….

    They are screwed, even though they followed the correct memory management rules. Because you did not override release, the shared instance variable you check in init is not nil, but is now invalid!

    No, it’s still a valid object, because the singleton owns itself.

    There may actually be a bug along those lines in the implementation shown above, though. I really need to put this into version control and write a bunch of tests for it.

    I would argue if you are going to override release you may as well do retain as well to match, to keep the retain count at the value you would expect a singleton to have.

    Correct. If you do override them, you should also override retainCount, to return UINT_MAX.

  33. Owen Hartnett Says:

    When I first saw Apple’s Singleton code, I thought it perverted at best, and not well thought out. (An associate described it as “baroque.”) Thanks for bringing some thought (and better code) to the subject!

  34. Christiaan Hofman Says:

    Apple\’s current example it still problematic. If the singleton is instantiated in a NIB, through alloc+init, then -init is executed *twice* on the shared instance, so when that is overridden you have a problem. This is why the shared instance must be assigned in -init, not in +sharedWhatever or +allocWithZone:. Now if you don\’t care about the possibility that an instance could be created through alloc+init, e.g. in a NIB, then I also don\’t see why you would care about enforcing a singleton (i.e. overriding +allocWithZone:) in the first place.

Leave a Reply

Do not delete the second sentence.


Warning: Undefined array key "ntt_saved_comment_text" in /home/public/blog/wp-content/plugins/negative-turing-test/negative-turing-test.php on line 143