Skip to content

Commit

Permalink
time: Remove HandlePriv (#1896)
Browse files Browse the repository at this point in the history
## Motivation

#1800 removed the lazy binding of `Delay`s to timers. With the removal of the
logic required for that, `HandlePriv` is no longer needed. This PR removes the
use of `HandlePriv`.

A `TODO` was also removed that would panic if when registering a new `Delay`
the current timer handle was full. That has been fixed to now immediately
transition that `Delay` to an error state that can be handled in a similar way
to other error states.

Signed-off-by: Kevin Leimkuhler <[email protected]>
  • Loading branch information
kleimkuhler authored Dec 5, 2019
1 parent 0e729aa commit dbcd1f9
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 63 deletions.
56 changes: 29 additions & 27 deletions tokio/src/time/driver/entry.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::loom::sync::atomic::AtomicU64;
use crate::sync::AtomicWaker;
use crate::time::driver::{HandlePriv, Inner};
use crate::time::driver::{Handle, Inner};
use crate::time::{Duration, Error, Instant};

use std::cell::UnsafeCell;
Expand Down Expand Up @@ -105,36 +105,24 @@ const ERROR: u64 = u64::MAX;

impl Entry {
pub(crate) fn new(deadline: Instant, duration: Duration) -> Arc<Entry> {
let handle_priv = HandlePriv::current();
let handle = handle_priv.inner().unwrap();
let inner = Handle::current().inner().unwrap();
let entry: Entry;

// Increment the number of active timeouts
if handle.increment().is_err() {
// TODO(kleimkuhler): Transition to error state instead of
// panicking?
panic!("failed to add entry; timer at capacity");
};

let when = handle.normalize_deadline(deadline);
let state = if when <= handle.elapsed() {
ELAPSED
if inner.increment().is_err() {
entry = Entry::new2(deadline, duration, Weak::new(), ERROR)
} else {
when
};

let entry = Arc::new(Entry {
time: CachePadded(UnsafeCell::new(Time { deadline, duration })),
inner: handle_priv.into_inner(),
waker: AtomicWaker::new(),
state: AtomicU64::new(state),
queued: AtomicBool::new(false),
next_atomic: UnsafeCell::new(ptr::null_mut()),
when: UnsafeCell::new(None),
next_stack: UnsafeCell::new(None),
prev_stack: UnsafeCell::new(ptr::null_mut()),
});
let when = inner.normalize_deadline(deadline);
let state = if when <= inner.elapsed() {
ELAPSED
} else {
when
};
entry = Entry::new2(deadline, duration, Arc::downgrade(&inner), state);
}

if handle.queue(&entry).is_err() {
let entry = Arc::new(entry);
if inner.queue(&entry).is_err() {
entry.error();
}

Expand Down Expand Up @@ -314,6 +302,20 @@ impl Entry {
}
}

fn new2(deadline: Instant, duration: Duration, inner: Weak<Inner>, state: u64) -> Self {
Self {
time: CachePadded(UnsafeCell::new(Time { deadline, duration })),
inner,
waker: AtomicWaker::new(),
state: AtomicU64::new(state),
queued: AtomicBool::new(false),
next_atomic: UnsafeCell::new(ptr::null_mut()),
when: UnsafeCell::new(None),
next_stack: UnsafeCell::new(None),
prev_stack: UnsafeCell::new(ptr::null_mut()),
}
}

fn upgrade_inner(&self) -> Option<Arc<Inner>> {
self.inner.upgrade()
}
Expand Down
43 changes: 8 additions & 35 deletions tokio/src/time/driver/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,20 @@ use std::marker::PhantomData;
use std::sync::{Arc, Weak};

/// Handle to time driver instance.
#[derive(Debug, Clone)]
pub(crate) struct Handle {
inner: Option<HandlePriv>,
}

/// Like `Handle` but never `None`.
#[derive(Clone)]
pub(crate) struct HandlePriv {
pub(crate) struct Handle {
inner: Weak<Inner>,
}

thread_local! {
/// Tracks the timer for the current execution context.
static CURRENT_TIMER: RefCell<Option<HandlePriv>> = RefCell::new(None)
static CURRENT_TIMER: RefCell<Option<Handle>> = RefCell::new(None)
}

#[derive(Debug)]
/// Guard that unsets the current default timer on drop.
pub(crate) struct DefaultGuard<'a> {
prev: Option<HandlePriv>,
prev: Option<Handle>,
_lifetime: PhantomData<&'a u8>,
}

Expand All @@ -47,10 +41,6 @@ pub(crate) fn set_default(handle: &Handle) -> DefaultGuard<'_> {
let mut current = current.borrow_mut();
let prev = current.take();

let handle = handle
.as_priv()
.unwrap_or_else(|| panic!("`handle` does not reference a timer"));

*current = Some(handle.clone());

DefaultGuard {
Expand All @@ -61,23 +51,11 @@ pub(crate) fn set_default(handle: &Handle) -> DefaultGuard<'_> {
}

impl Handle {
pub(crate) fn new(inner: Weak<Inner>) -> Handle {
let inner = HandlePriv { inner };
Handle { inner: Some(inner) }
/// Create a new timer `Handle` from a shared `Inner` timer state.
pub(crate) fn new(inner: Weak<Inner>) -> Self {
Handle { inner }
}

fn as_priv(&self) -> Option<&HandlePriv> {
self.inner.as_ref()
}
}

impl Default for Handle {
fn default() -> Handle {
Handle { inner: None }
}
}

impl HandlePriv {
/// Try to get a handle to the current timer.
///
/// # Panics
Expand All @@ -94,15 +72,10 @@ impl HandlePriv {
pub(crate) fn inner(&self) -> Option<Arc<Inner>> {
self.inner.upgrade()
}

/// Consume the handle, returning the weak Inner ref.
pub(crate) fn into_inner(self) -> Weak<Inner> {
self.inner
}
}

impl fmt::Debug for HandlePriv {
impl fmt::Debug for Handle {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "HandlePriv")
write!(f, "Handle")
}
}
2 changes: 1 addition & 1 deletion tokio/src/time/driver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod entry;
use self::entry::Entry;

mod handle;
pub(crate) use self::handle::{set_default, Handle, HandlePriv};
pub(crate) use self::handle::{set_default, Handle};

mod registration;
pub(crate) use self::registration::Registration;
Expand Down

0 comments on commit dbcd1f9

Please sign in to comment.