On Tue, 16 Mar 2021 20:53:27 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> 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_.
> 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());
What I mean is that the new suggestion will not impact callers of
JCAUtil.getSecureRandom() which I thought what your performance regression
means. To enforce that the most preferred SecureRandom impl is used, this for
sure will incur some cost especially when the existing impl just returns a
cached obj w/o any checking.
-------------
PR: https://git.openjdk.java.net/jdk/pull/3018
