From af5e5ff19eefa8c0ddb85ff4a9c40614378232e2 Mon Sep 17 00:00:00 2001 From: javalsai Date: Sun, 20 Jul 2025 01:36:48 +0200 Subject: [PATCH] safety: what I think is a safe macro for linked list creation + some docs changes and cleanup --- README.md | 7 +- src/double/mod.rs | 140 +++++++++++++++++++++++++---- src/double/node/topology_safety.rs | 5 +- src/double/tests.rs | 18 ++-- src/lib.rs | 7 +- 5 files changed, 143 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 11d620e..0cce9aa 100644 --- a/README.md +++ b/README.md @@ -1,9 +1,14 @@ -A [`LinkedList`] implementation avoiding the use of [`Arc`]s in favor of unsafe manual removal of nodes when the caller knows all possible references are left unused. +A **linked list** implementation avoiding the use of [`Arc`]s in favor of unsafe manual removal of nodes when the caller knows all possible references are left unused. The point of this crate is to offer [`Pin`] guarantees on the references into the list while allowing it to be modified. The implementation of all this doesn't require mutable access to the linked list itself so as a side effect it's possible to use the list in concurrent manners. This means that it will try as smartly as possible to allow concurrent modifications to it as long as the nodes affected are unrelated. +# Types + +There could be different types of linked list implementations in the future, like safer ones with [`Arc`], single-threaded ones, etc. But right now there's only: +* [`DoublyLinkedList`]: [`crate::double`] doubly linked list only in the heap with manual unsafe removal of items in it. + --- `cargo doc` is supported and is the main documentation of the library. But there's no official hosting of the document files. diff --git a/src/double/mod.rs b/src/double/mod.rs index e526821..cfd3bcc 100644 --- a/src/double/mod.rs +++ b/src/double/mod.rs @@ -2,7 +2,7 @@ //! //! Doubly as each node points to the next and previous node. -use std::{mem::transmute, ops::Deref, pin::Pin}; +use std::{marker::PhantomPinned, mem::transmute, ops::Deref, pin::Pin}; use parking_lot::RwLock; @@ -26,28 +26,16 @@ impl Default for NodeHeadInner<'_, T> { } } -pub struct LinkedList<'ll, T>(RwLock>); +pub struct NodeHead<'ll, T>(RwLock>); -impl<'ll, T> Drop for LinkedList<'ll, T> { - fn drop(&mut self) { - // SAFETY: this is the drop impl so we can guarantee the reference is valid for the - // lifetime of the struct itself and external references would be invalidated right after - // the [`Drop`] of it (this fn). I don't think there's a way to differenciate the lifetimes - // by a drop implementation so this would be safe as no external references lifetimes would - // be valid after drop finishes - let myself = unsafe { transmute::<&mut Self, &'ll mut Self>(self) }; - while unsafe { myself.pop().is_some() } {} - } -} - -impl Default for LinkedList<'_, T> { +impl Default for NodeHead<'_, T> { #[must_use] fn default() -> Self { Self(RwLock::new(NodeHeadInner::default())) } } -impl<'ll, T> Deref for LinkedList<'ll, T> { +impl<'ll, T> Deref for NodeHead<'ll, T> { type Target = RwLock>; fn deref(&self) -> &Self::Target { @@ -55,7 +43,7 @@ impl<'ll, T> Deref for LinkedList<'ll, T> { } } -impl<'ll, T> LinkedList<'ll, T> { +impl<'ll, T> NodeHead<'ll, T> { #[must_use] pub fn new() -> Self { Self::default() @@ -85,6 +73,18 @@ impl<'ll, T> LinkedList<'ll, T> { unsafe { transmute::<&Self, &'ll Self>(myself.get_ref()) } } + /// # Safety + /// + /// Must be at the end at the end of its scope, when there's no references into it left. + pub unsafe fn manual_drop(&'ll self) { + // SAFETY: this is the drop impl so we can guarantee the reference is valid for the + // lifetime of the struct itself and external references would be invalidated right after + // the [`Drop`] of it (this fn). I don't think there's a way to differenciate the lifetimes + // by a drop implementation so this would be safe as no external references lifetimes would + // be valid after drop finishes + while unsafe { self.pop().is_some() } {} + } + pub fn prepend(&'ll self, data: T) { let self_lock = self.write(); let next = self_lock.start; @@ -135,5 +135,111 @@ impl<'ll, T> LinkedList<'ll, T> { } } +// Can't quite make this work how I want 😭 + +/// Attempt to safe wrap around a [`NodeHead`] to allow a sound API with [`Drop`] implementation. +/// +/// Please see [`create_ll`] and [`del_ll`] +pub struct LinkedList<'ll, T> { + head: NodeHead<'ll, T>, + _pinned: PhantomPinned, +} + +impl<'ll, T> LinkedList<'ll, T> { + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// # Safety + /// + /// NEVER EVER EVER extend the lifetime of this beyond the end of scope of the linked list + /// itself, it's all good and safe UNTIL it's dropped. + /// + /// Allows you to associate the lifetime of this to something else to prevent accidentall + /// over-extending the lifetime. + pub unsafe fn extend_for<'scope>(&self, _: &'scope impl std::any::Any) -> &'ll NodeHead<'ll, T> + where + 'scope: 'll, + { + unsafe { transmute(&self.head) } + } +} + +impl Default for LinkedList<'_, T> { + #[must_use] + fn default() -> Self { + Self { + head: NodeHead::new(), + _pinned: PhantomPinned, + } + } +} + +impl Drop for LinkedList<'_, T> { + fn drop(&mut self) { + // Extend to 'static so the compiler doesn't cry, we know this covers 'll + let myself = unsafe { self.extend_for(&()) }; + // And this is `Drop` so there shouldn't be any refs as the end of this function would be + // where their lifetime ('ll) ends + unsafe { myself.manual_drop() } + } +} + +// I'm not so sure this is that much bulletproof but behaves sollidly enough + +/// Unsafe macro that automatically creates a linked list and an extended reference. +/// +/// Also creates a `scope = ()` variable at the same level as `$val` to mimic its scope and prevent +/// accidental expansion of the unsafe lifetime beyond the current scope. +/// +/// For example: +/// +/// ``` +/// use concurrent_linked_list::double::{create_ll, del_ll, LinkedList}; +/// +/// create_ll!(LinkedList::::new(), ll_val, ll); +/// ll.prepend("test".to_string()); +/// del_ll!(ll_val, ll); +/// ``` +/// +/// But trying to use `ll` after the deletion should fail: +/// +/// ```compile_fail +/// use concurrent_linked_list::double::{create_ll, del_ll, LinkedList}; +/// +/// create_ll!(LinkedList::::new(), ll_val, ll); +/// ll.prepend("test".to_string()); +/// del_ll!(ll_val, ll); +/// ll.prepend("test2".to_string()); +/// ``` +/// +/// Or trying to expand `ll` beyond the scope it was defined in, even if not deleted: +/// +/// ```compile_fail +/// use concurrent_linked_list::double::{create_ll, del_ll, LinkedList}; +/// +/// let ll = { +/// create_ll!(LinkedList::::new(), ll_val, ll); +/// ll.prepend("test".to_string()); +/// ll +/// } +/// ll.prepend("test2".to_string()); +/// ``` +pub macro create_ll($rhs:expr, $val:ident, $ref:ident) { + let $val = $rhs; + let scope = (); + let $ref = unsafe { $val.extend_for(&scope) }; +} + +/// Macro that attempts to run some higene cleanup on [`create_ll`] to avoid accidental use ot the +/// reference further too. Other functions could still extend the lifetime beyond acceptable +/// though. +pub macro del_ll($val:ident, $ref:ident) { + #[allow(unused_variables)] + let $ref = (); + drop($val); +} + #[cfg(test)] mod tests; diff --git a/src/double/node/topology_safety.rs b/src/double/node/topology_safety.rs index 4b704e3..85c7510 100644 --- a/src/double/node/topology_safety.rs +++ b/src/double/node/topology_safety.rs @@ -68,4 +68,7 @@ //! This is more a graph than a linked list. #[allow(unused_imports)] -use {super::NodeHeadInner, std::mem::ManuallyDrop}; +use { + super::{super::LinkedList, NodeHeadInner}, + std::mem::ManuallyDrop, +}; diff --git a/src/double/tests.rs b/src/double/tests.rs index 0e7efa2..87346f4 100644 --- a/src/double/tests.rs +++ b/src/double/tests.rs @@ -2,7 +2,6 @@ use std::{ fmt::Debug, - pin::pin, sync::{ Barrier, atomic::{AtomicUsize, Ordering}, @@ -42,10 +41,7 @@ fn concurrency_and_scoped_drop() { const NOT_POP: usize = 20; { - // TODO: make this a macro or a "guard" struct that can be dropped - let ll = pin!(LinkedList::::new()); - let llref = unsafe { LinkedList::get_self_ref(ll.as_ref()) }; - + create_ll!(LinkedList::::new(), ll_val, ll); let barrier = Barrier::new(3); thread::scope(|s| { @@ -53,14 +49,14 @@ fn concurrency_and_scoped_drop() { barrier.wait(); for n in 0..CREATE1 { - llref.prepend(format!("A {n}").into()); + ll.prepend(format!("A {n}").into()); } }); s.spawn(|| { barrier.wait(); for n in 0..CREATE2 { - llref.prepend(format!("B {n}").into()); + ll.prepend(format!("B {n}").into()); } }); s.spawn(|| { @@ -68,7 +64,7 @@ fn concurrency_and_scoped_drop() { for _ in 0..(CREATE1 + CREATE2 - NOT_POP) { unsafe { - while llref.pop().is_none() { + while ll.pop().is_none() { std::thread::yield_now(); } } @@ -77,7 +73,9 @@ fn concurrency_and_scoped_drop() { }); assert_eq!(DROP_C.load(Ordering::Relaxed), CREATE1 + CREATE2 - NOT_POP); - } - assert_eq!(DROP_C.load(Ordering::Relaxed), CREATE1 + CREATE2); + del_ll!(ll_val, ll); + + assert_eq!(DROP_C.load(Ordering::Relaxed), CREATE1 + CREATE2); + } } diff --git a/src/lib.rs b/src/lib.rs index 2098141..2bdd64e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,8 +1,5 @@ #![doc = include_str!("../README.md")] -#![feature( - arbitrary_self_types, - arbitrary_self_types_pointers, -)] +#![feature(arbitrary_self_types, arbitrary_self_types_pointers, decl_macro)] #![warn(clippy::pedantic)] #[cfg(doc)] @@ -10,4 +7,4 @@ use std::{pin::Pin, sync::Arc}; pub mod double; -pub use double::NodeHeadInner as DoublyLinkedList; +pub use double::LinkedList as DoublyLinkedList;