From c18ea4c4f055988b79ee11cc281b5cd440392232 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 27 Jan 2020 18:24:55 -0500 Subject: [PATCH] Replace `UnsafeCell` with `Cell` in `DateServiceInner` This ensures that it's impossible to cause undefined behavior by accidentally violating Rust's aliasing rules (e.g. passing a closure to `set_date` which ends up invoking `reset` or `update` on the inner `DateServiceInner`). There might be a tiny amount of overhead from copying the `Option<(Date, Instant)>` rather than taking a reference, but it shouldn't be measureable. Since the wrapped type is `Copy`, a `Cell` can be used, avoiding the runtime overhead of a `RefCell`. --- actix-http/src/config.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/actix-http/src/config.rs b/actix-http/src/config.rs index be949aae..f2bd8ecb 100644 --- a/actix-http/src/config.rs +++ b/actix-http/src/config.rs @@ -1,4 +1,4 @@ -use std::cell::UnsafeCell; +use std::cell::Cell; use std::fmt::Write; use std::rc::Rc; use std::time::Duration; @@ -228,24 +228,24 @@ impl fmt::Write for Date { struct DateService(Rc); struct DateServiceInner { - current: UnsafeCell>, + current: Cell>, } impl DateServiceInner { fn new() -> Self { DateServiceInner { - current: UnsafeCell::new(None), + current: Cell::new(None), } } fn reset(&self) { - unsafe { (&mut *self.current.get()).take() }; + self.current.take(); } fn update(&self) { let now = Instant::now(); let date = Date::new(); - *(unsafe { &mut *self.current.get() }) = Some((date, now)); + self.current.set(Some((date, now))); } } @@ -255,7 +255,7 @@ impl DateService { } fn check_date(&self) { - if unsafe { (&*self.0.current.get()).is_none() } { + if self.0.current.get().is_none() { self.0.update(); // periodic date update @@ -269,12 +269,12 @@ impl DateService { fn now(&self) -> Instant { self.check_date(); - unsafe { (&*self.0.current.get()).as_ref().unwrap().1 } + self.0.current.get().unwrap().1 } fn set_date(&self, mut f: F) { self.check_date(); - f(&unsafe { (&*self.0.current.get()).as_ref().unwrap().0 }) + f(&self.0.current.get().unwrap().0); } }