3

I have implemented the Singleton class as below:

public class Singleton {

    private static  Singleton instance = null;


    private Singleton() { 
    }

private synchronized static void createInstance() {
    instance = new Singletone();
}


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

}

But I want to know if it is a correct implementation of a singleton. Are there any problem in multithreaded environment.

AlexR
  • 114,158
  • 16
  • 130
  • 208
Sharad Ahire
  • 758
  • 2
  • 16
  • 32
  • Typo: It's _Singleton_ not Singletone. And yes, this is not a thread safe Singleton. – zengr Nov 30 '11 at 07:00
  • 1
    possible duplicate of [java singleton thread safe](http://stackoverflow.com/questions/4965534/java-singleton-thread-safe) – zengr Nov 30 '11 at 07:02
  • 1
    Possible duplicate: http://stackoverflow.com/questions/70689/efficient-way-to-implement-singleton-pattern-in-java – Alex K Nov 30 '11 at 07:35

7 Answers7

7

Your implementation is almost correct. The problem is that it is not thread-safe. 2 separate threads may enter getInstance() simultaneously, check that instance is null and then create 2 instances of your class. Here is the fix:

public static synchronized Singletone getInstance() {
    if(instance == null){
        createInstance();
    }
    return instance;
} 

Please pay attention on word synchronized.

AlexR
  • 114,158
  • 16
  • 130
  • 208
7
public enum Singleton {
    INSTANCE;
    private int val;

    public int getVal() {
        return val;
    }
}

Usage:

Singleton.INSTANCE.getVal();

This is the perfect singleton for Java versions > 5.0 where you have enum support.

Also mentioned in Joshua Bloch's Effective Java. A blog post about it here: Enum Singleton

Update:
Also, please use singletons only when you are 100% sure you need one! It kills testability of the code! But you cannot avoid it in places, say in a Factory.
But please do not abuse it, use it where ever you actually need it. Understand its use.

zengr
  • 38,346
  • 37
  • 130
  • 192
2

The best mechanism that I have come across other than the enum above is called static initialization. With this you rely on the guarantees of Java's memory model, so it's guaranteed to always work. Here's a snippet from an answer to a different question that demonstrates this:

class Singleton {
   static class SingletonHolder {
      static final Singleton INSTANCE = new Singleton();
   }
   public static Singleton instance() {
      return SingletonHolder.INSTANCE;
   }
}

The SingletonHolder class object with the Singleton instantiation will be created the first time that SingletonHolder.INSTANCE is invoked.

The Java memory model guarantees that static code (new Singleton()) will only be executed by one thread. So no double checked locking (which doesn't work), and no unnecessary synchronization. All subsequent calls will fetch that one instance.

Community
  • 1
  • 1
Jakub Korab
  • 4,974
  • 2
  • 24
  • 34
0

This is the correct implementation of the singleton pattern, although you don't really need a createInstance method; you could just inline this in getInstance. Also, it's spelled "Singleton", no "e" at the end.

Hypothetically, you could create an issue in a multithreaded environment. If two frame pointers enter getInstance at the same time, the one that entered first could get an instance of Singleton, and the second one would get another instance.

This depends on how you use it though. If you use the singleton before you set up the threads, there will be no problem. If this is a concern, you may consider initialising the singleton first. You can also solve this by using the synchronized keyword in the method declaration.

Steve Rukuts
  • 9,167
  • 3
  • 50
  • 72
0

Singleton is not thread safe. It is for this reason, Double-checked Locking was introduced.

As of Java SE 5 and higher, you can volatile your static instance. The Java VM will know how to handle the Singleton correctly when multiple threads run.

More on Double-checked locking.

Michał Kuliński
  • 1,928
  • 3
  • 27
  • 51
Buhake Sindi
  • 87,898
  • 29
  • 167
  • 228
0

For the lazy initialization implementation, which is your case, getInstance() method should be synchronized for thread safety.

public static synchronized Singleton getInstance()

Or you can simply initialize it at the class loading time which is already thread safe.

private static  Singleton instance = new MySingleton(); 
bsrykt
  • 1,275
  • 9
  • 9
-1

Your implementation seems just fine. Multithreading in a single JVM is not a problem for that kind of singleton problems but it will cause problems in clustered (two or more JVMs).

See http://java.sun.com/developer/technicalArticles/Programming/singletons/ for a run down of these issues.

Oh and it's singleton, not singletone. :)

vertti
  • 7,539
  • 4
  • 51
  • 81
  • 2
    Actually it **is** a **possible*** multithreading-problem even in single JVMs. Your link also covers this under "Multiple Instances Resulting from Incorrect Synchronization". – Boris Nov 30 '11 at 13:35