JDK-7051516 : ThreadLocalRandom seed is never initialized so all instances generate the same sequence
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util
  • Affected Version: 7
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2011-06-04
  • Updated: 2014-09-24
  • Resolved: 2012-05-13
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 7 JDK 8
7u2Fixed 8 b01Fixed
Related Reports
Relates :  
Relates :  
Description
From: "Aleksey Shipilev" <###@###.###>
Date: Fri, 3 Jun 2011 00:29:12 +1000
To: "concurrency-interest" <###@###.###>

Hi,

I've been stumbled upon ThreadLocalRandom seed behavior. JavaDoc
reads: "ThreadLocalRandom is initialized with an internally generated
seed that may not otherwise be modified." I would expect TLR called in
several threads concurrently to have different global values. But
apparently the internal seed in TLR always has the same seed (which is
default value for long).

Is this intentional? Or just oversight that should be fixed?

I.e. I would expect my test [1] print all-different values per thread,
like regular Random does.

That's what happens now:

Regular thread-local Random
780
4307
9112
4368
6673
========
4143
4905
2331
154
2887
========
9586
6161
9948
4179
3608
========

ThreadLocalRandom
0
6118
1895
7186
7366
========
0
6118
1895
7186
7366
========
0
6118
1895
7186
7366
========

Thanks,
Aleksey.

[1]
import java.util.Random;
import java.util.concurrent.ThreadLocalRandom;

public class Main {

    public static ThreadLocal<Random> random = new ThreadLocal<Random>() {
        @Override
        protected Random initialValue() {
            return new Random();
        }
    };

    public static void main(String[] args) throws InterruptedException {

        System.out.println("Regular thread-local Random");
        for (int i = 0; i < 3; i++) {
            Thread t = new Thread(new Runnable() {
                @Override
                public void run() {
                    for (int c = 0; c < 5; c++) {
                        System.out.println(random.get().nextInt(10000));
                    }
                    System.out.println("========");
                }
            });
            t.start();
            t.join();
        }

        System.out.println("ThreadLocalRandom");
        for (int i = 0; i < 5; i++) {
            Thread t = new Thread(new Runnable() {
                @Override
                public void run() {
                    for (int c = 0; c < 5; c++) {

System.out.println(ThreadLocalRandom.current().nextLong(10000));
                    }
                    System.out.println("========");
                }
            });
            t.start();
            t.join();
        }

    }

}

Comments
EVALUATION JDK8 changeset: Changeset: 411d02c13385 Author: dl Date: 2011-06-20 12:27 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/411d02c13385 7051516: ThreadLocalRandom seed is never initialized so all instances generate the same sequence Reviewed-by: chegar, dholmes, mduigou ! src/share/classes/java/util/Random.java
20-06-2011

SUGGESTED FIX diff --git a/src/share/classes/java/util/Random.java b/src/share/classes/java/util/Random.java --- a/src/share/classes/java/util/Random.java +++ b/src/share/classes/java/util/Random.java @@ -118,7 +118,13 @@ * @see #setSeed(long) */ public Random(long seed) { - this.seed = new AtomicLong(initialScramble(seed)); + if (getClass() == Random.class) + this.seed = new AtomicLong(initialScramble(seed)); + else { + // subclass might have overriden setSeed + this.seed = new AtomicLong(0L); + setSeed(seed); + } }
04-06-2011

EVALUATION From: "Martin Buchholz" <###@###.###> Date: Sat, 4 Jun 2011 07:35:52 +1000 To: "Aleksey Shipilev" <###@###.###> CC: "concurrency-interest" <###@###.###> This is my fault. As penance, here's a test for the TLR tck testcase: Index: ThreadLocalRandomTest.java =================================================================== RCS file: /export/home/jsr166/jsr166/jsr166/src/test/tck/ThreadLocalRandomTest.java,v retrieving revision 1.10 diff -u -r1.10 ThreadLocalRandomTest.java --- ThreadLocalRandomTest.java 31 May 2011 16:16:24 -0000 1.10 +++ ThreadLocalRandomTest.java 3 Jun 2011 21:29:51 -0000 @@ -6,6 +6,8 @@ import junit.framework.*; import java.util.*; import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; public class ThreadLocalRandomTest extends JSR166TestCase { @@ -252,4 +254,40 @@ } } + /** + * Different threads produce different pseudo-random sequences + */ + public void testDifferentSequences() { + // Don't use main thread's ThreadLocalRandom - it is likely to + // be polluted by previous tests. + final AtomicReference<ThreadLocalRandom> threadLocalRandom = + new AtomicReference<ThreadLocalRandom>(); + final AtomicLong rand = new AtomicLong(); + + long firstRand = 0; + ThreadLocalRandom firstThreadLocalRandom = null; + + final CheckedRunnable getRandomState = new CheckedRunnable() { + public void realRun() { + ThreadLocalRandom current = ThreadLocalRandom.current(); + assertSame(current, ThreadLocalRandom.current()); + assertNotSame(current, threadLocalRandom.get()); + rand.set(current.nextLong()); + threadLocalRandom.set(current); + }}; + + Thread first = newStartedThread(getRandomState); + awaitTermination(first); + firstRand = rand.get(); + firstThreadLocalRandom = threadLocalRandom.get(); + + for (int i = 0; i < NCALLS; i++) { + Thread t = newStartedThread(getRandomState); + awaitTermination(t); + if (firstRand != rand.get()) + return; + } + fail("all threads generate the same pseudo-random sequence"); + } + } and here's a fix for Random.java: I tried to save a couple of volatile writes in the common case, and this is a slightly gross way of continuing to do that: (of course, the "clean" version of this works as well) diff --git a/src/share/classes/java/util/Random.java b/src/share/classes/java/util/Random.java --- a/src/share/classes/java/util/Random.java +++ b/src/share/classes/java/util/Random.java @@ -118,7 +118,13 @@ * @see #setSeed(long) */ public Random(long seed) { - this.seed = new AtomicLong(initialScramble(seed)); + if (getClass() == Random.class) + this.seed = new AtomicLong(initialScramble(seed)); + else { + // subclass might have overriden setSeed + this.seed = new AtomicLong(0L); + setSeed(seed); + } } private static long initialScramble(long seed) {
04-06-2011

EVALUATION From: "Mark Thornton" <###@###.###> Date: Fri, 3 Jun 2011 18:57:46 +1000 To: <###@###.###> ThreadLocalRandom uses its own seed, not the seed of the super class. ThreadLocalRandom() { super(); initialized = true; } This uselessly initialises the parent seed via the default constructor but leaves the real seed at zero.
04-06-2011