Code Review 7051516: ThreadLocalRandom seed is never initialized so all instances generate the same sequence (original) (raw)

Chris Hegarty chris.hegarty at oracle.com
Thu Jun 16 16:37:13 UTC 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/jdk8webrev.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 core-libs-dev mailing list