0
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
parthi
  • 101
  • 8
  • 1
    You're [fine](https://stackoverflow.com/questions/52687983/is-java-eager-singleton-creation-thread-safe), that's a valid eager thread-safe singleton initialization. – Gryphon Sep 02 '20 at 04:08
  • 1
    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` – adarsh Sep 02 '20 at 04:10
  • 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 @Gryphon – parthi Sep 02 '20 at 04:14
  • 1
    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. – MNEMO Sep 02 '20 at 04:19
  • 1
    @parthi 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** – adarsh Sep 02 '20 at 04:21
  • Done - added Private constructor - The idea is to understand if just having a static final field good enough to make it singleton – parthi Sep 02 '20 at 04:22
  • 2
    @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](https://docs.oracle.com/javase/specs/jls/se14/html/jls-12.html#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. – Holger Sep 02 '20 at 07:46

0 Answers0