Aleksey Shipilev | 2 Jun 16:29 2011
Picon

ThreadLocalRandom initial seed

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
(Continue reading)

Kasper Nielsen | 2 Jun 23:43 2011
Picon

Re: ThreadLocalRandom initial seed

Good catch,

that is definitely a bug.

Looks like the fix to
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6937857
introduced a non-compatible change by not calling setSeed() in 
Random(long seed) anymore.

Cheers
   Kasper

On 02-06-2011 16:29, Aleksey Shipilev wrote:
> 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
(Continue reading)

David Holmes | 3 Jun 00:09 2011
Picon

Re: ThreadLocalRandom initial seed

I thought this had been addressed by 6955840

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6955840

"ThreadLocalRandom bug - overridden setSeed(long) method is not invoked for
java.util.Random(long)"

David

> -----Original Message-----
> From: concurrency-interest-bounces <at> cs.oswego.edu
> [mailto:concurrency-interest-bounces <at> cs.oswego.edu]On Behalf Of Kasper
> Nielsen
> Sent: Friday, 3 June 2011 7:44 AM
> To: concurrency-interest <at> cs.oswego.edu
> Subject: Re: [concurrency-interest] ThreadLocalRandom initial seed
>
>
> Good catch,
>
> that is definitely a bug.
>
> Looks like the fix to
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6937857
> introduced a non-compatible change by not calling setSeed() in
> Random(long seed) anymore.
>
> Cheers
>    Kasper
>
(Continue reading)

Aleksey Shipilev | 3 Jun 07:22 2011
Picon

Re: ThreadLocalRandom initial seed

Hi David, Kasper,

This issue is still reproduced on jdk7b144, as well as
bootclasspath'ed today's binary bundle of jsr166.jar. I guess it still
not fixed then.

-Aleksey.

On Fri, Jun 3, 2011 at 2:09 AM, David Holmes <davidcholmes <at> aapt.net.au> wrote:
> I thought this had been addressed by 6955840
>
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6955840
>
> "ThreadLocalRandom bug - overridden setSeed(long) method is not invoked for
> java.util.Random(long)"
>
> David
>
>> -----Original Message-----
>> From: concurrency-interest-bounces <at> cs.oswego.edu
>> [mailto:concurrency-interest-bounces <at> cs.oswego.edu]On Behalf Of Kasper
>> Nielsen
>> Sent: Friday, 3 June 2011 7:44 AM
>> To: concurrency-interest <at> cs.oswego.edu
>> Subject: Re: [concurrency-interest] ThreadLocalRandom initial seed
>>
>>
>> Good catch,
>>
>> that is definitely a bug.
(Continue reading)

David Holmes | 3 Jun 08:43 2011
Picon

Re: ThreadLocalRandom initial seed

Aleksey,

The problem is not the seed - all threads do get a distinct seed. The problem is that the first call to
nextInt(n) is always returning zero - hence the sequence will always be the same. I haven't yet determined
exactly what is going wrong.

David

> -----Original Message-----
> From: Aleksey Shipilev [mailto:aleksey.shipilev <at> gmail.com]
> Sent: Friday, 3 June 2011 3:23 PM
> To: dholmes <at> ieee.org
> Cc: Kasper Nielsen; concurrency-interest <at> cs.oswego.edu
> Subject: Re: [concurrency-interest] ThreadLocalRandom initial seed
> 
> 
> Hi David, Kasper,
> 
> This issue is still reproduced on jdk7b144, as well as
> bootclasspath'ed today's binary bundle of jsr166.jar. I guess it still
> not fixed then.
> 
> -Aleksey.
> 
> On Fri, Jun 3, 2011 at 2:09 AM, David Holmes 
> <davidcholmes <at> aapt.net.au> wrote:
> > I thought this had been addressed by 6955840
> >
> > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6955840
> >
(Continue reading)

Aleksey Shipilev | 3 Jun 10:12 2011
Picon

Re: ThreadLocalRandom initial seed

David,

I had checked out jsr166 source, instrumented TLR, and this is the output:

