Fix random numbers not advancing the rng when seeded

CHANGELOG.md

@@ -8,6 +8,8 @@ All notable changes to MiniJinja are documented here.
 - The contrib crate now uses a basic xorrand implementation instead
   of depending on all of the `rand` module.  #696
 - Added temps, a way to stash away temporary state during rendering.  #697
+- Fixed a bug that caused the random functions in the contrib crate
+  to not advance the RNG between calls.  #698
 
 ## 2.8.0
 

minijinja-contrib/src/filters/mod.rs

@@ -75,12 +75,11 @@ pub fn pluralize(
 #[cfg(feature = "rand")]
 #[cfg_attr(docsrs, doc(cfg(feature = "rand")))]
 pub fn random(state: &minijinja::State, seq: &Value) -> Result<Value, Error> {
-    use crate::globals::get_rng;
     use minijinja::value::ValueKind;
 
     if matches!(seq.kind(), ValueKind::Seq | ValueKind::String) {
         let len = seq.len().unwrap_or(0);
-        let idx = get_rng(state).next_usize(len);
+        let idx = crate::rand::XorShiftRng::for_state(state).next_usize(len);
         seq.get_item_by_index(idx)
     } else {
         Err(Error::new(

minijinja-contrib/src/globals.rs

@@ -123,19 +123,6 @@ pub fn joiner(sep: Option<Value>) -> Value {
     })
 }
 
-/// Returns the rng for the state
-#[cfg(feature = "rand")]
-pub(crate) fn get_rng(state: &State) -> crate::rand::XorShiftRng {
-    // XXX: we have no way to stash the rng away today which renders this
-    // feature rather useless.  Repeated calls to randrange etc. will
-    // always yield the same number.
-    crate::rand::XorShiftRng::new(
-        state
-            .lookup("RAND_SEED")
-            .and_then(|x| u64::try_from(x).ok()),
-    )
-}
-
 /// Returns a random number in a given range.
 ///
 /// If only one parameter is provided it's taken as exclusive upper
@@ -152,7 +139,7 @@ pub fn randrange(state: &State, n: i64, m: Option<i64>) -> i64 {
         Some(m) => (n, m),
     };
 
-    get_rng(state).random_range(lower, upper)
+    crate::rand::XorShiftRng::for_state(state).random_range(lower, upper)
 }
 
 /// Generates a random lorem ipsum.
@@ -215,7 +202,7 @@ pub fn lipsum(
     let n = n.or(n_kwargs).unwrap_or(5);
     let mut rv = String::new();
 
-    let mut rng = get_rng(state);
+    let rng = crate::rand::XorShiftRng::for_state(state);
 
     for _ in 0..n {
         let mut next_capitalized = true;

minijinja-contrib/src/rand.rs

@@ -1,12 +1,14 @@
 use std::collections::hash_map::RandomState;
 use std::hash::{BuildHasher, Hasher};
+use std::sync::atomic::{AtomicU64, Ordering};
 use std::sync::Arc;
 
 use minijinja::value::{Object, ObjectRepr};
+use minijinja::State;
 
 #[derive(Debug)]
 pub struct XorShiftRng {
-    seed: u64,
+    seed: AtomicU64,
 }
 
 impl Object for XorShiftRng {
@@ -16,33 +18,42 @@ impl Object for XorShiftRng {
 }
 
 impl XorShiftRng {
+    pub fn for_state(state: &State) -> Arc<XorShiftRng> {
+        state.get_or_set_temp_object("minijinja-contrib-rng", || {
+            XorShiftRng::new(
+                state
+                    .lookup("RAND_SEED")
+                    .and_then(|x| u64::try_from(x).ok()),
+            )
+        })
+    }
+
     pub fn new(seed: Option<u64>) -> XorShiftRng {
         XorShiftRng {
-            seed: match seed {
-                Some(seed) => seed,
-                None => RandomState::new().build_hasher().finish(),
-            },
+            seed: AtomicU64::from(
+                seed.unwrap_or_else(|| RandomState::new().build_hasher().finish()),
+            ),
         }
     }
 
-    pub fn next(&mut self) -> u64 {
-        let mut rv = self.seed;
+    pub fn next(&self) -> u64 {
+        let mut rv = self.seed.load(Ordering::Relaxed);
         rv ^= rv << 13;
         rv ^= rv >> 7;
         rv ^= rv << 17;
-        self.seed = rv;
+        self.seed.store(rv, Ordering::Relaxed);
         rv
     }
 
-    pub fn next_usize(&mut self, max: usize) -> usize {
+    pub fn next_usize(&self, max: usize) -> usize {
         (self.random() * max as f64) as usize
     }
 
-    pub fn random(&mut self) -> f64 {
+    pub fn random(&self) -> f64 {
         (self.next() as f64) / (u64::MAX as f64)
     }
 
-    pub fn random_range(&mut self, lower: i64, upper: i64) -> i64 {
+    pub fn random_range(&self, lower: i64, upper: i64) -> i64 {
         (self.random() * (upper - lower) as f64) as i64 + lower
     }
 }

minijinja-contrib/tests/globals.rs

@@ -80,6 +80,6 @@ fn test_randrange() {
     let mut env = Environment::new();
     env.add_function("randrange", randrange);
 
-    assert_snapshot!(render!(in env, r"{% set RAND_SEED = 42 %}{{ randrange(10) }}"), @"0");
+    assert_snapshot!(render!(in env, r"{% set RAND_SEED = 42 %}{{ randrange(10) }}|{{ randrange(10) }}"), @"0|6");
     assert_snapshot!(render!(in env, r"{% set RAND_SEED = 42 %}{{ randrange(-50, 50) }}"), @"-50");
 }