time: eliminate UnsafeCell around the TimerShared (#7329)

This commit is contained in:
Qi
2025-05-23 19:24:29 +02:00
committed by GitHub
parent 55e3ed2a39
commit 7ec77a0677
+34 -24
View File
@@ -63,7 +63,6 @@ use crate::sync::AtomicWaker;
use crate::time::Instant; use crate::time::Instant;
use crate::util::linked_list; use crate::util::linked_list;
use std::cell::UnsafeCell as StdUnsafeCell;
use std::task::{Context, Poll, Waker}; use std::task::{Context, Poll, Waker};
use std::{marker::PhantomPinned, pin::Pin, ptr::NonNull}; use std::{marker::PhantomPinned, pin::Pin, ptr::NonNull};
@@ -291,9 +290,8 @@ pub(crate) struct TimerEntry {
/// therefore other references can exist to it while mutable references to /// therefore other references can exist to it while mutable references to
/// Entry exist. /// Entry exist.
/// ///
/// This is manipulated only under the inner mutex. TODO: Can we use loom /// This is manipulated only under the inner mutex.
/// cells for this? inner: Option<TimerShared>,
inner: StdUnsafeCell<Option<TimerShared>>,
/// Deadline for the timer. This is used to register on the first /// Deadline for the timer. This is used to register on the first
/// poll, as we can't register prior to being pinned. /// poll, as we can't register prior to being pinned.
deadline: Instant, deadline: Instant,
@@ -479,26 +477,22 @@ impl TimerEntry {
Self { Self {
driver: handle, driver: handle,
inner: StdUnsafeCell::new(None), inner: None,
deadline, deadline,
registered: false, registered: false,
_m: std::marker::PhantomPinned, _m: std::marker::PhantomPinned,
} }
} }
fn is_inner_init(&self) -> bool { fn inner(&self) -> Option<&TimerShared> {
unsafe { &*self.inner.get() }.is_some() self.inner.as_ref()
} }
// This lazy initialization is for performance purposes. fn init_inner(&mut self) {
fn inner(&self) -> &TimerShared { match self.inner {
let inner = unsafe { &*self.inner.get() }; Some(_) => {}
if inner.is_none() { None => self.inner = Some(TimerShared::new()),
unsafe {
*self.inner.get() = Some(TimerShared::new());
}
} }
return inner.as_ref().unwrap();
} }
pub(crate) fn deadline(&self) -> Instant { pub(crate) fn deadline(&self) -> Instant {
@@ -506,15 +500,20 @@ impl TimerEntry {
} }
pub(crate) fn is_elapsed(&self) -> bool { pub(crate) fn is_elapsed(&self) -> bool {
self.is_inner_init() && !self.inner().state.might_be_registered() && self.registered let Some(inner) = self.inner() else {
return false;
};
!inner.state.might_be_registered() && self.registered
} }
/// Cancels and deregisters the timer. This operation is irreversible. /// Cancels and deregisters the timer. This operation is irreversible.
pub(crate) fn cancel(self: Pin<&mut Self>) { pub(crate) fn cancel(self: Pin<&mut Self>) {
// Avoid calling the `clear_entry` method, because it has not been initialized yet. // Avoid calling the `clear_entry` method, because it has not been initialized yet.
if !self.is_inner_init() { let Some(inner) = self.inner() else {
return; return;
} };
// We need to perform an acq/rel fence with the driver thread, and the // We need to perform an acq/rel fence with the driver thread, and the
// simplest way to do so is to grab the driver lock. // simplest way to do so is to grab the driver lock.
// //
@@ -537,7 +536,7 @@ impl TimerEntry {
// driver did so far and happens-before everything the driver does in // driver did so far and happens-before everything the driver does in
// the future. While we have the lock held, we also go ahead and // the future. While we have the lock held, we also go ahead and
// deregister the entry if necessary. // deregister the entry if necessary.
unsafe { self.driver().clear_entry(NonNull::from(self.inner())) }; unsafe { self.driver().clear_entry(NonNull::from(inner)) };
} }
pub(crate) fn reset(mut self: Pin<&mut Self>, new_time: Instant, reregister: bool) { pub(crate) fn reset(mut self: Pin<&mut Self>, new_time: Instant, reregister: bool) {
@@ -545,16 +544,24 @@ impl TimerEntry {
this.deadline = new_time; this.deadline = new_time;
this.registered = reregister; this.registered = reregister;
let tick = self.driver().time_source().deadline_to_tick(new_time); let tick = this.driver().time_source().deadline_to_tick(new_time);
let inner = match this.inner() {
Some(inner) => inner,
None => {
this.init_inner();
this.inner()
.expect("inner should already be initialized by `this.init_inner()`")
}
};
if self.inner().extend_expiration(tick).is_ok() { if inner.extend_expiration(tick).is_ok() {
return; return;
} }
if reregister { if reregister {
unsafe { unsafe {
self.driver() this.driver()
.reregister(&self.driver.driver().io, tick, self.inner().into()); .reregister(&this.driver.driver().io, tick, inner.into());
} }
} }
} }
@@ -574,7 +581,10 @@ impl TimerEntry {
self.as_mut().reset(deadline, true); self.as_mut().reset(deadline, true);
} }
self.inner().state.poll(cx.waker()) let inner = self
.inner()
.expect("inner should already be initialized by `self.reset()`");
inner.state.poll(cx.waker())
} }
pub(crate) fn driver(&self) -> &super::Handle { pub(crate) fn driver(&self) -> &super::Handle {