RFR: JDK-8288475: Initializing RandomGeneratorFactory.FactoryMapHolder fails if a SecurityManager is installed

[Johannes Kuhn]

On Fri, 17 Jun 2022 07:04:47 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> * This adds additional permissions to the jdk.random module (`RuntimePermission "accessClassInPackage.jdk.internal.util.random"`)
>> * The annotations of the provider classes are now parsed early.  
>>   This avoids putting the parts that can trigger the parsing into an `AccessController.doPrivileged()` block.
>> * If a `SecurityManager` is installed, `RandomGeneratorFactory.all()` will only return `RandomGenerator`s that are loaded by a system domain loader.  
>>   This avoids parsing annotations of user classes from a privileged context.
>
> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 141:
> 
>> 139: 
>> 140:     private static class FactoryMapHolder {
>> 141:         static final Map<String, Provider<? extends RandomGenerator>> FACTORY_MAP = createFactoryMap();
> 
> Unrelated to this PR, but a more general question. It appears that this `FACTORY_MAP` gets cached on first use/call. A few questions about the `createFactoryMap` method:
> 1. The javadoc of that private method says:
> 
> /**
>          * Returns the factory map, lazily constructing map on first use.
>          *
>          * @return Map of RandomGeneratorFactory classes.
>          */
> 
> But the implementation and the signature of this method actually return a Map of `RandomGenerator` classes and not the `RandomGeneratorFactory` classes.
> 2. The implementation of this method internally uses the `ServiceLoader` to load the `RandomGenerator` service provider implementations. The internal implementation of the `ServiceLoader` will use a Thread context classloader that is set on the calling thread. The result of the call to this `createFactoryMap` is then cached once and for all in the `FACTORY_MAP`. Would this then lead to a non-deterministic behaviour where whoever ends up initializing this `FactoryMapHolder` first, will end up storing those RandomGenerators for every one else? Is this intentional? Do you think this caching should be reviewed?

Good point. It might be useful to explicitly pass the boot layer to the service loader.

But that is outside the scope of this bug - my goal here is just to make it not throw an exception when running with a SecurityManager while not introducing security vulnerabilities.

-------------

PR: https://git.openjdk.org/jdk/pull/9180