JDK-6379897 : Random is not thread-safe; seed should be final
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util
  • Affected Version: 6
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2006-02-01
  • Updated: 2010-04-02
  • Resolved: 2006-02-18
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 6
6 b73Fixed
Related Reports
Relates :  
JDK-6348370 - java.math.BigInteger is *not* thread-safe immutable
Description
Random has its state in an AtomicLong, which should be final so that
no threads see it as uninitialized.

Doug Lea writes:

David Holmes wrote:

>> Hi Doug,
>> 
>> This is a link to the java.net forums for mustang.
>> 
>> http://forums.java.net/jive/thread.jspa?threadID=2443&tstart=0
>> 

Yup. The seed field ought to be declared final.
I bet the reason that it wasn't (when the internal sun.AtomicLong was used, in
1.4) was the need to set in readObject. Which had no solution
until 1.5, but no one noticed (It probably should have been us when
converting to use j.u.c.a.AtomicLong).
Coping requires an ugly unsafe.putObjectVolatile.
Comments
EVALUATION Doug Lea responds: "That version has the problem that non-atomic/volatile longs don't have guaranteed read/write atomicity. The idea of using weakCompareAndSet here is fine and possibly worth incorporating, although makes no difference in current hotspot. Given the extra write it contains in next, I'm surprised that the submitter found that version runs faster (although the report says it tested only on a uniprocessor)."
07-02-2006
SUGGESTED FIX --- /tmp/geta23827 2006-02-06 16:37:09.130278000 -0800 +++ Random.java 2006-02-06 16:36:50.558317000 -0800 @@ -4,14 +4,15 @@ * Copyright 2006 Sun Microsystems, Inc. All rights reserved. * SUN PROPRIETARY/CONFIDENTIAL. Use is subject to license terms. */ package java.util; import java.io.*; import java.util.concurrent.atomic.AtomicLong; +import sun.misc.Unsafe; /** * An instance of this class is used to generate a stream of * pseudorandom numbers. The class uses a 48-bit seed, which is * modified using a linear congruential formula. (See Donald Knuth, * <i>The Art of Computer Programming, Volume 3</i>, Section 3.2.1.) * <p> @@ -43,15 +44,15 @@ /** * The internal state associated with this pseudorandom number generator. * (The specs for the methods in this class describe the ongoing * computation of this value.) * * @serial */ - private AtomicLong seed; + private final AtomicLong seed; private final static long multiplier = 0x5DEECE66DL; private final static long addend = 0xBL; private final static long mask = (1L << 48) - 1; /** * Creates a new random number generator. This constructor sets @@ -488,15 +489,15 @@ // The seed is read in as {@code long} for // historical reasons, but it is converted to an AtomicLong. long seedVal = (long) fields.get("seed", -1L); if (seedVal < 0) throw new java.io.StreamCorruptedException( "Random: invalid seed"); - seed = new AtomicLong(seedVal); + resetSeed(seedVal); nextNextGaussian = fields.get("nextNextGaussian", 0.0); haveNextNextGaussian = fields.get("haveNextNextGaussian", false); } /** * Save the {@code Random} instance to a stream. */ @@ -511,8 +512,20 @@ fields.put("nextNextGaussian", nextNextGaussian); fields.put("haveNextNextGaussian", haveNextNextGaussian); // save them s.writeFields(); } + // Support for resetting seed while deserializing + private static final Unsafe unsafe = Unsafe.getUnsafe(); + private static final long seedOffset; + static { + try { + seedOffset = unsafe.objectFieldOffset + (Random.class.getDeclaredField("seed")); + } catch (Exception ex) { throw new Error(ex); } + } + private void resetSeed(long seedVal) { + unsafe.putObjectVolatile(this, seedOffset, new AtomicLong(seedVal)); + } }
07-02-2006
EVALUATION Contribution-Forum:https://jdk-collaboration.dev.java.net/servlets/ProjectForumMessageView?messageID=11246&forumID=1463
06-02-2006
EVALUATION The submitter is correct (according to the Java Memory Model). However, we can't seem to make these bugs actually manifest in the wild. We would love an actual test on the Sun JDK that demonstrates the failure (even if only once in a million tries).
01-02-2006