TLR.<init>(): rnd = 0
TLR.next(): rnd before = 0
TLR.next(): rnd after = 11
0
TLR.next(): rnd before = 11
TLR.next(): rnd after = 277363943098
6118
TLR.next(): rnd before = 277363943098
TLR.next(): rnd after = 11718085204285
1895
TLR.next(): rnd before = 11718085204285
TLR.next(): rnd after = 49720483695876
7186
TLR.next(): rnd before = 49720483695876
TLR.next(): rnd after = 102626409374399
7366

So, rnd (TLR's "seed") is always 0, and setSeed() is not being called
from super-Random. Forcibly setting rnd = System.nanoTime() in
TLR.<init>() helps.

-Aleksey.
On Fri, Jun 3, 2011 at 10:43 AM, David Holmes <davidcholmes <at> aapt.net.au> wrote:
> Aleksey,
>
> The problem is not the seed - all threads do get a distinct seed. The problem is that the first call to
(Continue reading)

Mark Thornton | 3 Jun 10:57 2011

Re: ThreadLocalRandom initial seed

On 03/06/11 07:43, David Holmes wrote:
Aleksey, The problem is not the seed - all threads do get a distinct seed. The problem is that the first call to nextInt(n) is always returning zero - hence the sequence will always be the same. I haven't yet determined exactly what is going wrong. David
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.
ThreadLocalRandom() { super(0); // don't bother generating a seed we don't use setSeed(System.nanoTime()); initialized = true; } Mark Thornton

_______________________________________________
Concurrency-interest mailing list
Concurrency-interest <at> cs.oswego.edu
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
David Holmes | 3 Jun 13:38 2011
Picon

Re: ThreadLocalRandom initial seed

Thanks for correcxting me on that. This sure is confusing. It would seem that  6955840 although trying to allow setSeed to be called from the random constructor was completely pointless because Random stopped calling the setSeed method.
 
I hope Martin is watching and can give some input here. It certainly seems that we never properly initialize a ThreadLocalRandom!
 
David
-----Original Message-----
From: concurrency-interest-bounces <at> cs.oswego.edu [mailto:concurrency-interest-bounces <at> cs.oswego.edu]On Behalf Of Mark Thornton
Sent: Friday, 3 June 2011 6:58 PM
To: concurrency-interest <at> cs.oswego.edu
Subject: Re: [concurrency-interest] ThreadLocalRandom initial seed

On 03/06/11 07:43, David Holmes wrote:
Aleksey, The problem is not the seed - all threads do get a distinct seed. The problem is that the first call to nextInt(n) is always returning zero - hence the sequence will always be the same. I haven't yet determined exactly what is going wrong. David
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.
ThreadLocalRandom() { super(0); // don't bother generating a seed we don't use setSeed(System.nanoTime()); initialized = true; } Mark Thornton

_______________________________________________
Concurrency-interest mailing list
Concurrency-interest <at> cs.oswego.edu
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Mark Thornton | 3 Jun 15:11 2011

Re: ThreadLocalRandom initial seed

On 03/06/11 12:38, David Holmes wrote:
Thanks for correcxting me on that. This sure is confusing. It would seem that  6955840 although trying to allow setSeed to be called from the random constructor was completely pointless because Random stopped calling the setSeed method.
 
I hope Martin is watching and can give some input here. It certainly seems that we never properly initialize a ThreadLocalRandom!
 
David

ThreadLocalRandom() { super(0); // don't bother generating a seed we don't use setSeed(System.nanoTime()); initialized = true; } Mark Thornton

For people with very fast machines (and/or poor implementations of System.nanoTime()), the following initialisation may be better:

private static final Random seedSource = new Random(); ThreadLocalRandom() { super(0); // don't bother generating a seed we don't use setSeed(seedSource.nextLong()); initialized = true; }
Mark

_______________________________________________
Concurrency-interest mailing list
Concurrency-interest <at> cs.oswego.edu
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Martin Buchholz | 3 Jun 23:35 2011
Picon

Re: ThreadLocalRandom initial seed

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
 <at>  <at>  -6,6 +6,8  <at>  <at> 
 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 {

 <at>  <at>  -252,4 +254,40  <at>  <at> 
         }
     }

+    /**
+     * 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
 <at>  <at>  -118,7 +118,13  <at>  <at> 
      *  <at> 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) {

Gmane