From 8ce871b58a5bf7af5b51190980be4c2106dc7d04 Mon Sep 17 00:00:00 2001
From: Paul Dicker <pitdicker@gmail.com>
Date: Sat, 17 Mar 2018 17:40:05 +0100
Subject: [PATCH] Use UnsafeCell in ThreadRng

---
 src/thread_rng.rs | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/thread_rng.rs b/src/thread_rng.rs
index 9217d7b8851..fb15f07118f 100644
--- a/src/thread_rng.rs
+++ b/src/thread_rng.rs
@@ -10,7 +10,7 @@
 
 //! Thread-local random number generator
 
-use std::cell::RefCell;
+use std::cell::UnsafeCell;
 use std::rc::Rc;
 
 use {RngCore, CryptoRng, SeedableRng, EntropyRng};
@@ -18,6 +18,24 @@ use prng::hc128::Hc128Core;
 use {Distribution, Uniform, Rng, Error};
 use reseeding::ReseedingRng;
 
+// Rationale for using `UnsafeCell` in `ThreadRng`:
+//
+// Previously we used a `RefCell`, with an overhead of ~15%. There will only
+// ever be one mutable reference to the interior of the `UnsafeCell`, because
+// we only have such a reference inside `next_u32`, `next_u64`, etc. Within a
+// single thread (which is the definition of `ThreadRng`), there will only ever
+// be one of these methods active at a time.
+//
+// A possible scenario where there could be multiple mutable references is if
+// `ThreadRng` is used inside `next_u32` and co. But the implementation is
+// completely under our control. We just have to ensure none of them use
+// `ThreadRng` internally, which is nonsensical anyway. We should also never run
+// `ThreadRng` in destructors of its implementation, which is also nonsensical.
+//
+// The additional `Rc` is not strictly neccesary, and could be removed. For now
+// it ensures `ThreadRng` stays `!Send` and `!Sync`, and implements `Clone`.
+
+
 // Number of generated bytes after which to reseed `TreadRng`.
 //
 // The time it takes to reseed HC-128 is roughly equivalent to generating 7 KiB.
@@ -32,18 +50,18 @@ const THREAD_RNG_RESEED_THRESHOLD: u64 = 32*1024*1024; // 32 MiB
 /// [`thread_rng`]: fn.thread_rng.html
 #[derive(Clone, Debug)]
 pub struct ThreadRng {
-    rng: Rc<RefCell<ReseedingRng<Hc128Core, EntropyRng>>>,
+    rng: Rc<UnsafeCell<ReseedingRng<Hc128Core, EntropyRng>>>,
 }
 
 thread_local!(
-    static THREAD_RNG_KEY: Rc<RefCell<ReseedingRng<Hc128Core, EntropyRng>>> = {
+    static THREAD_RNG_KEY: Rc<UnsafeCell<ReseedingRng<Hc128Core, EntropyRng>>> = {
         let mut entropy_source = EntropyRng::new();
         let r = Hc128Core::from_rng(&mut entropy_source).unwrap_or_else(|err|
                 panic!("could not initialize thread_rng: {}", err));
         let rng = ReseedingRng::new(r,
                                     THREAD_RNG_RESEED_THRESHOLD,
                                     entropy_source);
-        Rc::new(RefCell::new(rng))
+        Rc::new(UnsafeCell::new(rng))
     }
 );
 
@@ -80,20 +98,20 @@ pub fn thread_rng() -> ThreadRng {
 impl RngCore for ThreadRng {
     #[inline(always)]
     fn next_u32(&mut self) -> u32 {
-        self.rng.borrow_mut().next_u32()
+        unsafe { (*self.rng.get()).next_u32() }
     }
 
     #[inline(always)]
     fn next_u64(&mut self) -> u64 {
-        self.rng.borrow_mut().next_u64()
+        unsafe { (*self.rng.get()).next_u64() }
     }
 
     fn fill_bytes(&mut self, bytes: &mut [u8]) {
-        self.rng.borrow_mut().fill_bytes(bytes)
+        unsafe { (*self.rng.get()).fill_bytes(bytes) }
     }
 
     fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> {
-        self.rng.borrow_mut().try_fill_bytes(dest)
+        unsafe { (*self.rng.get()).try_fill_bytes(dest) }
     }
 }