On Mon, 13 May 2024 13:41:50 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> All random number generator algorithms are implemented in module 
>> `java.base`. The usage of `ServiceLoader` in `j.u.r.RandomGeneratorFactory` 
>> is no longer needed.
>
> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 
> 299:
> 
>> 297:     private void ensureConstructors() {
>> 298:         if (ctor == null) {  // volatile load
>> 299:             synchronized (rgClass) {
> 
> Here too I think we should synchronize on `this` - we would want to allow 
> multiple different instances of a `RandomGeneratorFactory` for the same 
> `RandomGenerator` class type to be able to concurrently instantiate their 
> individual instance fields (like the `ctor`(s) and `properties`).

Then I would even remove the double-checking idiom, the `volatile` on `ctor` 
and `properties`, and declare methods `getProperties()` and 
`ensureConstructors()` as `synchronized`.
I'm not sure that the double-checking optimization brings much value on 
contemporary JVMs.

But I feel that the followup PR discussed before wouldn't need `synchronized` 
at all.

WDYT?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19212#discussion_r1598520001

Reply via email to