20260130 resolve crash in evict_to_len

src/arcache/ll.rs

@@ -22,14 +22,12 @@ fn alloc_nid() -> usize {
     {
         LL_ALLOC_LIST.with(|llist| llist.lock().unwrap().insert(nid));
     }
-    eprintln!("Allocate -> {:?}", nid);
+    // eprintln!("LL Allocate -> {:?}", nid);
     nid
 }
 
-#[cfg(all(test, not(miri)))]
+#[cfg(all(test, not(miri), not(feature = "dhat-heap")))]
 fn release_nid(nid: usize) {
-    println!("Release -> {:?}", nid);
-    #[cfg(not(feature = "dhat-heap"))]
     {
         let r = LL_ALLOC_LIST.with(|llist| llist.lock().unwrap().remove(&nid));
         assert!(r);
@@ -53,15 +51,6 @@ pub trait LLWeight {
     fn ll_weight(&self) -> usize;
 }
 
-/*
-impl<T> LLWeight for T {
-    #[inline]
-    default fn ll_weight(&self) -> usize {
-        1
-    }
-}
-*/
-
 #[derive(Clone, Debug)]
 pub(crate) struct LL<K>
 where
@@ -138,8 +127,7 @@ impl<K> LLNodeOwned<K>
 where
     K: LLWeight + Clone + Debug,
 {
-    #[allow(clippy::wrong_self_convention)]
-    fn into_inner(&mut self) -> *mut LLNode<K> {
+    fn into_inner(mut self) -> *mut LLNode<K> {
         let x = self.inner;
         self.inner = ptr::null_mut();
         x
@@ -187,7 +175,7 @@ where
                 debug_assert!((*self.inner).next.is_null());
                 debug_assert!((*self.inner).prev.is_null());
             }
-            panic!("dropping LLNodeOwned<K>");
+            panic!("dropping LLNodeOwned<K> which has valid content, this should never happen!");
         }
     }
 }
@@ -233,19 +221,29 @@ where
         self.append_n(n)
     }
 
-    // Append an arbitrary node into this set.
-    pub(crate) fn append_n(&mut self, mut owned: LLNodeOwned<K>) -> LLNodeRef<K> {
+    // Append an arbitrary node into this set, at the tail end.
+    pub(crate) fn append_n(&mut self, owned: LLNodeOwned<K>) -> LLNodeRef<K> {
         // Who is to the left of tail?
         let n = owned.into_inner();
         unsafe {
             self.size += (*(*n).k.as_ptr()).ll_weight();
             // must be untagged
             // assert!((*n).tag == 0);
+            // Tail has no trailing nodes.
             debug_assert!((*self.tail).next.is_null());
+            // Tail must have a previous node.
             debug_assert!(!(*self.tail).prev.is_null());
+            // What is that predecessor?
             let pred = (*self.tail).prev;
             debug_assert!(!pred.is_null());
+            // Assert that the predecessor points at tail correctly.
+            // We know now that:
+            //
+            // pred <-> tail
             debug_assert!((*pred).next == self.tail);
+
+            // Tell our node where we are.
+            // pred <-> n <-> tail
             (*n).prev = pred;
             (*n).next = self.tail;
             // (*n).tag = self.tag;
@@ -259,6 +257,7 @@ where
             debug_assert!(!(*(*n).next).prev.is_null());
             debug_assert!((*(*n).prev).next == n);
             debug_assert!((*(*n).next).prev == n);
+            // We have asserted the list is sane.
         };
         LLNodeRef { inner: n }
     }
@@ -269,30 +268,57 @@ where
         if n.inner == unsafe { (*self.tail).prev } {
             // Done, no-op
         } else {
+            let previous_size = self.size;
+
             let owned = self.extract(n);
             self.append_n(owned);
+
+            let after_size = self.size;
+
+            // Ensure that we didn't actually alter the size of the list during the operation,
+            // we only re-arranged the content.
+            debug_assert_eq!(previous_size, after_size);
         }
     }
 
     // remove this node from the ll, and return it's ptr.
