United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-6955840 : ThreadLocalRandom bug - overridden setSeed(long) method is not invoked for java.util.Random(long)

Details
Type:
Bug
Submit Date:
2010-05-26
Status:
Closed
Updated Date:
2012-03-22
Project Name:
JDK
Resolved Date:
2011-05-18
Component:
core-libs
OS:
generic
Sub-Component:
java.util
CPU:
generic
Priority:
P2
Resolution:
Fixed
Affected Versions:
7
Fixed Versions:

Related Reports
Relates:
Relates:

Sub Tasks

Description
The constructor of the java.util.Random class, which creates a new random number generator using a single {@code long} seed, doesn't calls setSeed(seed) method more.
In brief, spec should be clarified here. Actual spec says that Random(seed) is equivalent to 

Random rnd = new Random();
rnd.setSeed(seed);

But current implementation does the same as the code above, but does not call rnd.setSeed(seed). It is not obvious at all - does it still "equivalent" or not that clearly demonstrated by corresponding JCK test.
More precise the Random(seed) is not equivalent to

Random rnd = new Random();
rnd.setSeed(seed);

As described in the my comments the haveNextNextGaussian variable will not be cleaned if the instance of Random created with Random(seed) constructor.

                                    

Comments
EVALUATION

When it says "is equivalent to" this refers to the methods of the current class, not to any methods of a derived class. There is no requirement, nor should there be any expectation, that the setSeed method is actually invoked as part of the constructor. For example, a common implementation pattern for "equivalent" code would do:

    public Random(long seed) {
       ...
       setSeedInternal(seed);
    }
 
    public void setSeed(long seed) {
       setSeedInternal(seed);
    }

    private setSeedInternal(long seed) { ... }

Any class that overrides setSeed should not expect it to be invoked as part of the constructor. Any test that makes this assumption is invalid in my view.
                                     
2010-05-26
EVALUATION

The following JCK7 tests:
  api/java_util/Random/index.html#Random[Random2002,Random2004]
are invalid because they incorrectly assume that the setSeed method will be invoked as part of the Random(long) constructor call.
                                     
2010-05-27
EVALUATION

java.util.concurrent.ThreadLocalRandom.setSeed(long seed) does not throw UnsupportedOperationException due to wrong assumption that the setSeed method will be invoked as part of the java.util.Random constructors call.
                                     
2010-05-28
SUGGESTED FIX

Martin Buchholz suggests:

diff --git a/src/share/classes/java/util/concurrent/ThreadLocalRandom.java
b/src/share/classes/java/util/concurrent/ThreadLocalRandom.java
--- a/src/share/classes/java/util/concurrent/ThreadLocalRandom.java
+++ b/src/share/classes/java/util/concurrent/ThreadLocalRandom.java
@@ -73,10 +73,10 @@
     private long rnd;

     /**
-     * Initialization flag to permit the first and only allowed call
-     * to setSeed (inside Random constructor) to succeed.  We can't
-     * allow others since it would cause setting seed in one part of a
-     * program to unintentionally impact other usages by the thread.
+     * Initialization flag to permit calls to setSeed to succeed only
+     * while executing the Random constructor.  We can't allow others
+     * since it would cause setting seed in one part of a program to
+     * unintentionally impact other usages by the thread.
      */
     boolean initialized;

@@ -98,11 +98,10 @@

     /**
      * Constructor called only by localRandom.initialValue.
-     * We rely on the fact that the superclass no-arg constructor
-     * invokes setSeed exactly once to initialize.
      */
     ThreadLocalRandom() {
         super();
+        initialized = true;
     }

     /**
@@ -123,7 +122,6 @@
     public void setSeed(long seed) {
         if (initialized)
             throw new UnsupportedOperationException();
-        initialized = true;
         rnd = (seed ^ multiplier) & mask;
     }
                                     
2010-05-28
EVALUATION

As there is no spec or implementation issue with Random, and a seperate CR has been opened for the tests that made the invalid assumption, I hereby commandeer this CR for the remaining issue in ThreadLocalRandom - the synopsis has been changed accordingly.
                                     
2010-06-03
EVALUATION

Changeset: 1db252f307b6
Author:    martin
Date:      2010-06-02 17:53 -0700
URL:       http://hg.openjdk.java.net/jdk7/tl/jdk/rev/1db252f307b6

6955840: ThreadLocalRandom bug - overriden setSeed(long) method is not invoked for java.util.Random(long)
Summary: Allow setSeed only during construction
Reviewed-by: dl, dholmes

! src/share/classes/java/util/concurrent/ThreadLocalRandom.java
                                     
2010-06-03



Hardware and Software, Engineered to Work Together