Singleton in Java - I didn't do it

Question

I'm trying to track down an issue in our system and the following code worries me. The following occurs in our doPost() method in the primary servlet (names have been changed to protect the guilty):

...
if(Single.getInstance().firstTime()){
   doPreperations();
}
normalResponse();
...

The singleton 'Single' looks like this:

private static Single theInstance = new Single();

private Single() {
...load properties...
}

public static Single getInstance() {
    return theInstance;
}

With the way this is set to use a static initializer instead of checking for a null theInstance in the getInstance() method, could this get rebuilt over and over again?

PS - We're running WebSphere 6 with the App on Java 1.4

Answer 1

No it won't get built over and over again. It's static, so it'll only be constructed once, right when the class is touched for the first time by the Classloader.

Answer 2

No - the static initialization of the instance will only ever be done once. Two things to consider:

• This is not thread-safe (the instance is not "published" to main memory)
• Your firstTime method is probably called multiple times, unless properly synchronized

Answer 3

I found this on Sun's site:

Multiple Singletons Simultaneously Loaded by Different Class Loaders

When two class loaders load a class, you actually have two copies of the class, and each one can have its own Singleton instance. That is particularly relevant in servlets running in certain servlet engines (iPlanet for example), where each servlet by default uses its own class loader. Two different servlets accessing a joint Singleton will, in fact, get two different objects.

Multiple class loaders occur more commonly than you might think. When browsers load classes from the network for use by applets, they use a separate class loader for each server address. Similarly, Jini and RMI systems may use a separate class loader for the different code bases from which they download class files. If your own system uses custom class loaders, all the same issues may arise.