-    pub(crate) fn pop(&mut self) -> LLNodeOwned<K> {
-        let n = unsafe { (*self.head).next };
-        let owned = self.extract(LLNodeRef { inner: n });
-        debug_assert!(!owned.is_null());
-        debug_assert!(owned.inner != self.head);
-        debug_assert!(owned.inner != self.tail);
-        owned
+    pub(crate) fn pop(&mut self) -> Option<LLNodeOwned<K>> {
+        let next = unsafe { (*self.head).next };
+        if next == self.tail {
+            None
+        } else {
+            let n = unsafe { (*self.head).next };
+            let owned = self.extract(LLNodeRef { inner: n });
+            debug_assert!(!owned.is_null());
+            debug_assert!(owned.inner != self.head);
+            debug_assert!(owned.inner != self.tail);
+            Some(owned)
+        }
+    }
+
+    pub(crate) fn pop_n_free(&mut self) -> Option<K> {
+        if let Some(owned) = self.pop() {
+            let ll_node = owned.into_inner();
+            let k = LLNode::into_inner(ll_node);
+            Some(k)
+        } else {
+            None
+        }
     }
 
     // Cut a node out from this list from any location.
     pub(crate) fn extract(&mut self, n: LLNodeRef<K>) -> LLNodeOwned<K> {
-        assert!(self.size > 0);
         assert!(!n.is_null());
+        assert!(self.size > 0);
         unsafe {
             // We should have a prev and next
             debug_assert!(!(*n.inner).prev.is_null());
             debug_assert!(!(*n.inner).next.is_null());
             // And that prev's next is us, and next's prev is us.
+            // This is asserting that we have a proper construction
+            //
+            // prev <-> n <-> next
+            //
             debug_assert!(!(*(*n.inner).prev).next.is_null());
             debug_assert!(!(*(*n.inner).next).prev.is_null());
             debug_assert!((*(*n.inner).prev).next == n.inner);
@@ -305,14 +331,24 @@ where
         unsafe {
             let prev = (*n.inner).prev;
             let next = (*n.inner).next;
+            // Currently is:
+            //
             // prev <-> n <-> next
             (*next).prev = prev;
             (*prev).next = next;
+            // Now is
+            // prev <-> next
+
             // Null things for paranoia.
             if cfg!(test) || cfg!(debug_assertions) {
                 (*n.inner).prev = ptr::null_mut();
                 (*n.inner).next = ptr::null_mut();
             }
+            // Finally, we have
+            //
+            // null <- n -> null
+            // prev <-> next
+
             // (*n).tag = 0;
         }
 
@@ -324,10 +360,7 @@ where
     }
 
     pub(crate) fn drop_head(&mut self) {
-        assert!(self.size > 0);
-        let next = unsafe { (*self.head).next };
-        if next != self.tail {
-            let mut owned = self.pop();
+        if let Some(owned) = self.pop() {
             let n = owned.into_inner();
             LLNode::free(n);
         }
@@ -361,6 +394,62 @@ where
             Some(l)
         }
     }
+
+    #[cfg(test)]
+    pub(crate) fn verify(&self) {
+        // Walk the list to ensure it's sane.
+
+        let head = self.head;
+        let tail = self.tail;
+
+        let expect_size = self.size;
+        let mut size = 0;
+
+        // Establish that the sentinel nodes exist, and that they
+        // have the correct out bound null/non-null pointers.
+        assert_ne!(head, tail);
+
+        unsafe {
+            assert!((*head).prev.is_null());
+            assert!(!(*head).next.is_null());
+
+            assert!((*tail).next.is_null());
+            assert!(!(*tail).prev.is_null());
+        }
+
+        // Now we walk each item to assert that they have the correct structure.
+        let mut n = unsafe { (*head).next };
+        // We have to manually check head here.
+        unsafe {
+            assert_eq!((*n).prev, head);
+        }
+
+        while n != tail {
+            unsafe {
+                // Setup the pointer for the next iteration.
+                let next = (*n).next;
+
+                // Add the size.
+                size += (*(*n).k.as_ptr()).ll_weight();
+
+                // assert we have outbound pointers.
+                assert!(!(*n).prev.is_null());
+                assert!(!(*n).next.is_null());
+
+                // Assert that the previous node points to us.
+                assert!(!(*(*n).prev).next.is_null());
+                assert!((*(*n).prev).next == n);
+                // Assert that the next node points back to us.
+                // NOTE: This accounts for tail pointing back to us.
+                assert!(!(*(*n).next).prev.is_null());
+                assert!((*(*n).next).prev == n);
+
+                n = next;
+            }
+        }
+
+        assert_eq!(expect_size, size);
+    }
 }
 
 impl<K> Drop for LL<K>
@@ -446,21 +535,28 @@ where
     }
 
     #[inline]
-    fn free(v: *mut Self) {
+    fn into_inner(v: *mut Self) -> K {
         debug_assert!(!v.is_null());
         let llnode = unsafe { Box::from_raw(v) };
+        let k = unsafe { llnode.k.assume_init() };
+        #[cfg(all(test, not(miri), not(feature = "dhat-heap")))]
+        release_nid(llnode.nid);
+
+        k
+    }
+
+    #[inline]
+    fn free(v: *mut Self) {
         // drop the inner k.
-        let _ = unsafe { llnode.k.assume_init() };
-        #[cfg(all(test, not(miri)))]
-        release_nid(llnode.nid)
+        let _ = Self::into_inner(v);
     }
 
     #[inline]
     fn free_marker(v: *mut Self) {
         debug_assert!(!v.is_null());
         let _llnode = unsafe { Box::from_raw(v) };
         // Markers never have a k to drop.
-        #[cfg(all(test, not(miri)))]
+        #[cfg(all(test, not(miri), not(feature = "dhat-heap")))]
         release_nid(_llnode.nid)
     }
 }
@@ -552,7 +648,7 @@ mod tests {
         assert!(ll.peek_tail().unwrap().as_ref() == &1);
 
         // pop from head
-        let n3 = ll.pop();
+        let n3 = ll.pop().unwrap();
         assert!(ll.len() == 3);
         assert!(ll.peek_head().unwrap().as_ref() == &4);
         assert!(ll.peek_tail().unwrap().as_ref() == &1);
@@ -574,7 +670,7 @@ mod tests {
         assert!(ll.peek_head().unwrap().as_ref() == &4);
         assert!(ll.peek_tail().unwrap().as_ref() == &4);
         // Remove last
-        let n4 = ll.pop();
+        let n4 = ll.pop().unwrap();
         assert!(ll.len() == 0);
         assert!(ll.peek_head().is_none());
         assert!(ll.peek_tail().is_none());
@@ -610,9 +706,9 @@ mod tests {
         assert!(ll.len() == 8);
         let _n2 = ll.append_k(Weighted { _i: 2 });
         assert!(ll.len() == 16);
-        let n1 = ll.pop();
+        let n1 = ll.pop().unwrap();
         assert!(ll.len() == 8);
-        let n2 = ll.pop();
+        let n2 = ll.pop().unwrap();
         assert!(ll.len() == 0);
         // Add back so they drop
         ll.append_n(n1);