Skip to content

Commit 17fc129

Browse files
committed
revert unsafe impl linked_list::Link for TimerShared
1 parent 926729b commit 17fc129

4 files changed

Lines changed: 14 additions & 18 deletions

File tree

tokio/src/runtime/time/entry.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -415,16 +415,16 @@ impl TimerShared {
415415
}
416416

417417
unsafe impl linked_list::Link for TimerShared {
418-
type Handle = NonNull<TimerShared>;
418+
type Handle = TimerHandle;
419419

420420
type Target = TimerShared;
421421

422422
fn as_raw(handle: &Self::Handle) -> NonNull<Self::Target> {
423-
*handle
423+
handle.inner
424424
}
425425

426426
unsafe fn from_raw(ptr: NonNull<Self::Target>) -> Self::Handle {
427-
ptr
427+
TimerHandle { inner: ptr }
428428
}
429429

430430
unsafe fn pointers(
@@ -593,10 +593,6 @@ impl TimerHandle {
593593
pub(super) unsafe fn fire(self, completed_state: TimerResult) -> Option<Waker> {
594594
self.inner.as_ref().state.fire(completed_state)
595595
}
596-
597-
pub(super) fn inner(&self) -> NonNull<TimerShared> {
598-
self.inner
599-
}
600596
}
601597

602598
impl Drop for TimerEntry {

tokio/src/runtime/time/mod.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,16 +325,17 @@ impl Handle {
325325
// pinned in memory and is not dropped until the guarded list is dropped.
326326
let guard = TimerShared::new(id);
327327
pin!(guard);
328+
let guard_handle = guard.as_ref().get_ref().handle();
328329

329330
// * This list will be still guarded by the lock of the Wheel with the specefied id.
330331
// `EntryWaitersList` wrapper makes sure we hold the lock to modify it.
331332
// * This wrapper will empty the list on drop. It is critical for safety
332333
// that we will not leave any list entry with a pointer to the local
333334
// guard node after this function returns / panics.
334-
let mut list = lock.get_waiters_list(&expiration, guard.as_ref(), id, self);
335+
// Safety: The `TimerShared` inside this `TimerHandle` is pinned in the memory.
336+
let mut list = unsafe { lock.get_waiters_list(&expiration, guard_handle, id, self) };
335337

336338
while let Some(entry) = list.pop_back_locked(&mut lock) {
337-
let entry = unsafe { entry.as_ref().handle() };
338339
let deadline = expiration.deadline;
339340
// Try to expire the entry; this is cheap (doesn't synchronize) if
340341
// the timer is not expired, and updates cached_when.

tokio/src/runtime/time/wheel/level.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ impl Level {
121121
pub(crate) unsafe fn add_entry(&mut self, item: TimerHandle) {
122122
let slot = slot_for(item.true_when(), self.level);
123123

124-
self.slot[slot].push_front(item.inner());
124+
self.slot[slot].push_front(item);
125125

126126
self.occupied |= occupied_bit(slot);
127127
}

tokio/src/runtime/time/wheel/mod.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ mod level;
66
pub(crate) use self::level::Expiration;
77
use self::level::Level;
88

9-
use std::pin::Pin;
109
use std::{array, ptr::NonNull};
1110

1211
use super::entry::MAX_SAFE_MILLIS_DURATION;
@@ -37,12 +36,11 @@ impl<'a> Drop for EntryWaitersList<'a> {
3736
impl<'a> EntryWaitersList<'a> {
3837
fn new(
3938
unguarded_list: LinkedList<TimerShared, <TimerShared as linked_list::Link>::Target>,
40-
guard: Pin<&'a TimerShared>,
39+
guard_handle: TimerHandle,
4140
wheel_id: u32,
4241
handle: &'a Handle,
4342
) -> Self {
44-
let guard_ptr = NonNull::from(guard.get_ref());
45-
let list = unguarded_list.into_guarded(guard_ptr);
43+
let list = unguarded_list.into_guarded(guard_handle);
4644
Self {
4745
list,
4846
is_empty: false,
@@ -53,7 +51,7 @@ impl<'a> EntryWaitersList<'a> {
5351

5452
/// Removes the last element from the guarded list. Modifying this list
5553
/// requires an exclusive access to the Wheel with the specified `wheel_id`.
56-
pub(super) fn pop_back_locked(&mut self, _wheel: &mut Wheel) -> Option<NonNull<TimerShared>> {
54+
pub(super) fn pop_back_locked(&mut self, _wheel: &mut Wheel) -> Option<TimerHandle> {
5755
let result = self.list.pop_back();
5856
if result.is_none() {
5957
// Save information about emptiness to avoid waiting for lock
@@ -263,10 +261,11 @@ impl Wheel {
263261
}
264262

265263
/// Obtains the guarded list of entries that need processing for the given expiration.
266-
pub(super) fn get_waiters_list<'a>(
264+
/// Safety: The `TimerShared` inside `guard_handle` must be pinned in the memory.
265+
pub(super) unsafe fn get_waiters_list<'a>(
267266
&mut self,
268267
expiration: &Expiration,
269-
guard: Pin<&'a TimerShared>,
268+
guard_handle: TimerHandle,
270269
wheel_id: u32,
271270
handle: &'a Handle,
272271
) -> EntryWaitersList<'a> {
@@ -281,7 +280,7 @@ impl Wheel {
281280
// back into the same position; we must make sure we don't then process
282281
// those entries again or we'll end up in an infinite loop.
283282
let unguarded_list = self.levels[expiration.level].take_slot(expiration.slot);
284-
EntryWaitersList::new(unguarded_list, guard, wheel_id, handle)
283+
EntryWaitersList::new(unguarded_list, guard_handle, wheel_id, handle)
285284
}
286285

287286
pub(super) fn occupied_bit_maintain(&mut self, expiration: &Expiration) {

0 commit comments

Comments
 (0)