rtic-sync: add some newtypes & better comments

Author: datdenkikniet

rtic-sync/src/channel/channel.rs

@@ -18,23 +18,29 @@ use super::{Receiver, Sender};
 pub(crate) type WaitQueueData = (Waker, FreeSlotPtr);
 pub(crate) type WaitQueue = DoublyLinkedList<WaitQueueData>;
 
+#[derive(Debug)]
+pub(crate) struct FreeSlot(u8);
+
 /// A pointer to a free slot.
 #[derive(Clone)]
-pub(crate) struct FreeSlotPtr(*mut Option<u8>);
+pub(crate) struct FreeSlotPtr(*mut Option<FreeSlot>);
 
 impl FreeSlotPtr {
     /// SAFETY: `inner` must be valid until the [`Link`] containing this [`FreeSlotPtr`] is popped.
     /// Additionally, this [`FreeSlotPtr`] must have exclusive access to the data pointed to by
     /// `inner`.
-    pub unsafe fn new(inner: *mut Option<u8>) -> Self {
+    pub unsafe fn new(inner: *mut Option<FreeSlot>) -> Self {
         Self(inner)
     }
 
     /// Replace the value of this slot with `new_value`, and return
     /// the old value.
     ///
     /// SAFETY: the pointer in this [`FreeSlotPtr`] must be valid for writes.
-    pub(crate) unsafe fn take(&mut self, cs: critical_section::CriticalSection) -> Option<u8> {
+    pub(crate) unsafe fn take(
+        &mut self,
+        cs: critical_section::CriticalSection,
+    ) -> Option<FreeSlot> {
         self.replace(None, cs)
     }
 
@@ -45,9 +51,9 @@ impl FreeSlotPtr {
     /// be obtained from `freeq`.
     unsafe fn replace(
         &mut self,
-        new_value: Option<u8>,
+        new_value: Option<FreeSlot>,
         _cs: critical_section::CriticalSection,
-    ) -> Option<u8> {
+    ) -> Option<FreeSlot> {
         // SAFETY: we are in a critical section.
         unsafe { core::ptr::replace(self.0, new_value) }
     }
@@ -147,31 +153,32 @@ impl<T, const N: usize> Channel<T, N> {
     ///
     /// This will do one of two things:
     /// 1. If there are any waiting `send`-ers, wake the longest-waiting one and hand it `slot`.
-    /// 2. else, insert `slot` into `self.freeq`.
+    /// 2. else, insert `slot` into the free queue.
     ///
-    /// SAFETY: `slot` must be a `u8` that is obtained from [`Channel::readyq`] or through [`FreeSlotPtr::take`].
-    pub(crate) unsafe fn push_free_slot(&self, slot: u8) {
+    /// SAFETY: `slot` must be obtained from this exact channel instance.
+    pub(crate) unsafe fn return_free_slot(&self, slot: FreeSlot) {
         critical_section::with(|cs| {
             fence(Ordering::SeqCst);
 
             // If a sender is waiting in the `wait_queue`, wake the first one up & hand it the free slot.
             if let Some((wait_head, mut freeq_slot)) = self.wait_queue.pop() {
                 // SAFETY: `freeq_slot` is valid for writes: we are in a critical
-                // section & the `SlotPtr` lives for at least the duration of the wait queue link.
+                // section & the `FreeSlotPtr` lives for at least the duration of the wait queue link.
                 unsafe { freeq_slot.replace(Some(slot), cs) };
                 wait_head.wake();
             } else {
                 assert!(!self.access(cs).freeq.is_full());
-                unsafe { self.access(cs).freeq.push_back_unchecked(slot) }
+                unsafe { self.access(cs).freeq.push_back_unchecked(slot.0) }
             }
         });
     }
 
-    /// Push a value into a specific slot.
+    /// Send a value using the given `slot` in this channel.
     ///
-    /// SAFETY: `slot` must be obtained from [`Channel::pop_free_slot`] or through a popped [`SlotPtr`].
+    /// SAFETY: `slot` must be obtained from this exact channel instance.
     #[inline(always)]
-    pub(crate) unsafe fn slot_ready(&self, slot: u8, val: T) {
+    pub(crate) unsafe fn send_value(&self, slot: FreeSlot, val: T) {
+        let slot = slot.0;
         // Write the value to the slots, note; this memcpy is not under a critical section.
         unsafe { ptr::write(self.slots.get_unchecked(slot as usize).get() as *mut T, val) }
 
@@ -187,16 +194,21 @@ impl<T, const N: usize> Channel<T, N> {
         self.receiver_waker.wake();
     }
 
-    /// Pop a ready slot.
-    pub(crate) fn pop_ready_slot(&self) -> Option<T> {
+    /// Pop the value of a ready slot to make it available to a receiver.
+    ///
+    /// Internally, this function does these things:
+    /// 1. Pop a ready slot from the ready queue.
+    /// 2. If available, read the data from the backing slot storage.
+    /// 3. If available, return the now-free slot to the free queue.
+    pub(crate) fn receive_value(&self) -> Option<T> {
         let ready_slot = critical_section::with(|cs| self.access(cs).readyq.pop_front());
 
         if let Some(rs) = ready_slot {
             let r = unsafe { ptr::read(self.slots.get_unchecked(rs as usize).get() as *const T) };
 
             // Return the index to the free queue after we've read the value.
-            // SAFETY: `rs` comes directly from `readyq`.
-            unsafe { self.push_free_slot(rs) };
+            // SAFETY: `rs` is now a free slot obtained from this channel.
+            unsafe { self.return_free_slot(FreeSlot(rs)) };
 
             Some(r)
         } else {
@@ -207,7 +219,7 @@ impl<T, const N: usize> Channel<T, N> {
     /// Register a new waiter in the wait queue.
     ///
     /// SAFETY: `link` must be valid until it is popped.
-    pub(crate) unsafe fn push_to_wait_queue(&self, link: Pin<&Link<WaitQueueData>>) {
+    pub(crate) unsafe fn push_wait_queue(&self, link: Pin<&Link<WaitQueueData>>) {
         self.wait_queue.push(link);
     }
 
@@ -216,8 +228,9 @@ impl<T, const N: usize> Channel<T, N> {
     }
 
     /// Pop a free slot.
-    pub(crate) fn pop_free_slot(&self) -> Option<u8> {
-        critical_section::with(|cs| self.access(cs).freeq.pop_front())
+    pub(crate) fn pop_free_slot(&self) -> Option<FreeSlot> {
+        let slot = critical_section::with(|cs| self.access(cs).freeq.pop_front());
+        slot.map(FreeSlot)
     }
 
     pub(crate) fn num_senders(&self) -> usize {

rtic-sync/src/channel/receiver.rs

@@ -37,7 +37,7 @@ impl<T, const N: usize> Receiver<'_, T, N> {
     /// Receives a value if there is one in the channel, non-blocking.
     pub fn try_recv(&mut self) -> Result<T, ReceiveError> {
         // Try to get a ready slot.
-        let ready_slot = self.0.pop_ready_slot();
+        let ready_slot = self.0.receive_value();
 
         if let Some(value) = ready_slot {
             Ok(value)

rtic-sync/src/channel/sender.rs

@@ -3,7 +3,7 @@ use core::{future::poll_fn, pin::Pin, task::Poll};
 use rtic_common::{dropper::OnDrop, wait_queue::Link};
 
 use super::{
-    channel::{FreeSlotPtr, WaitQueueData},
+    channel::{FreeSlot, FreeSlotPtr, WaitQueueData},
     Channel,
 };
 
@@ -113,15 +113,15 @@ impl<T, const N: usize> Sender<'_, T, N> {
             return Err(TrySendError::Full(val));
         };
 
-        unsafe { self.0.slot_ready(idx, val) };
+        unsafe { self.0.send_value(idx, val) };
 
         Ok(())
     }
 
     /// Send a value. If there is no place left in the queue this will wait until there is.
     /// If the receiver does not exist this will return an error.
     pub async fn send(&mut self, val: T) -> Result<(), NoReceiver<T>> {
-        let mut free_slot_ptr: Option<u8> = None;
+        let mut free_slot_ptr: Option<FreeSlot> = None;
         let mut link_ptr: Option<Link<WaitQueueData>> = None;
 
         // Make this future `Drop`-safe.
@@ -146,8 +146,8 @@ impl<T, const N: usize> Sender<'_, T, N> {
             // about this a bit more...
             critical_section::with(|cs| {
                 if let Some(freed_slot) = unsafe { free_slot_ptr2.take(cs) } {
-                    // SAFETY: `freed_slot` is obtained through `FreeSlotPtr::take`.
-                    unsafe { self.0.push_free_slot(freed_slot) };
+                    // SAFETY: `freed_slot` is a free slot in our referenced channel.
+                    unsafe { self.0.return_free_slot(freed_slot) };
                 }
             });
         });
@@ -197,7 +197,7 @@ impl<T, const N: usize> Sender<'_, T, N> {
                     // are shadowed and we make sure in `dropper` that the link is removed from the queue
                     // before dropping `link_ptr` AND `dropper` makes sure that the shadowed
                     // `ptr`s live until the end of the stack frame.
-                    unsafe { self.0.push_to_wait_queue(Pin::new_unchecked(link_ref)) };
+                    unsafe { self.0.push_wait_queue(Pin::new_unchecked(link_ref)) };
 
                     Poll::Pending
                 }
@@ -210,7 +210,7 @@ impl<T, const N: usize> Sender<'_, T, N> {
 
         if let Ok(slot) = idx {
             // SAFETY: `slot` is provided through a `SlotPtr` or comes from `pop_free_slot`.
-            unsafe { self.0.slot_ready(slot, val) };
+            unsafe { self.0.send_value(slot, val) };
 
             Ok(())
         } else {