On Fri, 17 Jun 2022 07:04:47 GMT, Jaikiran Pai <j...@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
