From 0d8780017b871540897abae98e576de58ab6002f Mon Sep 17 00:00:00 2001 From: javalsai Date: Sat, 19 Jul 2025 23:26:22 +0200 Subject: [PATCH] add drop impl --- src/double/mod.rs | 47 +++++++++++++++++++++++++++++++++++---------- src/double/tests.rs | 18 +++++++++-------- src/lib.rs | 5 ++++- 3 files changed, 51 insertions(+), 19 deletions(-) diff --git a/src/double/mod.rs b/src/double/mod.rs index 10a9818..e526821 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::ops::Deref; +use std::{mem::transmute, ops::Deref, pin::Pin}; use parking_lot::RwLock; @@ -26,17 +26,20 @@ impl Default for NodeHeadInner<'_, T> { } } -// TODO: -// impl<'ll, T> Drop for LinkedList<'ll, T> { -// fn drop(&mut self) { -// // SAFETY: this is the very last ref of &self so it can pretty much assume no external -// // refs into the inner data as they would be invalid to live after this -// while unsafe { self.pop().is_some() } {} -// } -// } - pub struct LinkedList<'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> { #[must_use] fn default() -> Self { @@ -58,6 +61,30 @@ impl<'ll, T> LinkedList<'ll, T> { Self::default() } + /// # Safety + /// + /// The only context in which it's safe to call this when [`self`] was pinned immediately after + /// its creation so that it can be guaranteed that the returned reference is valid for all the + /// [`LinkedList`]'s lifetime. + /// + /// The issue arises from the [`Drop`] implementation. It takes a `&mut` reference, that means + /// that all previous immutable references are dropped. But most methods of the linked require + /// you to promise the borrow of [`self`] is valid for all `'ll` and that's only true if no + /// destructor runs. This makes [`Drop`] incompatible with the use of methods of the form + /// `fn(&'myself self)`. + /// + /// In turn to this, the [`Drop`] implementation must assume it's not the only reference even + /// thought it's `&mut`. Anyhow it should only be called when the scope of [`self`] is about to + /// end and the other references would be invalidated after the call to [`Drop`], though in + /// reality they really have no use before that call but to guarantee the "internal" self + /// references in the linked list remain valid through the destructor. It's kinda a really edge + /// case in the language with shared structures that require references with lifetimes as long + /// as self to guarantee their validity but still have [`Drop`] implementations. + #[must_use] + pub unsafe fn get_self_ref(myself: Pin<&Self>) -> &'ll Self { + unsafe { transmute::<&Self, &'ll Self>(myself.get_ref()) } + } + pub fn prepend(&'ll self, data: T) { let self_lock = self.write(); let next = self_lock.start; diff --git a/src/double/tests.rs b/src/double/tests.rs index 3b9d170..0e7efa2 100644 --- a/src/double/tests.rs +++ b/src/double/tests.rs @@ -2,6 +2,7 @@ use std::{ fmt::Debug, + pin::pin, sync::{ Barrier, atomic::{AtomicUsize, Ordering}, @@ -35,14 +36,16 @@ impl Debug for StringWithDrop { } #[test] -fn it_works() { +fn concurrency_and_scoped_drop() { const CREATE1: usize = 100; const CREATE2: usize = 100; const NOT_POP: usize = 20; - // TODO: Find out WHYTF do I need to scope this and compiler won't let me `drop` { - let ll = LinkedList::::new(); + // 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()) }; + let barrier = Barrier::new(3); thread::scope(|s| { @@ -50,14 +53,14 @@ fn it_works() { barrier.wait(); for n in 0..CREATE1 { - ll.prepend(format!("A {n}").into()); + llref.prepend(format!("A {n}").into()); } }); s.spawn(|| { barrier.wait(); for n in 0..CREATE2 { - ll.prepend(format!("B {n}").into()); + llref.prepend(format!("B {n}").into()); } }); s.spawn(|| { @@ -65,7 +68,7 @@ fn it_works() { for _ in 0..(CREATE1 + CREATE2 - NOT_POP) { unsafe { - while ll.pop().is_none() { + while llref.pop().is_none() { std::thread::yield_now(); } } @@ -76,6 +79,5 @@ fn it_works() { assert_eq!(DROP_C.load(Ordering::Relaxed), CREATE1 + CREATE2 - NOT_POP); } - // TODO: when drop is impl - // assert_eq!(DROP_C.load(Ordering::Relaxed), CREATE1 + CREATE2); + assert_eq!(DROP_C.load(Ordering::Relaxed), CREATE1 + CREATE2); } diff --git a/src/lib.rs b/src/lib.rs index 088a120..2098141 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,8 @@ #![doc = include_str!("../README.md")] -#![feature(arbitrary_self_types, arbitrary_self_types_pointers)] +#![feature( + arbitrary_self_types, + arbitrary_self_types_pointers, +)] #![warn(clippy::pedantic)] #[cfg(doc)]