Fix initrand to avoid random number sequences overlapping (#18744)

* Fix initrand to avoid random number sequences overlapping

* Minor fix

* Fix compile error on js backend

* Disable new test for js backend

* Minor fix

* tempfiles module uses random.initRand()

* Remove unused module import from lib/std/tempfiles.nim

* Initialize baseState in initRand()

* Run tests/stdlib/trandom.nim from tests/test_nimscript.nims

* baseState is initialized only with sysrand.urandom and quit if failed

* Add comments

lib/pure/random.nim

@@ -108,10 +108,19 @@ when defined(js):
     a0: 0x69B4C98Cu32,
     a1: 0xFED1DD30u32) # global for backwards compatibility
 else:
-  # racy for multi-threading but good enough for now:
+  const DefaultRandSeed = Rand(
-  var state = Rand(
     a0: 0x69B4C98CB8530805u64,
-    a1: 0xFED1DD3004688D67CAu64) # global for backwards compatibility
+    a1: 0xFED1DD3004688D67CAu64)
+
+  # racy for multi-threading but good enough for now:
+  var state = DefaultRandSeed # global for backwards compatibility
+
+func isValid(r: Rand): bool {.inline.} =
+  ## Check whether state of `r` is valid.
+  ##
+  ## In `xoroshiro128+`, if all bits of `a0` and `a1` are zero,
+  ## they are always zero after calling `next(r: var Rand)`.
+  not (r.a0 == 0 and r.a1 == 0)
 
 since (1, 5):
   template randState*(): untyped =
@@ -617,15 +626,28 @@ proc shuffle*[T](x: var openArray[T]) =
 
   shuffle(state, x)
 
-when not defined(nimscript) and not defined(standalone):
+when not defined(standalone):
-  import times
+  when defined(js):
+    import std/times
+  else:
+    when defined(nimscript):
+      import std/hashes
+    else:
+      import std/[hashes, os, sysrand, monotimes]
+
+      when compileOption("threads"):
+        import locks
+        var baseSeedLock: Lock
+        baseSeedLock.initLock
+
+    var baseState: Rand
 
   proc initRand(): Rand =
-    ## Initializes a new Rand state with a seed based on the current time.
+    ## Initializes a new Rand state.
     ##
     ## The resulting state is independent of the default RNG's state.
     ##
-    ## **Note:** Does not work for NimScript or the compile-time VM.
+    ## **Note:** Does not work for the compile-time VM.
     ##
     ## See also:
     ## * `initRand proc<#initRand,int64>`_ that accepts a seed for a new Rand state
@@ -635,20 +657,50 @@ when not defined(nimscript) and not defined(standalone):
       let time = int64(times.epochTime() * 1000) and 0x7fff_ffff
       result = initRand(time)
     else:
-      let now = times.getTime()
+      proc getRandomState(): Rand =
-      result = initRand(convert(Seconds, Nanoseconds, now.toUnix) + now.nanosecond)
+        when defined(nimscript):
+          result = Rand(
+            a0: CompileTime.hash.Ui,
+            a1: CompileDate.hash.Ui)
+          if not result.isValid:
+            result = DefaultRandSeed
+        else:
+          var urand: array[sizeof(Rand), byte]
+
+          for i in 0 .. 7:
+            if sysrand.urandom(urand):
+              copyMem(result.addr, urand[0].addr, sizeof(Rand))
+              if result.isValid:
+                break
+
+          if not result.isValid:
+            # Don't try to get alternative random values from other source like time or process/thread id,
+            # because such code would be never tested and is a liability for security.
+            quit("Failed to initializes baseState in random module as sysrand.urandom doesn't work.")
+
+      when compileOption("threads"):
+        baseSeedLock.withLock:
+          if not baseState.isValid:
+            baseState = getRandomState()
+          result = baseState
+          baseState.skipRandomNumbers
+      else:
+        if not baseState.isValid:
+          baseState = getRandomState()
+        result = baseState
+        baseState.skipRandomNumbers
 
   since (1, 5, 1):
     export initRand
 
   proc randomize*() {.benign.} =
     ## Initializes the default random number generator with a seed based on
-    ## the current time.
+    ## random number source.
     ##
     ## This proc only needs to be called once, and it should be called before
     ## the first usage of procs from this module that use the default RNG.
     ##
-    ## **Note:** Does not work for NimScript or the compile-time VM.
+    ## **Note:** Does not work for the compile-time VM.
     ##
     ## **See also:**
     ## * `randomize proc<#randomize,int64>`_ that accepts a seed

lib/std/tempfiles.nim

@@ -17,7 +17,7 @@ See also:
 * `mkstemp` (posix), refs https://man7.org/linux/man-pages/man3/mkstemp.3.html
 ]#
 
-import os, random, std/monotimes
+import os, random
 
 
 const
@@ -107,11 +107,8 @@ var nimTempPathState {.threadvar.}: NimTempPathState
 template randomPathName(length: Natural): string =
   var res = newString(length)
   if not nimTempPathState.isInit:
-    var time = getMonoTime().ticks
-    when compileOption("threads"):
-      time = time xor int64(getThreadId())
     nimTempPathState.isInit = true
-    nimTempPathState.state = initRand(time)
+    nimTempPathState.state = initRand()
 
   for i in 0 ..< length:
     res[i] = nimTempPathState.state.sample(letters)

tests/stdlib/trandom.nim

@@ -23,9 +23,10 @@ proc main() =
   doAssert a in [[0,1], [1,0]]
 
   doAssert rand(0) == 0
-  doAssert sample("a") == 'a'
+  when not defined(nimscript):
+    doAssert sample("a") == 'a'
 
-  when compileOption("rangeChecks"):
+  when compileOption("rangeChecks") and not defined(nimscript):
     doAssertRaises(RangeDefect):
       discard rand(-1)
 
@@ -92,7 +93,7 @@ block: # random int
 
   block: # again gives new numbers
     var rand1 = rand(1000000)
-    when not defined(js):
+    when not (defined(js) or defined(nimscript)):
       os.sleep(200)
 
     var rand2 = rand(1000000)
@@ -122,7 +123,7 @@ block: # random float
 
   block: # again gives new numbers
     var rand1: float = rand(1000000.0)
-    when not defined(js):
+    when not (defined(js) or defined(nimscript)):
       os.sleep(200)
 
     var rand2: float = rand(1000000.0)
@@ -248,3 +249,26 @@ block: # bug #17670
     type UInt48 = range[0'u64..2'u64^48-1]
     let x = rand(UInt48)
     doAssert x is UInt48
+
+block: # bug #17898
+  # Checks whether `initRand()` generates unique states.
+  # size should be 2^64, but we don't have time and space.
+
+  # Disable this test for js until js gets proper skipRandomNumbers.
+  when not defined(js):
+    const size = 1000
+    var
+      rands: array[size, Rand]
+      randSet: HashSet[Rand]
+    for i in 0..<size:
+      rands[i] = initRand()
+      randSet.incl rands[i]
+
+    doAssert randSet.len == size
+
+    # Checks random number sequences overlapping.
+    const numRepeat = 100
+    for i in 0..<size:
+      for j in 0..<numRepeat:
+        discard rands[i].next
+        doAssert rands[i] notin randSet

tests/test_nimscript.nims

@@ -71,6 +71,8 @@ import std/[
   decls, compilesettings, with, wrapnils
 ]
 
+import stdlib/trandom
+
 echo "Nimscript imports are successful."
 
 block: