Concurrent calls to new Random() not random enough

[Martin Buchholz]

[+fy, jeremymanson]

Here's a much better test case,
and a proposed fix:

http://cr.openjdk.java.net/~martin/webrevs/openjdk7/RandomSeedCollisions

This adds some initialization overhead, but also removes some
since
new Random()
no longer invokes a synchronized method.

----
import java.util.*;

public class RandomSeed {
    public static void main(String[] args) throws Throwable {
        class RandomCollector implements Runnable {
            long[] randoms = new long[1<<22];
            public void run() {
                for (int i = 0; i < randoms.length; i++)
                    randoms[i] = new Random().nextLong();
            }};
        final int threadCount = 2;
        List<RandomCollector> collectors = new ArrayList<RandomCollector>();
        List<Thread> threads = new ArrayList<Thread>();
        for (int i = 0; i < threadCount; i++) {
            RandomCollector r = new RandomCollector();
            collectors.add(r);
            threads.add(new Thread(r));
        }
        for (Thread thread : threads)
            thread.start();
        for (Thread thread : threads)
            thread.join();
        int collisions = 0;
        HashSet<Long> s = new HashSet<Long>();
        for (RandomCollector r : collectors) {
            for (long x : r.randoms) {
                if (s.contains(x))
                    collisions++;
                s.add(x);
            }
        }
        System.out.printf("collisions=%d%n", collisions);
    }
}


On Tue, Mar 23, 2010 at 15:50, Martin Buchholz <martinrb at google.com> wrote:
> Hi Sherman,
>
> This is a bug report (sorry, no fix this time)
>
> Synopsis: Concurrent calls to new Random() not random enough
> Description:
> new Random() promises this:
>    /**
>     * Creates a new random number generator. This constructor sets
>     * the seed of the random number generator to a value very likely
>     * to be distinct from any other invocation of this constructor.
>     */
>
> but if there are concurrent calls to new Random(), it does not
> do very well at fulfilling its contract.
>
> The following program should print out a number much closer to 0.
>
> import java.util.*;
>
> public class RandomSeed {
>    public static void main(String[] args) throws Throwable {
>        class RandomCollector implements Runnable {
>            int[] randoms = new int[1<<21];
>            public void run() {
>                for (int i = 0; i < randoms.length; i++)
>                    randoms[i] = new Random().nextInt();
>            }};
>        final int threadCount = 2;
>        List<RandomCollector> collectors = new ArrayList<RandomCollector>();
>        List<Thread> threads = new ArrayList<Thread>();
>        for (int i = 0; i < threadCount; i++) {
>            RandomCollector r = new RandomCollector();
>            collectors.add(r);
>            threads.add(new Thread(r));
>        }
>        for (Thread thread : threads)
>            thread.start();
>        for (Thread thread : threads)
>            thread.join();
>        int collisions = 0;
>        HashSet<Integer> s = new HashSet<Integer>();
>        for (RandomCollector r : collectors) {
>            for (int x : r.randoms) {
>                if (s.contains(x))
>                    collisions++;
>                s.add(x);
>            }
>        }
>        System.out.println(collisions);
>    }
> }
> ---
> ==> javac -source 1.6 -Xlint:all RandomSeed.java
> ==> java -esa -ea RandomSeed
> 876
>