Re: Is Apple's singleton sample code correct?
Re: Is Apple's singleton sample code correct?
- Subject: Re: Is Apple's singleton sample code correct?
- From: Dietmar Planitzer <email@hidden>
- Date: Fri, 2 Dec 2005 16:26:07 +0100
On Dec 2, 2005, at 5:25 AM, mmalcolm crawford wrote:
Agreed and disagreed.
If the point was simply that the example needs to be better
explained, then that's a fair comment and it would be appropriate
to file a bug report about it (again, I haven't seen one...). And
from my perspective, I do care about how novices who follow, so
have taken it upon myself to try to get it improved. There will be
a first pass revision shortly, and I would welcome suggestions for
how it might be made better with more time in the future...
I must say I'm quite a bit surprised at how this discussion has been
going on. I'm especially surprised when I consider on one hand that
the OP, as far as I remember, wondered why the singelton sample code
is overriding the various reference counting methods and that he was
seeking an answer to this question. On the other side I'm surprised
by the fact that as far as I can tell not too many people
participating in this discussion seem to have actually read the
sample code.
I encourage everyone who reads this lines to take a close look at the
sample code which can be found here: <
http://developer.apple.com/
documentation/Cocoa/Conceptual/CocoaObjects/Articles/
CreateSingleton.html>. When you do this you will notice that the code
as it is written there contains at least three quite obvious bugs.
I assume that the goal of the sample code is to create a singleton
which is supposed to be created on demand and which should then have
a lifetime that coincides with that of the application. I assume,
because the web page actually never makes it explicit whether that is
the goal of the sample code. What we must keep in mind in this case
is that the application implicitly retains the singleton as it is the
conceptual owner of the singleton object.
So with that in mind, take a close look at the +allocWithZone: method
and its implementation. The implementation presented in the sample
code violates an important Cocoa memory management rule. Namely the
rule that a method with the name "alloc", "copy" or "retain" is
supposed to return a strong reference to the caller. The problem here
is that the code fails to properly retain the singleton before it
returns the singleton to the caller, ignoring the fact that the
application is the conceptual owner of the singleton. Well actually,
the situation is worse. The code returns a strong reference the first
time it is called, but in the case of all further calls a weak
reference is returned. Thus the code should be changed to something
like this:
+ (id)allocWithZone:(NSZone *)zone
{
if (sharedGizmoManager == nil) {
sharedGizmoManager = [super allocWithZone:zone];
}
return [sharedGizmoManager retain];
}
Note that I have dropped the thread-safety stuff because it is
irrelevant in the context of this discussion. In fact it's just one
of the many subtopics that have been introduced in the course of this
discussion that have created more confusion then they have helped to
shed light on the original problem. Anyway, the implementation above
now correctly retains the singleton before it returns to the caller
in order to create a new strong reference to the singleton. That's
after all what the caller can expect according to the universally
loved Cocoa mem mag rules. Note that when +allocWithZone: is called
for the first time, it creates the singleton and the strong reference
which we get back from [super allocWithZone: zone] corresponds to the
implicit retain of the application. So together with the -retain call
at the bottom of the method, the first time our +allocWithZone: gets
called the singleton has a retain count of two. One of those counts
belongs to the application, the other one to the caller (client).
The second bug is in the -copyWithZone method. The problem here is in
fact the same as the previous one. Again, the correction consists of
the introduction of a retain call:
- (id)copyWithZone:(NSZone *)zone
{
return [self retain];
}
Now the third bug is maybe not so obvious after all. The problem is
that the documentation suggests that one should be able to use the
standard object allocation and initialization sequence on the
singleton and that the singleton automatically does the right thing.
So far so good, and indeed the implementation of +allocWithZone: does
the right thing. However, the -init method does not. If an
application executes this sequence more than once, then the result
will be a singleton in an unexpected or worse an ill-defined state
because every call to -init results in an re-initialization of the
singleton. The problem here is that the sample code does not override
the -init method in order to protect the singleton from multiple
initializations. The solution to this problem can be as simple as the
introduction of a boolean instance variable which is initially set to
false. The protection code can then look something like this:
- (id)init
{
if(_isInited)
return self;
[super init];
// do initialization
_isInited = YES;
return self;
}
Now, since we have corrected the implementation of the
+allocWithZone: method so that it actually respects the CMM rules, we
need to adjust the implementation of the +sharedManager method to the
new situation:
+ (MyGizmoClass*)sharedManager
{
if(sharedGizmoManager == nil) {
return [[[self alloc] init] autorelease];
}
return sharedGizmoManager;
}
Yes you are right. The code looks at first glance quite unintuitive
and weird. However, keep in mind that one of the goals of the
original specification was that the client of the singleton should be
able to get a strong reference to the singleton using the standard
object allocation and initialization pattern. Thus, it makes sense to
concentrate the implementation details of the singleton allocation
into a single place, namely the +allocWithZone: method. Now, if
+sharedManager gets called and the singleton does not yet exist, we
allocate it by calling alloc and init. Remember that the conceptual
owner of the singleton is the application and that the
+allocWithZone: method accounts for that fact. Thus, the newly
created strong reference we get back from +alloc, which is for the
client of the singleton, needs to be turned back into a weak
reference which we achieve by calling -autorelease on the strong
reference. After all, according to the CMM rules, +sharedManager is
supposed to return a weak reference. If on the other hand the
singleton already exists, then we simply return a weak reference by
returning the singleton directly saving some CPU cycles by avoiding
the -autorelease call.
At this point the obvious question is: what about the reference
counting methods ?
The answer is simple: overriding them is unnecessary and does not
serve a real technical purpose. In fact, by overriding them the
sample code has hidden the various memory management bugs it has and
additionally, and even worse, it has made it impossible to find
memory management bugs in the application code which is dealing with
the singleton. This however can easily become a problem on the day
that the singleton is replaced with a standard object because we may
need to add another feature to the application that makes it
impractical to retain the singleton aspect of the class which has so
far been implemented as a singleton.
Regards,
Dietmar Planitzer
_______________________________________________
Do not post admin requests to the list. They will be ignored.
Cocoa-dev mailing list (email@hidden)
Help/Unsubscribe/Update your Subscription:
This email sent to email@hidden