From bb4b01f6428a4f577e86c8e95b788080b43bbf76 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Fri, 28 Aug 2020 16:54:30 +0100 Subject: [PATCH 1/4] Fix #1022: doc for "sized iterators" --- src/seq/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/seq/mod.rs b/src/seq/mod.rs index dd542228689..2abc5abceec 100644 --- a/src/seq/mod.rs +++ b/src/seq/mod.rs @@ -265,7 +265,8 @@ pub trait SliceRandom { /// Extension trait on iterators, providing random sampling methods. /// -/// This trait is implemented on all sized iterators, providing methods for +/// This trait is implemented on all iterators `I` where `I: Iterator + Sized` +/// and provides methods for /// choosing one or more elements. You must `use` this trait: /// /// ``` From c368f7f4feda2129301e1baf52bb3fb0c612190a Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Tue, 1 Sep 2020 11:17:22 +0100 Subject: [PATCH 2/4] Change samples to support ThreadRng: !Copy --- src/distributions/mod.rs | 12 ++++++++---- src/rng.rs | 13 ++++++++----- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/distributions/mod.rs b/src/distributions/mod.rs index 2bd47952056..652f52a1831 100644 --- a/src/distributions/mod.rs +++ b/src/distributions/mod.rs @@ -162,17 +162,21 @@ pub trait Distribution { /// use rand::thread_rng; /// use rand::distributions::{Distribution, Alphanumeric, Uniform, Standard}; /// - /// let rng = thread_rng(); + /// let mut rng = thread_rng(); /// /// // Vec of 16 x f32: - /// let v: Vec = Standard.sample_iter(rng).take(16).collect(); + /// let v: Vec = Standard.sample_iter(&mut rng).take(16).collect(); /// /// // String: - /// let s: String = Alphanumeric.sample_iter(rng).take(7).map(char::from).collect(); + /// let s: String = Alphanumeric + /// .sample_iter(&mut rng) + /// .take(7) + /// .map(char::from) + /// .collect(); /// /// // Dice-rolling: /// let die_range = Uniform::new_inclusive(1, 6); - /// let mut roll_die = die_range.sample_iter(rng); + /// let mut roll_die = die_range.sample_iter(&mut rng); /// while roll_die.next().unwrap() != 6 { /// println!("Not a 6; rolling again!"); /// } diff --git a/src/rng.rs b/src/rng.rs index 5261b1c14aa..a5ad2cdf4f8 100644 --- a/src/rng.rs +++ b/src/rng.rs @@ -165,21 +165,24 @@ pub trait Rng: RngCore { /// use rand::{thread_rng, Rng}; /// use rand::distributions::{Alphanumeric, Uniform, Standard}; /// - /// let rng = thread_rng(); + /// let mut rng = thread_rng(); /// /// // Vec of 16 x f32: - /// let v: Vec = rng.sample_iter(Standard).take(16).collect(); + /// let v: Vec = (&mut rng).sample_iter(Standard).take(16).collect(); /// /// // String: - /// let s: String = rng.sample_iter(Alphanumeric).take(7).map(char::from).collect(); + /// let s: String = (&mut rng).sample_iter(Alphanumeric) + /// .take(7) + /// .map(char::from) + /// .collect(); /// /// // Combined values - /// println!("{:?}", rng.sample_iter(Standard).take(5) + /// println!("{:?}", (&mut rng).sample_iter(Standard).take(5) /// .collect::>()); /// /// // Dice-rolling: /// let die_range = Uniform::new_inclusive(1, 6); - /// let mut roll_die = rng.sample_iter(die_range); + /// let mut roll_die = (&mut rng).sample_iter(die_range); /// while roll_die.next().unwrap() != 6 { /// println!("Not a 6; rolling again!"); /// } From 81cfe132815040360758f4616f135facdb25d988 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Tue, 1 Sep 2020 11:18:00 +0100 Subject: [PATCH 3/4] ThreadRng: use Rc> --- CHANGELOG.md | 1 + src/rngs/thread.rs | 58 +++++++++++++++++++++++++++++----------------- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b8b3236eae..99070bb005f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ You may also find the [Upgrade Guide](https://rust-random.github.io/book/update. - Implement weighted sampling without replacement (#976, #1013) ### Changes +- `ThreadRng` is no longer `Copy` to enable safe usage within thread-local destructors (see #968) - `gen_range(a, b)` was replaced with `gen_range(a..b)`, and `gen_range(a..=b)` is supported (#744, #1003). Note that `a` and `b` can no longer be references or SIMD types. - Replace `AsByteSliceMut` with `Fill` (#940) diff --git a/src/rngs/thread.rs b/src/rngs/thread.rs index 3a216bc93db..14be2247068 100644 --- a/src/rngs/thread.rs +++ b/src/rngs/thread.rs @@ -9,7 +9,7 @@ //! Thread-local random number generator use core::cell::UnsafeCell; -use core::ptr::NonNull; +use std::rc::Rc; use std::thread_local; use super::std::Core; @@ -37,17 +37,19 @@ use crate::{CryptoRng, Error, RngCore, SeedableRng}; // of 32 kB and less. We choose 64 kB to avoid significant overhead. const THREAD_RNG_RESEED_THRESHOLD: u64 = 1024 * 64; -/// The type returned by [`thread_rng`], essentially just a reference to the -/// PRNG in thread-local memory. +/// A reference to the thread-local generator /// -/// `ThreadRng` uses the same PRNG as [`StdRng`] for security and performance. -/// As hinted by the name, the generator is thread-local. `ThreadRng` is a -/// handle to this generator and thus supports `Copy`, but not `Send` or `Sync`. +/// An instance can be obtained via [`thread_rng`] or via `ThreadRng::default()`. +/// This handle is safe to use everywhere (including thread-local destructors) +/// but cannot be passed between threads (is not `Send` or `Sync`). /// -/// Unlike `StdRng`, `ThreadRng` uses the [`ReseedingRng`] wrapper to reseed -/// the PRNG from fresh entropy every 64 kiB of random data. -/// [`OsRng`] is used to provide seed data. +/// `ThreadRng` uses the same PRNG as [`StdRng`] for security and performance +/// and is automatically seeded from [`OsRng`]. /// +/// Unlike `StdRng`, `ThreadRng` uses the [`ReseedingRng`] wrapper to reseed +/// the PRNG from fresh entropy every 64 kiB of random data as well as after a +/// fork on Unix (though not quite immediately; see documentation of +/// [`ReseedingRng`]). /// Note that the reseeding is done as an extra precaution against side-channel /// attacks and mis-use (e.g. if somehow weak entropy were supplied initially). /// The PRNG algorithms used are assumed to be secure. @@ -55,20 +57,22 @@ const THREAD_RNG_RESEED_THRESHOLD: u64 = 1024 * 64; /// [`ReseedingRng`]: crate::rngs::adapter::ReseedingRng /// [`StdRng`]: crate::rngs::StdRng #[cfg_attr(doc_cfg, doc(cfg(all(feature = "std", feature = "std_rng"))))] -#[derive(Copy, Clone, Debug)] +#[derive(Clone, Debug)] pub struct ThreadRng { - // inner raw pointer implies type is neither Send nor Sync - rng: NonNull>, + // Rc is explictly !Send and !Sync + rng: Rc>>, } thread_local!( - static THREAD_RNG_KEY: UnsafeCell> = { + // We require Rc<..> to avoid premature freeing when thread_rng is used + // within thread-local destructors. See #968. + static THREAD_RNG_KEY: Rc>> = { let r = Core::from_rng(OsRng).unwrap_or_else(|err| panic!("could not initialize thread_rng: {}", err)); let rng = ReseedingRng::new(r, THREAD_RNG_RESEED_THRESHOLD, OsRng); - UnsafeCell::new(rng) + Rc::new(UnsafeCell::new(rng)) } ); @@ -81,9 +85,8 @@ thread_local!( /// For more information see [`ThreadRng`]. #[cfg_attr(doc_cfg, doc(cfg(all(feature = "std", feature = "std_rng"))))] pub fn thread_rng() -> ThreadRng { - let raw = THREAD_RNG_KEY.with(|t| t.get()); - let nn = NonNull::new(raw).unwrap(); - ThreadRng { rng: nn } + let rng = THREAD_RNG_KEY.with(|t| t.clone()); + ThreadRng { rng } } impl Default for ThreadRng { @@ -92,23 +95,36 @@ impl Default for ThreadRng { } } +impl ThreadRng { + #[inline(always)] + fn rng(&mut self) -> &mut ReseedingRng { + unsafe { &mut *self.rng.get() } + } + + // Safe alternative using Rc>: + // #[inline(always)] + // fn rng(&mut self) -> RefMut> { + // self.rng.borrow_mut() + // } +} + impl RngCore for ThreadRng { #[inline(always)] fn next_u32(&mut self) -> u32 { - unsafe { self.rng.as_mut().next_u32() } + self.rng().next_u32() } #[inline(always)] fn next_u64(&mut self) -> u64 { - unsafe { self.rng.as_mut().next_u64() } + self.rng().next_u64() } fn fill_bytes(&mut self, dest: &mut [u8]) { - unsafe { self.rng.as_mut().fill_bytes(dest) } + self.rng().fill_bytes(dest) } fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { - unsafe { self.rng.as_mut().try_fill_bytes(dest) } + self.rng().try_fill_bytes(dest) } } From 62386e41a5f2c328a75ebe1dbb79e9446f52284c Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Tue, 1 Sep 2020 18:10:52 +0100 Subject: [PATCH 4/4] Remove code comment --- src/rngs/thread.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/rngs/thread.rs b/src/rngs/thread.rs index 14be2247068..c0e4726afad 100644 --- a/src/rngs/thread.rs +++ b/src/rngs/thread.rs @@ -100,12 +100,6 @@ impl ThreadRng { fn rng(&mut self) -> &mut ReseedingRng { unsafe { &mut *self.rng.get() } } - - // Safe alternative using Rc>: - // #[inline(always)] - // fn rng(&mut self) -> RefMut> { - // self.rng.borrow_mut() - // } } impl RngCore for ThreadRng {