On Tue, 16 Mar 2021 15:29:51 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:
>> Doesn't that still incur a JVM-wide lock whenever >> `CachedSecureRandomHolder.getDefSecureRandom()` is called? I recall a >> different JVM-wide lock in the `getInstance` path >> ([JDK-7107615](https://bugs.openjdk.java.net/browse/JDK-7107615)) creating >> significant performance issues. > > I did not get the point that the code path should be unaffected". As there > are a few update like: > init(keysize, JCAUtil.getSecureRandom()); > init(keysize, JCAUtil.getDefSecureRandom()); > > I think there are some impact on the existing application which use the > default sec > >> Or, I can also separate out the instance returned by getSecureRandom() so >> that code path should be unaffected, e.g.: > > I did not get the point that the "code path should be unaffected"? > JCAUtil.getDefSecureRandom() is used in updates like: > init(keysize, JCAUtil.getSecureRandom()); > init(keysize, JCAUtil.getDefSecureRandom()); > >> >> ``` >> // cached SecureRandom instance >> private static class CachedSecureRandomHolder { >> public static SecureRandom instance = new SecureRandom(); >> private static Provider[] cachedConfig = Security.getProviders(); >> private static SecureRandom def = instance; >> >> public static SecureRandom getDefault() { >> synchronized (CachedSecureRandomHolder.class) { >> Provider[] currConfig = Security.getProviders(); >> if (!Arrays.equals(cachedConfig, currConfig)) { >> def = new SecureRandom(); >> cachedConfig = currConfig; >> } >> } >> return def; >> } >> } >> >> /** >> * Get a SecureRandom instance. This method should be used by JDK >> * internal code in favor of calling "new SecureRandom()". That needs to >> * iterate through the provider table to find the default SecureRandom >> * implementation, which is fairly inefficient. >> */ >> public static SecureRandom getSecureRandom() { >> return CachedSecureRandomHolder.instance; >> } >> >> /** >> * Get the default SecureRandom instance. This method is the >> * optimized version of "new SecureRandom()" which re-uses the default >> * SecureRandom impl if the provider table is the same. >> */ >> public static SecureRandom getDefSecureRandom() { >> return CachedSecureRandomHolder.getDefault(); >> } >> ``` > As this behavior has been a while, I may not update the implementation. > Instead, I would like to add an "implNote" tag to state that once the default > provider is loaded, an implementation may be changed it any longer. > Applications could use provider parameter if there is a preference. Well, javadoc of the affected init methods clearly specifies that "get them using the SecureRandom implementation of the highest-priority installed provider as the source of randomness. ", thus I am not sure if we can just get around this with an _@implNote_. ------------- PR: https://git.openjdk.java.net/jdk/pull/3018