Singletons in Cocoa: Doing them wrong
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.
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 newMyFloozit
is allocated, and a staticMyFloozit
pointer is set. Whenfloozit1
is released, that static pointer is still pointing to the old instance. As a result, when we try to setfloozit2
(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-scopestatic
storage. Then it returns the One True Instance. - We’ll allow going through
alloc
andinit
, and return the same instance. We’ll do this inallocWithZone:
, as Apple did. We’ll also need to make sureinit
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:
sharedFramistan
always returns the same object.alloc
/init
always produce the same object (which, itself, is the same object assharedFramistan
).alloc
/init
will not return an object confused by multipleinit
messages.- Over-releasing causes a crash.
- Keeping
alloc
/allocWithZone:
/retain
balanced withrelease
never causes a crash. - 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.
June 17th, 2009 at 15:50:20
For the sake of argument, what’s wrong with this?
June 17th, 2009 at 15:55:03
Your singleton implementation isn’t thread-safe, btw.
June 17th, 2009 at 16:29:25
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.June 17th, 2009 at 18:59:37
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
June 17th, 2009 at 19:11:04
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.
June 17th, 2009 at 19:48:45
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.
June 17th, 2009 at 20:47:25
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. :-)
June 17th, 2009 at 21:09:51
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.
June 18th, 2009 at 04:13:28
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.
June 18th, 2009 at 10:40:31
Cédric Luthi: Throwing an exception in
allocWithZone:
orinit
would work. Doing so inretain
andrelease
would prevent you from setting the object as a timer target, since the timer will retain it.June 19th, 2009 at 00:10:37
Well reasoned. I accept your recommendations.
June 19th, 2009 at 18:00:52
\”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.
June 19th, 2009 at 21:56:41
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.)
June 20th, 2009 at 17:39:54
I suppose you don’t have to use threads with NSOp. But really, 99% of the time, you will.
July 6th, 2009 at 16:02:48
When does the static sharedFramistan get deallocated?
If it has retained other objects, can I just implement a dealloc method to release those objects?
July 6th, 2009 at 16:36:23
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 tonil
) in your notification handler.As for
dealloc
, yes, with one caveat: If you don’t release the singleton in aNSApplicationWillTerminateNotification
handler, the class owns its instance until the process exits, so the instance never formally dies, so itsdealloc
never gets called. Only if you do implement thatNSApplicationWillTerminateNotification
handler doesdealloc
get called (as usual).July 10th, 2009 at 09:04:16
Apple’s example is also not correct in 64 bit. Their UINT_MAX should be NSUIntegerMax.
July 10th, 2009 at 20:03:26
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.
August 2nd, 2009 at 21:01:30
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.
September 3rd, 2009 at 06:01:32
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.
September 4th, 2009 at 01:40:12
Think about this: Where is
sharedInstance
assigned?September 4th, 2009 at 04:48:10
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.
September 4th, 2009 at 19:18:37
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:
hasInited
was an instance variable.September 19th, 2009 at 10:17:06
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.
December 17th, 2009 at 07:11:09
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.
December 17th, 2009 at 11:55:34
This implies that it can be switched in and out of full-screen mode. Is that correct?
January 10th, 2010 at 04:52:12
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 extraautorelease
. In-init
you have two assignments that are not properly retained. So the assignment ofsharedInstance
should readsharedInstance = [self retain];
, as this is the place where the class will take ownership of this instance. Also, the opposite assignment later on should readself = [sharedInstance retain];
, the extraretain
essentially balances therelease
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.January 10th, 2010 at 05:21:03
More like properly not retained. If the caller goes directly to
alloc
/init
instead of going throughsharedInstance
, retaining the receiver ininit
will over-retain it (implicit retain byalloc
+ explicitretain
ininit
). Thus, explicitly retaining ininit
is not an option if I want to maintain compatibility with thealloc
/init
path. Without explicitly retaining ininit
, anautorelease
insharedInstance
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.)
January 10th, 2010 at 11:18:51
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
, thesharedInstance
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 aretain
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 thatself
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 extraretain
. Think about when somehow the code would go through this line as it is now: in that case the retain count forsharedInstance
would be the same, even though analloc/init
is used to get it, so a followingrelease
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 ofsharedInstance
to be reduced by one, and it would become invalid.In short, this is the correct code.
January 10th, 2010 at 13:03:11
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.
February 5th, 2010 at 13:07:37
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.
September 4th, 2010 at 05:12:53
Kendall Helmstetter Gelner:
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.
Correct. If you do override them, you should also override
retainCount
, to returnUINT_MAX
.October 12th, 2010 at 11:34:17
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!
February 19th, 2011 at 08:30:40
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.