From dc252159da6062e4e35168ae1f491e1d9af61295 Mon Sep 17 00:00:00 2001 From: Roman Varnava Date: Thu, 29 Oct 2026 16:48:14 +0300 Subject: [PATCH 1/2] test: dropping a LinkedList whose element destructor panics --- src/internal/linked_list.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/internal/linked_list.rs b/src/internal/linked_list.rs index cd12e41..54c8862 100644 --- a/src/internal/linked_list.rs +++ b/src/internal/linked_list.rs @@ -659,3 +659,36 @@ unsafe impl Send for RawIter {} unsafe impl Sync for RawIter {} unsafe impl Send for Cursor {} unsafe impl Sync for Cursor {} + +#[cfg(test)] +mod tests { + use super::LinkedList; + + #[test] + fn a_panicking_element_drop_still_frees_every_node() { + struct Bomb { + panics: bool, + _label: String, + } + impl Drop for Bomb { + fn drop(&mut self) { + if self.panics { + panic!("boom"); + } + } + } + + let mut list = LinkedList::new(); + for i in 0..6 { + list.push_back(Bomb { + panics: i == 2, + _label: i.to_string(), + }); + } + list.pop_back(); // leave a node on the free list too + let dropped = std::panic::catch_unwind(std::panic::AssertUnwindSafe(move || drop(list))); + assert!(dropped.is_err()); + // the other elements, the free list and the sentinel are freed; under + // Miri this also checks that nothing leaks + } +} From 18af2b6fbdf98c56a217871714a2ddbc4afdf449 Mon Sep 17 00:00:00 2001 From: Roman Varnava Date: Thu, 29 Oct 2026 16:48:14 +0300 Subject: [PATCH 2/2] fix: keep LinkedList consistent when an element's drop panics drop_nodes dropped the element before decrementing len, so a panicking destructor left len one too high and the drop guard then dropped the sentinel as an element. The sentinel was also leaked on that path. Move the element out and recycle the node first, and free the free list and the sentinel from the guard on both paths. --- src/internal/linked_list.rs | 42 ++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/src/internal/linked_list.rs b/src/internal/linked_list.rs index 54c8862..b3473ac 100644 --- a/src/internal/linked_list.rs +++ b/src/internal/linked_list.rs @@ -3,7 +3,6 @@ use std::alloc::dealloc; use std::alloc::handle_alloc_error; use std::alloc::Layout; use std::marker::PhantomData; -use std::mem; use std::ptr::NonNull; use std::ptr::{self}; @@ -212,20 +211,23 @@ impl LinkedList { fn drop_nodes(&mut self) { let s = self.sentinel.as_ptr(); while self.len != 0 { - unsafe { + let element = unsafe { // SAFETY: `self.len != 0`, so `(*s).next` points to a real node. let node = (*s).next; let second = (*node).next; (*second).prev = s; (*s).next = second; - let node_ptr = node as *mut Node; // SAFETY: layout-compatible (see `Node` docs); `element` still init. - ptr::drop_in_place(&mut (*node_ptr).element); - // SAFETY: element dropped above; node already unlinked. + let element = ptr::read(&(*(node as *mut Node)).element); + // SAFETY: element moved out above; node already unlinked. self.recycle_node(node); - } + element + }; self.len -= 1; + // Dropped last: if this panics, the list is already consistent and + // the drop guard can carry on. + drop(element); } } @@ -439,17 +441,22 @@ impl LinkedList { } } -// Guard ensures that if dropping an element panics, we still free -// the free-list allocations instead of leaking them (the sentinel -// is freed unconditionally below regardless of panics). +// Frees the rest of the list when it goes out of scope, which also covers an +// element destructor panicking in `LinkedList::drop`: the remaining elements, +// the free list and the sentinel are freed either way. struct DropGuard<'a, T>(&'a mut LinkedList); impl<'a, T> Drop for DropGuard<'a, T> { fn drop(&mut self) { self.0.drop_nodes(); - // SAFETY: `self` is being torn down during unwind; no other - // references to the free list exist, satisfying `drop_freelist`'s contract. - unsafe { self.0.drop_freelist() }; + unsafe { + // SAFETY: the list is being destroyed; no other references to the + // free list exist, satisfying `drop_freelist`'s contract. + self.0.drop_freelist(); + // SAFETY: `sentinel` was allocated in `new` with exactly this + // layout and is never dereferenced after this point. + dealloc(self.0.sentinel.as_ptr() as *mut u8, Layout::new::()); + } } } @@ -457,16 +464,7 @@ impl Drop for LinkedList { fn drop(&mut self) { let guard = DropGuard(self); guard.0.drop_nodes(); - // SAFETY: `drop_nodes` completed without panicking; the free list is - // only touched here and then never again (guard is forgotten next). - unsafe { guard.0.drop_freelist() }; - mem::forget(guard); - // SAFETY: `self.sentinel` was allocated in `new` with exactly this - // layout (`Layout::new::()`), and after this point `self` is - // being destroyed, so the pointer is never dereferenced again. - unsafe { - dealloc(self.sentinel.as_ptr() as *mut u8, Layout::new::()); - } + // `guard` drops here and frees the free list and the sentinel. } }