Singleton Pattern

[vbhavsar]

People have been coming up with creative solutions to lazily implement
the singleton pattern in a thread-safe way. We have seen things like
double-checked locking and creating instance via a single-elemnt enum
type.

I have thought of yet another way to implement this in a lazy and
thread-safe way. I haven't seen this proposed anywhere and it seems to
work unless I am missing something. Here it goes:

public class Singleton {

private static Singleton _instance;
private Singleton(){}

private synchronized static void createInstance(){
_instance = new Singleton();
}

public static Singleton getInstance(){
if (_instance == null){
createInstance();
}
return _instance;
}
}


The synchronized createInstance() method would eliminate the need to
do double-checked locking and the synchronization would happen only
when multiple threads call getInstance() before _instance has been
instantiated.

Anyone see any issues with this?

 

[markspace]

The synchronized createInstance() method would eliminate the need to
do double-checked locking

Nope, it's worse than standard double checked field and therefore
broken. Checking once outside of a synchronization block doesn't do
anything, it's still a read of an unsynchronized field.

That's why singleton patterns are often complicated, they're trying to
do something difficult.


See the comments I added to your snippet below.

 

[Eric Sosman]

People have been coming up with creative solutions to lazily implement
the singleton pattern in a thread-safe way. We have seen things like
double-checked locking and creating instance via a single-elemnt enum
type.

I have thought of yet another way to implement this in a lazy and
thread-safe way. I haven't seen this proposed anywhere and it seems to
work unless I am missing something. Here it goes:

public class Singleton {

private static Singleton _instance;
private Singleton(){}

private synchronized static void createInstance(){
_instance = new Singleton();
}

public static Singleton getInstance(){
if (_instance == null){
createInstance();
}
return _instance;
}
}


The synchronized createInstance() method would eliminate the need to
do double-checked locking and the synchronization would happen only
when multiple threads call getInstance() before _instance has been
instantiated.

Anyone see any issues with this?

Yes.

T1: if (_instance == null)
"Aha! It's null! Let's go make one."

** context switch **

T2: if (_instance == null)
"Aha! It's null! Let's go make one."

T2: _instance = createInstance(); // instance #1

** context switch **

T1: _instance = createInstance(); // instance #2

.... and the two threads go merrily on their way with references
to two different Singleton instances. With N threads, you could
get as many as N distinct instances.

 

[Lew]

Which, as you know, is broken. As it happens, it's broken in pretty much exactly the same way as your proposal.

This is one of the most well-discussed idioms in the literature. The flawsand solutions are pretty much out there for anyone to google.

Bear in mind that lazy initialization should be done judiciously, if at all.. See Joshua Bloch's /Effective Java/, Item 71, for example.
<http://java.sun.com/docs/books/effective/>

The Singleton (anti)pattern is also heavily abused.

and creating instance via a single-elemnt enum type.

I have thought of yet another [sic] way to implement this in a lazy and
thread-safe way. I haven't seen this proposed anywhere and it seems to
work unless I am missing something. Here it goes:

public class Singleton {

private static Singleton _instance;
private Singleton(){}

private synchronized static void createInstance(){
_instance = new Singleton();
}

public static Singleton getInstance(){
if (_instance == null){
createInstance();
}
return _instance;
}
}


The synchronized createInstance() method would eliminate the need to
do double-checked locking and the synchronization would happen only
when multiple threads call getInstance() before _instance has been
instantiated.

Anyone see any issues with this?

Yes.

T1: if (_instance == null)
"Aha! It's null! Let's go make one."

** context switch **

T2: if (_instance == null)
"Aha! It's null! Let's go make one."

T2: _instance = createInstance(); // instance #1

** context switch **

T1: _instance = createInstance(); // instance #2

... and the two threads go merrily on their way with references
to two different Singleton instances. With N threads, you could
get as many as N distinct instances.

See Doug Lea's and Brian Goetz's articles and books on Java threading, as well as the Java Language Specification. IBM Developerworks (Java) is a good online resource for articles. I recommend in particular /Java Concurrency in Practice/ by Goetz, et al., and many people urge /Concurrent Programming in Java/ by Mr. Lea. The aforementioned /Effective Java/ has several effective tips on the subject as well.

Concurrent programming is subtle and quick to anger. It pays to study the literature thoroughly.

 

[Rajeev]

People have been coming up with creative solutions to lazily implement
the singleton pattern in a thread-safe way. We have seen things like
double-checked locking and creating instance via a single-elemnt enum
type.

I have thought of yet another way to implement this in a lazy and
thread-safe way. I haven't seen this proposed anywhere and it seems to
work unless I am missing something. Here it goes:

public class Singleton {

        private static Singleton _instance;
        private Singleton(){}

        private synchronized static void createInstance(){
                _instance = new Singleton();
        }

        public static Singleton getInstance(){
                if (_instance == null){
                        createInstance();
                }
                return _instance;
        }

}

The synchronized createInstance() method would eliminate the need to
do double-checked locking and the synchronization would happen only
when multiple threads call getInstance() before _instance has been
instantiated.

Anyone see any issues with this?

As clearly pointed out by others, this code is far more vulnerable to
create duplicate instances than the double checked locking algorithm
itself. I think you have misunderstood the fundamental cause of the
problem here. Even I had confused myself (twice) before getting to
understand the solution. See [The "Double-Checked Locking is Broken"
Declaration] below

http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

-Rajeev Sreedharan

 

[Ian]

I'd never even heard of the "single-element enum type" variation of
singleton initialization. That one in particular sounds like a
completely over-engineered approach.

Pete

I wouldn't say over-engineered - it's about as verbose as a final field
declaration. ISTR one of the actual advantages is safety in the
presence of serialization, although Bloch mentions others . . .
I'd say it's the right tool for the job of creating instance-controlled
data safely.
[FX: Ducks!]
Ian.

 

[nalakathash2]

This is really weird. I have a wireless home LAN that my laptop connects
to to reach the internet. Earlier I did a virus check with TrendMicro
(found nothing). But since then I noticed that the friendly icon for my
wireless LAN - looks like a terminal with 2 "))" at the right upper corner
when connected, has disappeared from the taskbar. The wireless LAN is still
working fine and my connection to the internet is fine, but the icon is
gone! It is not hidden (I checked with customize taskbar, etc.). It has
just disappeared.

Instead it has been replaced by a different icon with a yellow bar at the
bottom which when I hover over it tells me about my local connection (which
is unplugged) and my wireless connection (which is connected). However when
I right click on it, I do not get the familiar "view available networks" or
repair, etc. Instead I get a different blue option list that comes from
"ConfigFree" the utility that Toshiba puts on its notebooks.

How do I regain my old network icon with the familiar "view available
networks"? Rebooting did not solve the problem.

 

[Lew]

I'd never even heard of the "single-element enum type" variation of
singleton initialization. That one in particular sounds like a
completely over-engineered approach.

I wouldn't say over-engineered - it's about as verbose as a final field
declaration. ISTR one of the actual advantages is safety in the
presence of serialization, although Bloch mentions others . . .
I'd say it's the right tool for the job of creating instance-controlled
data safely.
[FX: Ducks!]

Actually, pretty much any attempt to lazy initialize is over-engineering. Just declare a final member in most cases - why be lazy? It's a false optimization.

Final fields, especially immutable ones, make a lot of useful guarantees, and you don't have the field in the first place unless it's going to be used at some point anyway. Just freaking initialize it!

(I know, the enum pattern for final fields is not about lazy instantiation, although you do get that for free from it. I have no issue with that pattern and use it myself.)