JRUBY-6324: random seed for srand is not initialized properly

jruby -e 'p srand' returns previous (initial) srand seed.  In 1.8 mode,
it always returns 0 but it's not correct.  In 1.9 mode, it crashes with
NullPointerException because it returns null as RubyObject.  Anyway,
initial seed value is not initialized properly.

This commit includes several fixes for Random.

* Initialize the runtime-local PRNG and seed at Ruby#initCore().

* Keep seed for PRNG as not long but RubyInteger.
  % jruby165 -e 'n = 2**128; srand(n); p [n.object_id, srand.object_id]'
  [2000, 1] => should be the same object

* Introduced Ruby#randomSeedRandom as the runtime-local PRNG for
  generating seed by srand.  Do not use System.currentTimeMillis which
  is not suited for PRNG seed.

* Removed globalRandom in RubyRandom class which is used only in 1.9
  mode.  Use Ruby#getRandom() for global srand as same as 1.8 mode.

spec/regression/JRUBY-6324_random_srand_spec.rb

@@ -0,0 +1,14 @@
+require 'java'
+
+describe "JRUBY-6324: random seed for srand is not initialized properly" do
+  it "initializes initial seed for PRNG" do
+    rt = Java.org.jruby.embed.ScriptingContainer.new
+    rt.run_scriptlet("srand").should > 0
+  end
+
+  it "initializes initial seed for PRNG" do
+    rt = Java.org.jruby.embed.ScriptingContainer.new
+    id1, id2 = rt.run_scriptlet("n = 2**128; srand(n); [n.object_id, srand.object_id]")
+    id1.should == id2
+  end
+end

src/org/jruby/Ruby.java

