[concurrency-interest] Code Review 7051516: ThreadLocalRandom seed is never initialized so all instances generate the same sequence

Chris Hegarty chris.hegarty at oracle.com
Thu Jun 16 12:37:13 EDT 2011


On 06/16/11 05:32 PM, Mike Duigou wrote:
> Hi Chris;
>
> The getClass() seems unnecessary. Why not :
>
>      public Random(long seed) {
>              this.seed = new AtomicLong();
>              setSeed(seed);
>      }
>
> or
>
> private final AtomicLong seed = new AtomicLong();
>
> public Random(long seed) {
>      setSeed(seed);
>      }
>
> Both of these would seem to have the same effect without needing to do the explicit class check.

The change ( originally from Martin ) tries to save a couple of volatile 
writes in the common case. Yes, it's not a pretty as it could be.

-Chris.

>
> Mike
>
> On Jun 16 2011, at 03:18 , Chris Hegarty wrote:
>
>> Hi,
>>
>> ThreadLocalRandom uses its own seed, not the seed of the super class.
>>
>>     ThreadLocalRandom() {
>>         super();
>>         initialized = true;
>>     }
>>
>> This uselessly initializes the parent seed via the default constructor but leaves the real seed at zero.
>>
>> Webrev:
>>   http://cr.openjdk.java.net/~chegar/7051516/jdk8_webrev.00/webrev/
>>
>> Note:
>>   This is a port of the fix from Doug's CVS to OpenJDK. A test has been added to Doug's CVS tck tests for ThreadLocalRandom, and I will file a CR against the JCK to have it pulled in to JCK8.
>>
>> -Chris.
>


More information about the Concurrency-interest mailing list