rand: use ThreadRng wherever reseeding is ok

`SmallRng` was chosen in part due to limitation of old macOS versions. This is no longer relevant, since the affected versions are not supported by Rust anymore. Switch everything which does not need fixed sequences based on a seed to `ThreadRng`, which has better cryptographic properties and is occasionally reseeded. Performance differences should not matter much, since initialization of `ThreadRng` is not that expensive and it's generation speed is easily fast enough for our purposes. In some cases, like tests and the `random` builtin, we want a PRNG which consistently produces the same sequence of values for a given seed. `ThreadRng` does not do this, since it is occasionally reseeded automatically. In these cases, we keep using `SmallRng`. Part of #12030
2026-06-05 16:21:15 -03:00 · 2025-11-30 19:02:46 +01:00
parent c9ab2c26aa
commit 77fbd0a005
7 changed files with 73 additions and 38 deletions
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -566,6 +566,15 @@ version = "1.11.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f84267b20a16ea918e43c6a88433c2d54fa145c92a811b5b047ccbe153674483"

+[[package]]
+name = "ppv-lite86"
+version = "0.2.21"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "85eae3c4ed2f50dcfe72643da4befc30deadb458a9b590d720cde2f2b1e97da9"
+dependencies = [
+ "zerocopy",
+]
+
 [[package]]
 name = "proc-macro2"
 version = "1.0.101"
@@ -605,6 +614,17 @@ version = "0.9.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "6db2770f06117d490610c7488547d543617b21bfa07796d7a12f6f1bd53850d1"
 dependencies = [
+ "rand_chacha",
+ "rand_core 0.9.3",
+]
+
+[[package]]
+name = "rand_chacha"
+version = "0.9.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d3022b5f1df60f26e1ffddd6c66e8aa15de382ae63b3a0c1bfc0e4d3e3f325cb"
+dependencies = [
+ "ppv-lite86",
 "rand_core 0.9.3",
 ]

@@ -619,6 +639,9 @@ name = "rand_core"
 version = "0.9.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "99d9a13982dcf210057a8a78572b2217b667c3beacbf3a0d8b454f6f82837d38"
+dependencies = [
+ "getrandom 0.3.4",
+]

 [[package]]
 name = "redox_syscall"
@@ -970,3 +993,23 @@ name = "wit-bindgen"
 version = "0.46.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f17a85883d4e6d00e8a97c586de764dabcc06133f7f1d55dce5cdc070ad7fe59"
+
+[[package]]
+name = "zerocopy"
+version = "0.8.27"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0894878a5fa3edfd6da3f88c4805f4c8558e2b996227a3d864f47fe11e38282c"
+dependencies = [
+ "zerocopy-derive",
+]
+
+[[package]]
+name = "zerocopy-derive"
+version = "0.8.27"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "88d2b8d9c68ad2b9e4340d7832716a4d21a22a1154777ad56ea55c51a9cf3831"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -44,7 +44,10 @@ portable-atomic = { version = "1", default-features = false, features = [
    "fallback",
 ] }
 proc-macro2 = "1.0"