@@ -292,7 +292,7 @@ private Ruby(RubyInstanceConfig config) {
         this.jitCompiler        = new JITCompiler(this);
         this.parserStats        = new ParserStats(this);
 
-	this.hashSeed = this.random.nextInt();
+        this.hashSeed = this.random.nextInt();
 
         this.beanManager.register(new Config(this));
         this.beanManager.register(parserStats);
@@ -1300,6 +1300,13 @@ private void initCore() {
         if (is1_9()) {
             RubyRandom.createRandomClass(this);
         }
+        // initialize Random and RandomSeed as a part of Core since 1.8 mode
+        // does not have Random class. Ugly, but these definitions has been
+        // already rewritten at master for 1.7 release. It's 1.6 only logic.
+        long seedArg = newRandomSeed();
+        RubyBignum newSeed = RubyBignum.newBignum(this, seedArg);
+        this.setRandomSeed((RubyInteger) newSeed);
+        this.getRandom().setSeed(seedArg);
     }
 
     public static final int NIL_PREFILLED_ARRAY_SIZE = RubyArray.ARRAY_DEFAULT_SIZE * 8;
@@ -3382,18 +3389,24 @@ public RubySymbol.SymbolTable getSymbolTable() {
         return symbolTable;
     }
 
-    public void setRandomSeed(long randomSeed) {
+    public void setRandomSeed(RubyInteger randomSeed) {
         this.randomSeed = randomSeed;
     }
 
-    public long getRandomSeed() {
+    public RubyInteger getRandomSeed() {
         return randomSeed;
     }
 
     public Random getRandom() {
         return random;
     }
 
+    // Returns unsigned long (0..0x7fffffffffffffff) to follow CRuby's behavior
+    // (default seed is positive always)
+    public long newRandomSeed() {
+        return randomSeedRandom.nextLong() & 0x7fffffffffffffffL;
+    }
+
     public ObjectSpace getObjectSpace() {
         return objectSpace;
     }
@@ -3455,6 +3468,7 @@ public ChannelDescriptor getDescriptorByFileno(int aFileno) {
         return ChannelDescriptor.getDescriptorByFileno(aFileno);
     }
 
+    @Deprecated
     public long incrementRandomSeedSequence() {
         return randomSeedSequence++;
     }
@@ -3953,9 +3967,14 @@ public CoverageData getCoverageData() {
 
     private final RubySymbol.SymbolTable symbolTable = new RubySymbol.SymbolTable(this);
 
-    private long randomSeed = 0;
+    @Deprecated
     private long randomSeedSequence = 0;
+    /** The runtime-local PRNG for generating seed by srand */
+    private Random randomSeedRandom = new Random();
+    /** The runtime-local PRNG for Kernel.rand */
     private Random random = new Random();
+    /** The seed object for the seed value of the runtime-local PRNG */
+    private RubyInteger randomSeed;
     /** The runtime-local seed for hash randomization */
     private int hashSeed = 0;
 

src/org/jruby/RubyKernel.java

@@ -1580,34 +1580,29 @@ public static IRubyObject backquote(ThreadContext context, IRubyObject recv, IRu
     public static RubyInteger srand(ThreadContext context, IRubyObject recv) {
         Ruby runtime = context.getRuntime();
 
-        // Not sure how well this works, but it works much better than
-        // just currentTimeMillis by itself.
-        long oldRandomSeed = runtime.getRandomSeed();
-        runtime.setRandomSeed(System.currentTimeMillis() ^
-               recv.hashCode() ^ runtime.incrementRandomSeedSequence() ^
-               runtime.getRandom().nextInt(Math.max(1, Math.abs((int)runtime.getRandomSeed()))));
-
-        runtime.getRandom().setSeed(runtime.getRandomSeed());
-        return runtime.newFixnum(oldRandomSeed);
+        RubyInteger oldRandomSeed = runtime.getRandomSeed();
+        long seedArg = runtime.newRandomSeed();
+        runtime.setRandomSeed(RubyBignum.newBignum(runtime, seedArg));
+        runtime.getRandom().setSeed(seedArg);
+        return oldRandomSeed;
     }
 
     @JRubyMethod(name = "srand", module = true, visibility = PRIVATE, compat = RUBY1_8)
     public static RubyInteger srand(ThreadContext context, IRubyObject recv, IRubyObject arg) {
         IRubyObject newRandomSeed = arg.convertToInteger("to_int");
         Ruby runtime = context.getRuntime();
 
+        RubyInteger oldRandomSeed = runtime.getRandomSeed();
         long seedArg = 0;
         if (newRandomSeed instanceof RubyBignum) {
             seedArg = ((RubyBignum)newRandomSeed).getValue().longValue();
         } else if (!arg.isNil()) {
             seedArg = RubyNumeric.num2long(newRandomSeed);
         }
 
-        long oldRandomSeed = runtime.getRandomSeed();
-        runtime.setRandomSeed(seedArg);
-
-        runtime.getRandom().setSeed(runtime.getRandomSeed());
-        return runtime.newFixnum(oldRandomSeed);
+        runtime.setRandomSeed((RubyInteger)newRandomSeed);
+        runtime.getRandom().setSeed(seedArg);
+        return oldRandomSeed;
     }
 
 
@@ -1638,15 +1633,13 @@ public static RubyNumeric rand(ThreadContext context, IRubyObject recv, IRubyObj
     @JRubyMethod(name = "rand", module = true, visibility = PRIVATE, compat = RUBY1_9)
     public static RubyNumeric rand19(ThreadContext context, IRubyObject recv) {
         Ruby runtime = context.getRuntime();
-        return RubyFloat.newFloat(runtime, RubyRandom.globalRandom.nextDouble());
+        return RubyFloat.newFloat(runtime, runtime.getRandom().nextDouble());
     }
 
     @JRubyMethod(name = "rand", module = true, visibility = PRIVATE, compat = RUBY1_9)
     public static RubyNumeric rand19(ThreadContext context, IRubyObject recv, IRubyObject arg) {
         Ruby runtime = context.getRuntime();
-        Random random = RubyRandom.globalRandom;
-
-        return randCommon(context, runtime, random, recv, arg);
+        return randCommon(context, runtime, runtime.getRandom(), recv, arg);
     }
 
     private static RubyNumeric randCommon(ThreadContext context, Ruby runtime, Random random, IRubyObject recv, IRubyObject arg) {

src/org/jruby/RubyRandom.java

@@ -45,9 +45,6 @@
 @JRubyClass(name = "Random")
 public class RubyRandom extends RubyObject {
 
-    public static Random globalRandom = new Random();
-    private static IRubyObject globalSeed;
-
     private Random random = new Random();
     private IRubyObject seed;
 
@@ -57,9 +54,6 @@ public static RubyClass createRandomClass(Ruby runtime) {
 
         randomClass.defineAnnotatedMethods(RubyRandom.class);
 
-        // initialize the global seed by triggering an iteration
-        srandCommon(runtime.getCurrentContext(), randomClass, runtime.getNil(), false);
-
         return randomClass;
     }
 
@@ -107,12 +101,12 @@ public IRubyObject seed(ThreadContext context) {
 
     @JRubyMethod(name = "rand", meta = true, compat = RUBY1_9)
     public static IRubyObject rand(ThreadContext context, IRubyObject recv) {
-        return randCommon(context, context.nil, globalRandom, false);
+        return randCommon(context, context.nil, context.runtime.getRandom(), false);
     }
 
     @JRubyMethod(name = "rand", meta = true, compat = RUBY1_9)
     public static IRubyObject rand(ThreadContext context, IRubyObject recv, IRubyObject arg0) {
-        return randCommon(context, arg0, globalRandom, false);
+        return randCommon(context, arg0, context.runtime.getRandom(), false);
     }
 
     @JRubyMethod(name = "rand", compat = RUBY1_9)
@@ -205,7 +199,7 @@ public static IRubyObject srand(ThreadContext context, IRubyObject recv, IRubyOb
     public static IRubyObject srandCommon(ThreadContext context, IRubyObject recv, IRubyObject arg, boolean acceptZero) {
         Ruby runtime = context.getRuntime();
         IRubyObject newSeed = arg;
-        IRubyObject previousSeed = globalSeed;
+        IRubyObject oldRandomSeed = runtime.getRandomSeed();
 
         long seedArg = 0;
         if (arg instanceof RubyBignum) {
@@ -215,16 +209,13 @@ public static IRubyObject srandCommon(ThreadContext context, IRubyObject recv, I
         }
 
         if (arg.isNil() || (!acceptZero && seedArg == 0)) {
-            newSeed = RubyNumeric.int2fix(runtime, System.currentTimeMillis() ^
-                recv.hashCode() ^ runtime.incrementRandomSeedSequence() ^
-                runtime.getRandom().nextInt(Math.max(1, Math.abs((int)runtime.getRandomSeed()))));
-            seedArg = RubyNumeric.fix2long(newSeed);
+            seedArg = runtime.newRandomSeed();
+            newSeed = RubyBignum.newBignum(runtime, seedArg);
         }
 
-        globalSeed = newSeed;
-        globalRandom.setSeed(seedArg);
-
-        return previousSeed;
+        runtime.setRandomSeed((RubyInteger) newSeed);
+        runtime.getRandom().setSeed(seedArg);
+        return oldRandomSeed;
     }
 
     @Override
@@ -274,7 +265,7 @@ public IRubyObject bytes(ThreadContext context, IRubyObject arg) {
     public static double randomReal(ThreadContext context, IRubyObject obj) {
         Random random = null;
         if (obj.equals(context.runtime.getRandomClass())) {
-            random = globalRandom;
+            random = context.runtime.getRandom();
         }
         if (obj instanceof RubyRandom) {
             random = ((RubyRandom) obj).random;
@@ -292,9 +283,10 @@ public static double randomReal(ThreadContext context, IRubyObject obj) {
     @JRubyMethod(name = "new_seed", meta = true, compat = RUBY1_9)
     public static IRubyObject newSeed(ThreadContext context, IRubyObject recv) {
         Ruby runtime = context.getRuntime();
-        globalRandom = new Random();
-        long rand = globalRandom.nextLong();
-        globalRandom.setSeed(rand);
-        return RubyBignum.newBignum(runtime, rand);
+        long rand = runtime.getRandom().nextLong();
+        RubyBignum newSeed = RubyBignum.newBignum(runtime, rand);
+        runtime.setRandomSeed(newSeed);
+        runtime.getRandom().setSeed(rand);
+        return newSeed;
     }
 }