Repository: iluwatar/java-design-patterns
Issue: ThreadSafeDoubleCheckLocking.java: Instantiating by Reflection call will be successful if you do that firstly
Labels: type: bug
info: help wanted

https://github.com/iluwatar/java-design-patterns/blob/987994f0fe5747e5524bc43c3b2000bee478c0c3/singleton/src/main/java/com/iluwatar/singleton/ThreadSafeDoubleCheckLocking.java#L43-L45
Instance will be null, and you can create a singleton object by reflection without an exception. 

Issue comments:
Yes, that is true. But I think that is the way the code is supposed to work. It allows creating one and only one instance, right?
Actually you can create another instance via `ThreadSafeDoubleCheckLocking.getInstance()`, or as many instances as possible via reflection.
Can someone verify this?
@iluwatar Sure. I will have a look at it.
I tried testing this and it is still possible to create multiple instances if only using reflection. I saw this solution on StackOverflow which will not allow reflection to access private methods and it worked for me.

So I made this change:

private ThreadSafeDoubleCheckLocking() {
    // to prevent instantiating by Reflection call
	  checkPermission();
    if (instance != null) {
      throw new IllegalStateException("Already initialized.");
    }
    numOfInstances++;
  }
  
  void checkPermission() {
      Class self = sun.reflect.Reflection.getCallerClass(1);
      Class caller = sun.reflect.Reflection.getCallerClass(3);
      if (self != caller) {
          throw new java.lang.IllegalAccessError();
      }
  }

I would really appreciate feedback. 
I think we can create multiple instances of a singleton class only in case of lazy initialization as in the above example.
If we change that to eager initialization, this check will be sufficient to create multiple instances of this class.
Using sun.reflect.Reflection is not recommended and I don't really understand why it is able to create multiple instances inspite of what you have done in the constructor to try to avoid it, but this is something that can be done. Using reflection is not allowed on Google App Engine, I just saw, so maybe people are working around the security threat in that way. I found this solution here btw, forgot to mention in the previous comment: https://stackoverflow.com/questions/7566626/how-to-restrict-developers-to-use-reflection-to-access-private-methods-and-const
@iluwatar May I pick this up?
@Azureyjt please go ahead
PR created. Please kindly review it :) @iluwatar 

Pull request review comments:

Patch that resolved the issue:
diff --git a/singleton/pom.xml b/singleton/pom.xml
index e3d8f06e2ea5..038d6fdac040 100644
--- a/singleton/pom.xml
+++ b/singleton/pom.xml
@@ -38,5 +38,9 @@
       <artifactId>junit-jupiter-engine</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>junit</groupId>
+      <artifactId>junit</artifactId>
+    </dependency>
   </dependencies>
 </project>
diff --git a/singleton/src/main/java/com/iluwatar/singleton/ThreadSafeDoubleCheckLocking.java b/singleton/src/main/java/com/iluwatar/singleton/ThreadSafeDoubleCheckLocking.java
index a7557fda37e0..6c17f7c3f001 100644
--- a/singleton/src/main/java/com/iluwatar/singleton/ThreadSafeDoubleCheckLocking.java
+++ b/singleton/src/main/java/com/iluwatar/singleton/ThreadSafeDoubleCheckLocking.java
@@ -35,12 +35,16 @@ public final class ThreadSafeDoubleCheckLocking {
 
   private static volatile ThreadSafeDoubleCheckLocking instance;
 
+  private static boolean flag = true;
+
   /**
    * private constructor to prevent client from instantiating.
    */
   private ThreadSafeDoubleCheckLocking() {
     // to prevent instantiating by Reflection call
-    if (instance != null) {
+    if (flag) {
+      flag = false;
+    } else {
       throw new IllegalStateException("Already initialized.");
     }
   }


Test patch:
diff --git a/singleton/src/test/java/com/iluwatar/singleton/ThreadSafeDoubleCheckLockingTest.java b/singleton/src/test/java/com/iluwatar/singleton/ThreadSafeDoubleCheckLockingTest.java
index 1d00d9f8f624..6286b09fb2a4 100644
--- a/singleton/src/test/java/com/iluwatar/singleton/ThreadSafeDoubleCheckLockingTest.java
+++ b/singleton/src/test/java/com/iluwatar/singleton/ThreadSafeDoubleCheckLockingTest.java
@@ -22,6 +22,11 @@
  */
 package com.iluwatar.singleton;
 
+import org.junit.Test;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+
 /**
  * Date: 12/29/15 - 19:26 PM
  *
@@ -36,4 +41,15 @@ public ThreadSafeDoubleCheckLockingTest() {
     super(ThreadSafeDoubleCheckLocking::getInstance);
   }
 
+  /**
+   * Test creating new instance by refection
+   */
+  @Test(expected = InvocationTargetException.class)
+  public void testCreatingNewInstanceByRefection() throws Exception {
+    ThreadSafeDoubleCheckLocking instance1 = ThreadSafeDoubleCheckLocking.getInstance();
+    Constructor constructor = ThreadSafeDoubleCheckLocking.class.getDeclaredConstructor();
+    constructor.setAccessible(true);
+    ThreadSafeDoubleCheckLocking instance2 = (ThreadSafeDoubleCheckLocking) constructor.newInstance(null);
+  }
+
 }
\ No newline at end of file