7
\$\begingroup\$

For a server-side Java application I want to have a random number generator. I'm using the ISAAC algorithm from Apache Commons Math 3 because it is both cryptographically strong and fast. I have written the following code to initialise the generator:

public class ThreadLocalRandomGenerator {
    private static ThreadLocal<RandomGenerator> instances = new ThreadLocal<>();

    public static RandomGenerator get() {
        RandomGenerator generator = instances.get();
        // Initialise new generator if needed.
        if (generator == null) {
            int[] seed = new int[256];
            seed[0] = (int) (System.currentTimeMillis() & 0xffffffffL);
            seed[1] = System.identityHashCode(new Object());
            for (int i=2 ; i<256 ; i++) {
                seed[i] = 0;
                for (int j=0; j<32; j++) {
                    seed[i] |= (int) ((System.nanoTime() & 1L) << j);
                }
            }
            System.out.println("Initialised random seed: " + Arrays.toString(seed));
            // Create generator.
            generator = new ISAACRandom(seed);
            instances.set(generator);
            // Warm up.
            for (int i=0 ; i<1_000 ; i++) {
                generator.nextLong();
            }
        }
        return generator;
    }
}

Basically I'm using a ThreadLocal to keep several generators around for use in threads, and I'm initialising each one using an array of integers generated from the least significant bits of nanoTime(). The reason for using 256 integers is because that's the internal size of the ISAACRandom class.

Does this seem sensible?

\$\endgroup\$
4
  • \$\begingroup\$ What problem does this solve (what's its use-case)? \$\endgroup\$
    – Mast
    Oct 24, 2015 at 17:26
  • \$\begingroup\$ The problem is I want a high quality random generator and I want it initialised properly. Anyone can use a random number generator, but if it's not initialised properly then it could emit low-quality numbers. \$\endgroup\$
    – KennethJ
    Oct 24, 2015 at 17:38
  • \$\begingroup\$ @Winther Some PRNGs may produce non-random data for the first few outputs. Such a vulnerability was discovered in RC4 (en.wikipedia.org/wiki/RC4#Fluhrer.2C_Mantin_and_Shamir_attack). I doubt such a vulnerability exists in ISAAC or other secure PRNGs, but I don't know for sure, so it's better to be safe than sorry. \$\endgroup\$
    – KennethJ
    Oct 25, 2015 at 23:57
  • 3
    \$\begingroup\$ Please don't edit code into your post after receiving reviews. If you would like that code reviewed, you can create a new question. \$\endgroup\$
    – SirPython
    Nov 11, 2015 at 20:04

1 Answer 1

8
\$\begingroup\$

For the most part, that's nice code. I can read it somewhat easily, and for the most part, it is logical, and structured.

There are a number of issues in your code, though, some style issues, and some functional issues.

First up, the style issues.

Using the existing libraries

There's a nice feature in ThreadLocal in Java8 that allows you to have a supplier for the structure. Consider a method like:

private static RandomGenerator buildGenerator() {
    .....
}

then your thread local can be created as:

private static ThreadLocal<RandomGenerator> instances = 
       ThreadLocal.withInitial(ThreadLocalRandomGenerator::buildGenerator);

Your main method can become a simple:

public static RandomGenerator get() {
    return instances.get();
}

It is guaranteed that there will be an instance created for you on the first call from a thread.

Getting good seeding

Your seeding system is both complicated, and wrong.

If you are looking to have a special random generator that you feel out-performs the existing Java Random class, that's OK, but, it needs to be seeded right.

There are a few problems in your seeding that I can see. First up:

seed[0] = (int) (System.currentTimeMillis() & 0xffffffffL);

That can be simply an int cast. No need to mask off the bits:

seed[0] = (int)System.currentTimeMillis();

Next up, is that computers have different precision in their nano clocks. A nano-second is far faster than the actual computer's time interrupt, so, it is common for computers to "step" significantly as it updates. The steps can be as much as 190 nanoseconds is where i have seen it....

... what that means, is that the results for all your seed values here may be the same:

seed[i] |= (int) ((System.nanoTime() & 1L) << j);

In other words, you will have int values filled with either 0's or 1's... and no variations. Also, if the "step" for the nano-second clock happens to be an even number, it is possible that the low-order bit for your computer will always be the same value.... and all your seeds will be identical.

While the Random class in Java may be slower than you want, you can at least use it to seed the faster ISAAC algorithm.

I would simply use the following to seed your class:

Random rand = new Random();
int seed[] = rand.ints(256).toArray();

Use the slower Random to seed your class, then let your class run with the rest.

Note, there is no real way to use the time systems in a computer to get random values... it's just not infused with enough entropy to be random, or predictability to be reliable.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you for your comments. Good point about using ThreadLocal with a supplier. As for using Random as a seed for ISAAC, that seems counter-productive to me. The reason I want to use ISAAC is not speed, but mainly quality. I'm no expert, but it's my understanding that ISAAC is of much higher quality than Java's default Random, which I believe is a simple linear congruential generator. Seeding a superior generator with data from an inferior one defeats the purpose no? I'll test the nanoTime properly, perhaps do a self-check with a fallback. Thanks for pointing that out. \$\endgroup\$
    – KennethJ
    Oct 24, 2015 at 18:22
  • \$\begingroup\$ Agreed, the RNG should be seeded from SecureRandom \$\endgroup\$
    – Tim McLean
    Oct 26, 2015 at 4:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.