-rand = { version = "0.9.2", default-features = false, features = ["small_rng"] }
+rand = { version = "0.9.2", default-features = false, features = [
+    "small_rng",
+    "thread_rng",
+] }
 rsconf = "0.2.2"
 rust-embed = { version = "8.7.2", features = [
    "deterministic-timestamps",
--- a/src/builtins/random.rs
+++ b/src/builtins/random.rs
@@ -1,13 +1,14 @@
 use super::prelude::*;

-use crate::util::get_rng;
+use crate::util::get_seeded_rng;
 use crate::wutil;
 use once_cell::sync::Lazy;
 use rand::rngs::SmallRng;
-use rand::{Rng, SeedableRng};
+use rand::{Rng, RngCore};
 use std::sync::Mutex;

-static RNG: Lazy<Mutex<SmallRng>> = Lazy::new(|| Mutex::new(get_rng()));
+static RNG: Lazy<Mutex<SmallRng>> =
+    Lazy::new(|| Mutex::new(get_seeded_rng(rand::rng().next_u64())));

 pub fn random(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> BuiltinResult {
    let cmd = argv[0];
@@ -96,7 +97,7 @@ fn parse_ull(streams: &mut IoStreams, cmd: &wstr, num: &wstr) -> Result<u64, wut
                Err(_) => return Err(STATUS_INVALID_ARGS),
                Ok(x) => {
                    let mut engine = RNG.lock().unwrap();
-                    *engine = SmallRng::seed_from_u64(x as u64);
+                    *engine = get_seeded_rng(x as u64);
                }
            }
            return Ok(SUCCESS);
--- a/src/common.rs
+++ b/src/common.rs
@@ -2035,8 +2035,10 @@ mod tests {
        ScopeGuard, ScopedCell, ScopedRefCell, UnescapeStringStyle, bytes2wcstring, escape_string,
        truncate_at_nul, unescape_string, wcs2bytes,
    };
-    use crate::util::{get_rng_seed, get_seeded_rng};
-    use crate::wchar::{L, WString, wstr};
+    use crate::{
+        util::get_seeded_rng,
+        wchar::{L, WString, wstr},
+    };
    use rand::{Rng, RngCore};

    #[test]
@@ -2109,7 +2111,7 @@ fn test_escape_var() {
    }

    fn escape_test(escape_style: EscapeStringStyle, unescape_style: UnescapeStringStyle) {
-        let seed: u128 = 92348567983274852905629743984572;
+        let seed = rand::rng().next_u64();
        let mut rng = get_seeded_rng(seed);

        let mut random_string = WString::new();
@@ -2125,11 +2127,13 @@ fn escape_test(escape_style: EscapeStringStyle, unescape_style: UnescapeStringSt
            escaped_string = escape_string(&random_string, escape_style);
            let Some(unescaped_string) = unescape_string(&escaped_string, unescape_style) else {
                let slice = escaped_string.as_char_slice();
-                panic!("Failed to unescape string {slice:?}");
+                panic!("Failed to unescape string {slice:?}. Generated from seed {seed}.");
            };
            assert_eq!(
                random_string, unescaped_string,
-                "Escaped and then unescaped string {random_string:?}, but got back a different string {unescaped_string:?}. The intermediate escape looked like {escaped_string:?}."
+                "Escaped and then unescaped string {random_string:?}, but got back a different string {unescaped_string:?}. \
+                The intermediate escape looked like {escaped_string:?}. \
+                Generated from seed {seed}."
            );
        }
    }
@@ -2187,7 +2191,7 @@ fn bytes2hex(input: &[u8]) -> String {
    /// string comes back through double conversion.
    #[test]
    fn test_convert() {
-        let seed = get_rng_seed();
+        let seed = rand::rng().next_u64();
        let mut rng = get_seeded_rng(seed);
        let mut origin = Vec::new();

--- a/src/history/history.rs
+++ b/src/history/history.rs
@@ -57,7 +57,7 @@
    parse_util::{parse_util_detect_errors, parse_util_unescape_wildcards},
    path::{path_get_config, path_get_data, path_is_valid},
    threads::assert_is_background_thread,
-    util::{find_subslice, get_rng},
+    util::find_subslice,
    wchar::prelude::*,
    wcstringutil::subsequence_in_string,
    wildcard::{ANY_STRING, wildcard_match},
@@ -731,7 +731,7 @@ fn save_unless_disabled(&mut self) {
        // the counter.
        let countdown_to_vacuum = self
            .countdown_to_vacuum
-            .get_or_insert_with(|| get_rng().random_range(0..VACUUM_FREQUENCY));
+            .get_or_insert_with(|| rand::rng().random_range(0..VACUUM_FREQUENCY));

        // Determine if we're going to vacuum.
        let mut vacuum = false;
@@ -1779,12 +1779,11 @@ mod tests {
    use crate::fs::{LockedFile, WriteMethod};
    use crate::path::path_get_data;
    use crate::tests::prelude::*;
-    use crate::util::get_rng;
    use crate::wchar::prelude::*;
    use crate::wcstringutil::{string_prefixes_string, string_prefixes_string_case_insensitive};
    use fish_build_helper::workspace_root;
    use rand::Rng;
-    use rand::rngs::SmallRng;
+    use rand::rngs::ThreadRng;
    use std::collections::VecDeque;
    use std::io::BufReader;
    use std::os::unix::ffi::OsStrExt;
@@ -1806,7 +1805,7 @@ fn history_contains(history: &History, txt: &wstr) -> bool {
        false
    }

-    fn random_string(rng: &mut SmallRng) -> WString {
+    fn random_string(rng: &mut ThreadRng) -> WString {
        let mut result = WString::new();
        let max = rng.random_range(1..=32);
        for _ in 0..max {
@@ -1928,7 +1927,7 @@ macro_rules! test_history_matches {
        let mut after: VecDeque<HistoryItem> = VecDeque::new();
        history.clear();
        let max = 100;
-        let mut rng = get_rng();
+        let mut rng = rand::rng();
        for i in 1..=max {
            // Generate a value.
            let mut value = WString::from_str("test item ") + &i.to_wstring()[..];
--- a/src/util.rs
+++ b/src/util.rs
@@ -4,7 +4,6 @@
 use rand::{SeedableRng, rngs::SmallRng};
 use std::cmp::Ordering;
 use std::time;
-use std::time::{SystemTime, UNIX_EPOCH};

 /// Compares two wide character strings with an (arguably) intuitive ordering. This function tries
 /// to order strings in a way which is intuitive to humans with regards to sorting strings
@@ -172,27 +171,14 @@ pub fn get_time() -> i64 {
    }
 }

-// Helper to get a small RNG seed, based on nanoseconds.
-pub fn get_rng_seed() -> u128 {
-    // Note we use an explicit seed to avoid the "getrandom" crate, which uses `getentropy()` on macOS which
-    // is not available before macOS 10.12.
-    SystemTime::now()
-        .duration_since(UNIX_EPOCH)
-        .unwrap_or_default()
-        .as_nanos()
-}
-
-// Helper to get a small RNG with a seed.
-pub fn get_seeded_rng(seed: u128) -> SmallRng {
-    let seed = ((seed >> 64) as u64) ^ (seed as u64);
+/// Helper to get a small RNG with a seed.
+/// This should only be used for testing, where a PRNG which always produces the same output for a
+/// given seed is useful, e.g. to reproduce a failing test.
+/// In cases where reproducible results are not important, prefer `rand::rng()`.
+pub fn get_seeded_rng(seed: u64) -> SmallRng {
    SmallRng::seed_from_u64(seed)
 }

-// Helper to get a small RNG using the current time.
-pub fn get_rng() -> SmallRng {
-    get_seeded_rng(get_rng_seed())
-}
-
 // Compare the strings to see if they begin with an integer that can be compared and return the
 // result of that comparison.
 fn wcsfilecmp_leading_digits(a: &wstr, b: &wstr) -> (Ordering, usize, usize) {
--- a/src/wutil/mod.rs
+++ b/src/wutil/mod.rs
@@ -483,7 +483,6 @@ mod tests {
    use crate::common::wcs2bytes;
    use crate::fds::AutoCloseFd;
    use crate::tests::prelude::*;
-    use crate::util::get_rng;
    use crate::wchar::prelude::*;
    use libc::{O_CREAT, O_RDWR, O_TRUNC, SEEK_SET, c_void};
    use rand::Rng;
@@ -665,7 +664,7 @@ fn test_wwrite_to_fd() {
        let _cleanup = test_init();
        let temp_file = fish_tempfile::new_file().unwrap();
        let filename = CString::new(temp_file.path().to_str().unwrap()).unwrap();
-        let mut rng = get_rng();
+        let mut rng = rand::rng();
        let sizes = [1, 2, 3, 5, 13, 23, 64, 128, 255, 4096, 4096 * 2];
        for &size in &sizes {
            let fd = AutoCloseFd::new(unsafe {