If loaded by different class loaders, two classes with the same name, even the same package name, are treated as distinct -- even if, in fact, they are byte-for-byte the same class. The different class loaders represent different namespaces that distinguish classes (even though the classes' names are the same), so that the two MySingleton classes are in fact distinct. (See "Class Loaders as a Namespace Mechanism" in Resources.) Since two Singleton objects belong to two classes of the same name, it will appear at first glance that there are two Singleton objects of the same class.

Citation.

In addition to the above issue, if firstTime() is not synchronized, you could have threading issues there as well.

Answer 4

In theory it will be built only once. However, this pattern breaks in various application servers, where you can get multiple instances of 'singleton' classes (since they are not thread-safe).

Also, the singleton pattern has been critized a lot. See for instance Singleton I love you, but you're bringing me down

Answer 5

The only thing I would change about that Singleton implementation (other than not using a Singleton at all) is to make the instance field final. The static field will be initialised once, on class load. Since classes are loaded lazily, you effectively get lazy instantiation for free.

Of course, if it's loaded from separate class loaders you get multiple "singletons", but that's a limitation of every singleton idiom in Java.

EDIT: The firstTime() and doPreparations() bits do look suspect though. Can't they be moved into the constructor of the singleton instance?

Answer 6

As others have mentioned, the static initializer will only be run once per classloader.

One thing I would take a look at is the firstTime() method - why can't the work in doPreparations() be handled within the singleton itself?

Sounds like a nasty set of dependencies.

Answer 7

This will get only loaded once when the class is loaded by the classloader. This example provides a better Singleton implementation however, it's as lazy-loaded as possible and thread-safe. Moreover, it works in all known versions of Java. This solution is the most portable across different Java compilers and virtual machines.

public class Single {

private static class SingleHolder {
   private static final Single INSTANCE = new Single();
}

private Single() {
...load properties...
}

public static Single getInstance() {
    return SingleHolder.INSTANCE;
}

}

The inner class is referenced no earlier (and therefore loaded no earlier by the class loader) than the moment that getInstance() is called. Thus, this solution is thread-safe without requiring special language constructs (i.e. volatile and/or synchronized).

Answer 8

There is absolutely no difference between using a static initializer and lazy initialization. In fact it's far easier to mess up the lazy initialization, which also enforces synchronization. The JVM guarantees that the static initializer is always run before the class is accessed and it will happen once and only once.

That said JVM does not guarantee that your class will be loaded only once. However even if it is loaded more than once, your web application will still see only the relevant singleton, as it will be loaded either in the web application classloader or its parent. If you have several web application deployed, then firstTime() will be called once for each application.

The most apparent things to check is that firstTime() needs to be synchronized and that the firstTime flag is set before exiting that method.

Answer 9

please note that the original example is indeed not correct. no one has pointed out that the classloader reference loading correctness is guaranteed only if the reference is final!

Answer 10

No, It won't create multiple copies of 'Single'. ( Classloader issue will be visited later )

The implementation you outlined is described as 'Eager Initialization' by in Briant Goetz's book - 'Java Concurrency in Practice'.

public class Single
{
    private static Single theInstance = new Single();

    private Single() 
    { 
        // load properties
    }

    public static Single getInstance() 
    {
        return theInstance;
    }
}

However, the code is not you wanted. Your code is trying to perform lazy-initialization after the instance is created. This requires all the client library to perform 'firstTime()/doPreparation()' before using it. You are going to rely on the client to do right thing which make the code very fragile.

You can modify the code as the following so there won't be any duplicate code.

public class Single
{
    private static Single theInstance = new Single();

    private Single() 
    { 
        // load properties
    }

    public static Single getInstance() 
    {   
        // check for initialization of theInstance
        if ( theInstance.firstTime() )
           theInstance.doPreparation();

        return theInstance;
    }
}

Unfortunately, this is a poor implementation of lazy initialization and this will not work in concurrent environment ( like J2EE container ).

There are many articles written about Singleton initialization, specifically on memory model. JSR 133 addressed many weakness in Java memory model in Java 1.5 and 1.6.

With Java 1.5 & 1.6, you have several choices and they are mentioned in the book 'Effective Java' by Joshua Bloch.

1. Eager Initialziation, like the above [EJ Item 3]
2. Lazy Initalization Holder Class Idiom [EJ Item 71]
3. Enum Type [EJ Item 3]
4. Double Checked Locking with 'volatile' static field [EJ Item 71]

Solution 3 and 4 will only work in Java 1.5 and above. So the best solution would be #2.

Here is the psuedo-implementation.

public class Single
{
    private static class SingleHolder
    {
        public static Single theInstance = new Single();
    }

    private Single() 
    { 
        // load properties
        doPreparation();
    }

    public static Single getInstance() 
    {
        return SingleHolder.theInstance;
    }
}

Notice that 'doPreparation()' is inside of the constructor so you are guarantee to get the properly initialized instance. Also, you are piggying back on JVM's lazy class loading and do not need any synchronization 'getInstance()'.

One thing you noticed that static field theInstance is not 'final'. The example on Java Concurrency does not have 'final' but EJ does. Maybe James's can add more color to his answer on 'classloader' and requirement of 'final' to guarantee correctness,

Having said that, there are a side-effect that with using 'static final'. Java compiler is very aggressive when it sees 'static final' and tries to inline it as much as possible. This is mentioned on a blog posting by Jeremy Manson.

Here is a simple example.

file: A.java

public class A
{
    final static String word = "Hello World";
}

file: B.java

public class B
{
    public static void main(String[] args) {
        System.out.println(A.word);
    }
}

After you compile both A.java and B.java, you change A.java to following.

file: A.java

public class A
{
    final static String word = "Goodbye World";
}

You recompile 'A.java' and rerun B.class. The output you would get is

Hello World

As for the classloader issue, the answer is yes, you can have more than one instance of Singleton in multiple classloaders. You can find more information on wikipedia. There is also a specific article on Websphere.

Answer 11

It's not mandatory for the single instance to be final (it's not a good idea at all indeed, because this will avoid you to switch it's behaviour using other patterns).

In the code below you can see how it gets instantiated only once (first time you call the constructor)

package date;

import java.util.Date;

public class USDateFactory implements DateFactory { private static USDateFactory usdatefactory = null;

private USDateFactory () { }

public static USDateFactory getUsdatefactory() {
    if(usdatefactory==null) {
        usdatefactory = new USDateFactory();
    }
    return usdatefactory;
}

public String getTextDate (Date date) {
    return null;
}

public NumericalDate getNumericalDate (Date date) {
    return null;
}

}