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

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
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
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

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
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

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



Hardware and Software, Engineered to Work Together