rtic-sync: No need for c-s to check if link is in wait queue if link is popped

rtic-sync/CHANGELOG.md

@@ -7,8 +7,9 @@ For each category, _Added_, _Changed_, _Fixed_ add new entries at the top!
 
 ## [Unreleased]
 
+- Avoid a critical section when a `send`-link is popped and when returning `free_slot`.
 - Don't force `Signal` import when using `make_signal` macro
-- Update `make_signal`'s documentation to match `make_channel`'s 
+- Update `make_signal`'s documentation to match `make_channel`'s
 
 ## v1.3.2 - 2025-03-16
 

rtic-sync/src/channel.rs

@@ -246,7 +246,19 @@ impl SlotPtr {
         new_value: Option<u8>,
         _cs: critical_section::CriticalSection,
     ) -> Option<u8> {
-        // SAFETY: we are in a critical section.
+        // SAFETY: the critical section guarantees exclusive access, and the
+        // caller guarantees that the pointer is valid.
+        self.replace_exclusive(new_value)
+    }
+
+    /// Replace the value of this slot with `new_value`, and return
+    /// the old value.
+    ///
+    /// SAFETY: the pointer in this `SlotPtr` must be valid for writes, and the caller must guarantee exclusive
+    /// access to the underlying value..
+    unsafe fn replace_exclusive(&mut self, new_value: Option<u8>) -> Option<u8> {
+        // SAFETY: the caller has ensured that we have exclusive access & that
+        // the pointer is valid.
         unsafe { core::ptr::replace(self.0, new_value) }
     }
 }
@@ -338,17 +350,14 @@ impl<T, const N: usize> Sender<'_, T, N> {
             }
 
             // Return our potentially-unused free slot.
-            // Potentially unnecessary c-s because our link was already popped, so there
-            // is no way for anything else to access the free slot ptr. Gotta think
-            // about this a bit more...
-            critical_section::with(|cs| {
-                if let Some(freed_slot) = unsafe { free_slot_ptr2.replace(None, cs) } {
-                    // SAFETY: freed slot is passed to us from `return_free_slot`, which either
-                    // directly (through `try_recv`), or indirectly (through another `return_free_slot`)
-                    // comes from `readyq`.
-                    unsafe { self.0.return_free_slot(freed_slot) };
-                }
-            });
+            // Since we are certain that our link has been removed from the list (either
+            // pop-ed or removed just above), we have exclusive access to the free slot pointer.
+            if let Some(freed_slot) = unsafe { free_slot_ptr2.replace_exclusive(None) } {
+                // SAFETY: freed slot is passed to us from `return_free_slot`, which either
+                // directly (through `try_recv`), or indirectly (through another `return_free_slot`)
+                // comes from `readyq`.
+                unsafe { self.0.return_free_slot(freed_slot) };
+            }
         });
 
         let idx = poll_fn(|cx| {
@@ -366,11 +375,16 @@ impl<T, const N: usize> Sender<'_, T, N> {
                 let link = unsafe { link_ptr.get() };
 
                 // We are already in the wait queue.
-                if let Some(link) = link {
-                    if link.is_popped() {
+                if let Some(queue_link) = link {
+                    if queue_link.is_popped() {
                         // SAFETY: `free_slot_ptr` is valid for writes until the end of this future.
                         let slot = unsafe { free_slot_ptr.replace(None, cs) };
 
+                        // Our link was popped, so it is most definitely not in the list.
+                        // We can safely & correctly `take` it to prevent ourselves from
+                        // redundantly attempting to remove it from the list a 2nd time.
+                        link.take();
+
                         // If our link is popped, then:
                         // 1. We were popped by `return_free_lot` and provided us with a slot.
                         // 2. We were popped by `Receiver::drop` and it did not provide us with a slot, and the channel is closed.