On 9/29/15 10:33 PM, Xuelei Fan wrote:
private synchronized static KeyStore getDefaultCacertsKeyStore()
================================================================
---------------
private synchronized static KeyStore getDefaultCacertsKeyStore(
String javaHome, String type,
String provider, String password,
String dbgOption) throws Exception {
if (defaultCacertsKeyStore != null) {
return defaultCacertsKeyStore;
}
---------------
Looks like the parameters would impact the returned value. However, as
defaultCacertsKeyStore is singleton instance, these parameters have no
impact any more after the initialization. This is a behavior change.
The behavior change is OK to me, but we maybe want to add a comment or
twist the method a little bit.
This is true, but I don't think this is really a legitimate use case.
First, changing the system properties while an app is running is not
going to work very well in a multithreaded environment. Second, even in
a single threaded case, I can't think of a valid use case where you
would want to use the jssecacerts/cacerts file (i.e.
javax.net.ssl.trustStore is not set) and subsequently change the values
of the keystore type, provider or password system properties while the
app was running. Note that this behavior is still supported as before if
you do set the javax.net.ssl.trustStore property, and the properties are
read and used each time to create a new KeyStore instance.
I agree a comment would be helpful though, so I will add that. If you
think we should also release note this, let me know.
For lazy static fields, I would like to use "Lazy initialization holder
class idiom" [#71, Effective Java, 2ED], rather than synchronized
method, for performance benefits.
Good point, although I am a bit wary of using that pattern in this case
because it can throw an Exception. See "Only One Chance To Initialize"
at https://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom
--Sean
Thanks,
Xuelei
On 9/30/2015 2:07 AM, Sean Mullan wrote:
Please review this fix to modify the TrustManagerFactory implementation
to create a single instance of the cacerts or jssecacerts KeyStore. This
significantly improves performance in a multithreaded environment.
The code has been refactored a bit to move common code into a few
private methods.
webrev: http://cr.openjdk.java.net/~mullan/webrevs/8129988/webrev.00/
Thanks,
Sean
