Skip to content

Commit

Permalink
use checked indexing
Browse files Browse the repository at this point in the history
  • Loading branch information
camshaft committed Dec 11, 2024
1 parent 8e720c1 commit 2b3b9c1
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 56 deletions.
31 changes: 8 additions & 23 deletions dc/s2n-quic-dc/src/stream/server/tokio/tcp/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,7 @@ where
let capacity = workers.len();
let mut free = List::default();
for idx in 0..capacity {
unsafe {
// SAFETY: idx is in bounds
free.push(&mut workers, idx);
}
free.push(&mut workers, idx);
}

let by_sojourn_time = List::default();
Expand Down Expand Up @@ -203,12 +200,9 @@ where
clock,
);

unsafe {
// SAFETY: the idx is in bounds
self.inner
.by_sojourn_time
.push(&mut self.inner.workers, idx);
}
self.inner
.by_sojourn_time
.push(&mut self.inner.workers, idx);

// kick off the initial poll to register wakers with the socket
self.inner.poll_worker(idx, cx, publisher, clock);
Expand Down Expand Up @@ -296,11 +290,8 @@ where
}

// the worker is all done so indicate we have another free slot
unsafe {
// SAFETY: list entries are managed by the list impl; idx is in bounds
self.by_sojourn_time.remove(&mut self.workers, idx);
self.free.push(&mut self.workers, idx);
}
self.by_sojourn_time.remove(&mut self.workers, idx);
self.free.push(&mut self.workers, idx);

cf
}
Expand All @@ -311,10 +302,7 @@ where
C: Clock,
{
// if we have a free worker then use that
if let Some(idx) = unsafe {
// SAFETY: free list manages `workers` linked status
self.free.pop(&mut self.workers)
} {
if let Some(idx) = self.free.pop(&mut self.workers) {
trace!(op = %"next_worker", free = idx);
return Some(idx);
}
Expand All @@ -325,10 +313,7 @@ where
// if the worker's sojourn time exceeds the maximum, then reclaim it
if sojourn >= self.max_sojourn_time() {
trace!(op = %"next_worker", injected = idx, ?sojourn);
return unsafe {
// SAFETY: by_sojourn_time list manages `workers` linked status
self.by_sojourn_time.pop(&mut self.workers)
};
return self.by_sojourn_time.pop(&mut self.workers);
}

trace!(op = %"next_worker", ?sojourn, max_sojourn_time = ?self.max_sojourn_time());
Expand Down
49 changes: 16 additions & 33 deletions dc/s2n-quic-dc/src/stream/server/tokio/tcp/manager/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,8 @@ impl List {
self.len == 0
}

/// # Safety
///
/// Callers must ensure:
///
/// * `entries` is only managed by [`List`]
/// * `idx` is less than `usize::MAX`
#[inline]
pub unsafe fn pop<L>(&mut self, entries: &mut [L]) -> Option<usize>
pub fn pop<L>(&mut self, entries: &mut [L]) -> Option<usize>
where
L: AsMut<Link>,
{
Expand All @@ -61,14 +55,14 @@ impl List {
}

let idx = self.head;
let link = entries.get_unchecked_mut(idx).as_mut();
let link = entries[idx].as_mut();
self.head = link.next;
link.reset();

if self.head == usize::MAX {
self.tail = usize::MAX;
} else {
entries.get_unchecked_mut(self.head).as_mut().prev = usize::MAX;
entries[self.head].as_mut().prev = usize::MAX;
}

self.set_linked_status(idx, false);
Expand All @@ -85,62 +79,51 @@ impl List {
}
}

/// # Safety
///
/// Callers must ensure:
///
/// * `entries` is only managed by [`List`]
/// * `idx` is in bounds of `entries`
/// * `idx` is less than `usize::MAX`
#[inline]
pub unsafe fn push<L>(&mut self, entries: &mut [L], idx: usize)
pub fn push<L>(&mut self, entries: &mut [L], idx: usize)
where
L: AsMut<Link>,
{
debug_assert!(idx < usize::MAX);

let tail = self.tail;
if tail != usize::MAX {
entries.get_unchecked_mut(tail).as_mut().next = idx;
entries[tail].as_mut().next = idx;
} else {
debug_assert!(self.is_empty());
self.head = idx;
}
self.tail = idx;

let link = entries.get_unchecked_mut(idx).as_mut();
let link = entries[idx].as_mut();
link.prev = tail;
link.next = usize::MAX;

self.set_linked_status(idx, true);
}

/// # Safety
///
/// Callers must ensure:
///
/// * `entries` is only managed by [`List`]
/// * `idx` is in bounds of `entries`
/// * `idx` must be less that `usize::MAX`
#[inline]
pub unsafe fn remove<L>(&mut self, entries: &mut [L], idx: usize)
pub fn remove<L>(&mut self, entries: &mut [L], idx: usize)
where
L: AsMut<Link>,
{
debug_assert!(!self.is_empty());
debug_assert!(idx < usize::MAX);

let link = entries.get_unchecked_mut(idx).as_mut();
let link = entries[idx].as_mut();
let next = link.next;
let prev = link.prev;
link.reset();

if prev != usize::MAX {
entries.get_unchecked_mut(prev).as_mut().next = next;
entries[prev].as_mut().next = next;
} else {
debug_assert!(self.head == idx);
self.head = next;
}

if next != usize::MAX {
entries.get_unchecked_mut(next).as_mut().prev = prev;
entries[next].as_mut().prev = prev;
} else {
debug_assert!(self.tail == idx);
self.tail = prev;
Expand Down Expand Up @@ -254,22 +237,22 @@ mod tests {
impl CheckedList {
#[inline]
fn pop(&mut self, entries: &mut [Link]) -> Option<usize> {
let v = unsafe { self.list.pop(entries) };
let v = self.list.pop(entries);
assert_eq!(v, self.oracle.pop_front());
self.invariants(entries);
v
}

#[inline]
fn push(&mut self, entries: &mut [Link], v: usize) {
unsafe { self.list.push(entries, v) };
self.list.push(entries, v);
self.oracle.push_back(v);
self.invariants(entries);
}

#[inline]
fn remove(&mut self, entries: &mut [Link], v: usize) {
unsafe { self.list.remove(entries, v) };
self.list.remove(entries, v);
let idx = self.oracle.iter().position(|&x| x == v).unwrap();
self.oracle.remove(idx);
self.invariants(entries);
Expand Down

0 comments on commit 2b3b9c1

Please sign in to comment.