ThreadSafeDoubleCheckLocking.java: Instantiating by Reflection call will be successful if you do that firstly

[DovahYol]

java-design-patterns/singleton/src/main/java/com/iluwatar/singleton/ThreadSafeDoubleCheckLocking.java

Lines 43 to 45 in 987994f

	if (instance != null) {
	throw new IllegalStateException("Already initialized.");
	}

Instance will be null, and you can create a singleton object by reflection without an exception.

[iluwatar]

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?

[DovahYol]

Actually you can create another instance via ThreadSafeDoubleCheckLocking.getInstance(), or as many instances as possible via reflection.

[iluwatar]

Can someone verify this?

[npathai]

@iluwatar Sure. I will have a look at it.

[AnaghaSasikumar]

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.

[IAmPramod]

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.

[AnaghaSasikumar]

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

[Azureyjt]

@iluwatar May I pick this up?

[iluwatar]

@Azureyjt please go ahead

[Azureyjt]

PR created. Please kindly review it :) @iluwatar