United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-7051516 ThreadLocalRandom seed is never initialized so all instances generate the same sequence
JDK-7051516 : ThreadLocalRandom seed is never initialized so all instances generate the same sequence

Details
Type:
Bug
Submit Date:
2011-06-04
Status:
Closed
Updated Date:
2012-05-14
Project Name:
JDK
Resolved Date:
2012-05-13
Component:
core-libs
OS:
generic
Sub-Component:
java.util
CPU:
generic
Priority:
P3
Resolution:
Fixed
Affected Versions:
7
Fixed Versions:

Related Reports
Backport:
Relates:
Relates:

Sub Tasks

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
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);
+        }
     }
                                     
2011-06-04
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) {
                                     
2011-06-04
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.
                                     
2011-06-04
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
                                     
2011-06-20



Hardware and Software, Engineered to Work Together