1
0
mirror of https://github.com/fafhrd91/actix-web synced 2024-11-27 17:52:56 +01:00

Replace UnsafeCell with Cell in DateServiceInner (#1325)

* 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
measurable.

Since the wrapped type is `Copy`, a `Cell` can be used, avoiding the
runtime overhead of a `RefCell`.

Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
This commit is contained in:
Aaron Hill 2020-01-29 07:05:08 -05:00 committed by GitHub
parent 664f9a8b2d
commit 276a5a3ee4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -1,4 +1,4 @@
use std::cell::UnsafeCell; use std::cell::Cell;
use std::fmt::Write; use std::fmt::Write;
use std::rc::Rc; use std::rc::Rc;
use std::time::Duration; use std::time::Duration;
@ -228,24 +228,24 @@ impl fmt::Write for Date {
struct DateService(Rc<DateServiceInner>); struct DateService(Rc<DateServiceInner>);
struct DateServiceInner { struct DateServiceInner {
current: UnsafeCell<Option<(Date, Instant)>>, current: Cell<Option<(Date, Instant)>>,
} }
impl DateServiceInner { impl DateServiceInner {
fn new() -> Self { fn new() -> Self {
DateServiceInner { DateServiceInner {
current: UnsafeCell::new(None), current: Cell::new(None),
} }
} }
fn reset(&self) { fn reset(&self) {
unsafe { (&mut *self.current.get()).take() }; self.current.take();
} }
fn update(&self) { fn update(&self) {
let now = Instant::now(); let now = Instant::now();
let date = Date::new(); 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) { fn check_date(&self) {
if unsafe { (&*self.0.current.get()).is_none() } { if self.0.current.get().is_none() {
self.0.update(); self.0.update();
// periodic date update // periodic date update
@ -269,12 +269,12 @@ impl DateService {
fn now(&self) -> Instant { fn now(&self) -> Instant {
self.check_date(); self.check_date();
unsafe { (&*self.0.current.get()).as_ref().unwrap().1 } self.0.current.get().unwrap().1
} }
fn set_date<F: FnMut(&Date)>(&self, mut f: F) { fn set_date<F: FnMut(&Date)>(&self, mut f: F) {
self.check_date(); self.check_date();
f(&unsafe { (&*self.0.current.get()).as_ref().unwrap().0 }) f(&self.0.current.get().unwrap().0);
} }
} }
@ -282,6 +282,19 @@ impl DateService {
mod tests { mod tests {
use super::*; use super::*;
// Test modifying the date from within the closure
// passed to `set_date`
#[test]
fn test_evil_date() {
let service = DateService::new();
// Make sure that `check_date` doesn't try to spawn a task
service.0.update();
service.set_date(|_| {
service.0.reset()
});
}
#[test] #[test]
fn test_date_len() { fn test_date_len() {
assert_eq!(DATE_VALUE_LENGTH, "Sun, 06 Nov 1994 08:49:37 GMT".len()); assert_eq!(DATE_VALUE_LENGTH, "Sun, 06 Nov 1994 08:49:37 GMT".len());