Does below class implement correct Java Singleton Object and provide Thread Safety .. It seems so looking at my Unit Test Case

Question

package samples.study;

/**
 * 
 * @author
 */
public class Singleton {
    private Singleton(){}
    private static final Singleton instance = new Singleton();

    public static Singleton getInstance() {
        System.out.println(Thread.currentThread().getName() + " getInstance");
        return Singleton.instance;
    }
}

Unit Test Case Seems to work fine and singleton objects are returned from Multiple threads. Help me spot any issues here ?

package samples.study;

import static org.junit.jupiter.api.Assertions.assertSame;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

class SingletonTest {

    @BeforeEach
    void setUp() throws Exception {
    }

    @Test
    void test() throws InterruptedException {

        ExecutorService s = Executors.newCachedThreadPool();
        Singleton obj = Singleton.getInstance();
        int threadCount = 10;
        List<Singleton> objList = new ArrayList<Singleton>();
        while (threadCount > 0) {
            s.submit(() -> {
                objList.add(getInstance(s));
            });
            threadCount--;
        }
        while (threadCount > 0) {
            assertSame(obj, objList.get(threadCount));
        }
    }

    public Singleton getInstance(ExecutorService s) {
        Singleton obj = Singleton.getInstance();
        System.out.println("1 = " + Thread.currentThread().getName() + obj);
        return obj;
    }

}

Result of UT :

• main getInstance pool-1-thread-3 getInstance pool-1-thread-2 getInstance 1 = pool-1-thread-2samples.study.Singleton@75cbce36 pool-1-thread-4 getInstance 1 = pool-1-thread-4samples.study.Singleton@75cbce36 pool-1-thread-8 getInstance 1 = pool-1-thread-8samples.study.Singleton@75cbce36 pool-1-thread-7 getInstance pool-1-thread-9 getInstance pool-1-thread-6 getInstance pool-1-thread-10 getInstance 1 = pool-1-thread-10samples.study.Singleton@75cbce36 1 = pool-1-thread-6samples.study.Singleton@75cbce36 pool-1-thread-5 getInstance 1 = pool-1-thread-5samples.study.Singleton@75cbce36 1 = pool-1-thread-9samples.study.Singleton@75cbce36 1 = pool-1-thread-7samples.study.Singleton@75cbce36 pool-1-thread-1 getInstance 1 = pool-1-thread-1samples.study.Singleton@75cbce36 1 = pool-1-thread-3samples.study.Singleton@75cbce36

Pierrepierrepont

You're fine, that's a valid eager thread-safe singleton initialization. – Agosto

Yes, that's fine. Just one thing: public static final Singleton instance = new Singleton(); Make this private. When you have getter, you shoud make it private – Snicker

The main reason I tried above code was to find the difference between another Singleton Sample I found on the web as it claimed to be safest in a multithreaded scenario. Why make it complicated and add a static inner class ? public class Singleton { private Singleton() {} private static class SingletonHolder { public static final Singleton instance = new Singleton(); } public static Singleton getInstance() { return SingletonHolder.instance; } } @Adarsh @Agosto – Pierrepierrepont

At least, you must define constructor with no arguments which is private. Your intention is to get the instance via Singleton.getInstance() but users can go with new Singleton() and can break thread-safe-ness easily. – Brew

@Pierrepierrepont By using a static inner class, you're not making it complicated. That's the LazyHolder pattern or the instance is lazily loaded. Or, the instance is created when you invoke the getInstance method. On the contrary, in the post, you're loading it eagerly (loading when the class loads). Also, in the post above, you must make the constructor private – Snicker

Done - added Private constructor - The idea is to understand if just having a static final field good enough to make it singleton – Pierrepierrepont

@Adarsh that’s just wrong. You are confusing class loading and initialization. The instance will be created when the class is initialized which will happen when one of the conditions specified in JLS &12.4.1 is fulfilled. In practice, it’s when the getInstance() method is invoked for the first time. Your assumption that the inner class’ initialization is lazy, is based on exactly the same mechanism. There are no different rules for nested classes. The nested class is just obsolete here. – Seena