diff --git a/src/tests.rs b/src/tests.rs index 4e664a9..7bbaf2a 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -209,6 +209,47 @@ fn test_reserve_increases_capacity() { assert!(erased.capacity() >= 100); } +#[test] +fn test_reserve_is_unwind_safe() { + let mut erased = TypeErasedVec::new(vec![1_u8]); + + let result = panic::catch_unwind(panic::AssertUnwindSafe(|| { + erased.reserve(usize::MAX); + })); + + assert!(result.is_err()); + assert!(erased.is_empty()); +} + +#[test] +fn test_clear_is_unwind_safe_when_element_drop_panics() { + struct PanicOnFirstDrop { + has_panicked: Rc>, + } + + impl Drop for PanicOnFirstDrop { + fn drop(&mut self) { + assert!( + self.has_panicked.replace(true), + "simulated panic while clearing an element" + ); + } + } + + let has_panicked = Rc::new(Cell::new(false)); + let mut erased = TypeErasedVec::new(vec![PanicOnFirstDrop { + has_panicked: has_panicked.clone(), + }]); + + let result = panic::catch_unwind(panic::AssertUnwindSafe(|| { + erased.clear(); + })); + + assert!(result.is_err()); + assert!(has_panicked.get()); + assert!(erased.is_empty()); +} + #[test] fn test_cast_type_success_and_mutation() { let mut vec = Vec::::with_capacity(10); diff --git a/src/vtable.rs b/src/vtable.rs index b042e74..1b38c9f 100644 --- a/src/vtable.rs +++ b/src/vtable.rs @@ -17,27 +17,36 @@ impl TypeErasedVecVtable { } unsafe fn clear_vec(parts: &mut VecParts) { - // SAFETY: We reconstruct the Vec to call clear, dropping only the elements. - // Using into_raw_parts ensures we don't accidentally drop the allocation itself. + let original_parts = std::mem::replace(parts, VecParts::from_vec(Vec::::new())); + + // SAFETY: We transferred ownership of the original allocation out of + // `parts` before reconstructing the Vec. If `clear` unwinds, the Vec + // owns and frees that allocation while `parts` remains a valid empty Vec. unsafe { - let mut vec = parts.into_vec::(); + let mut vec = original_parts.into_vec::(); vec.clear(); let new_parts = VecParts::from_vec(vec); - debug_assert!(std::ptr::eq(new_parts.ptr.as_ptr(), parts.ptr.as_ptr())); - debug_assert_eq!(new_parts.cap, parts.cap); + debug_assert!(std::ptr::eq( + new_parts.ptr.as_ptr(), + original_parts.ptr.as_ptr() + )); + debug_assert_eq!(new_parts.cap, original_parts.cap); debug_assert_eq!(new_parts.len, 0); *parts = new_parts; } } unsafe fn reserve_vec(parts: &mut VecParts, additional: usize) { - // SAFETY: We reconstruct the Vec to trigger a reserve. - // Capacity might change upon reallocation. + let original_parts = std::mem::replace(parts, VecParts::from_vec(Vec::::new())); + + // SAFETY: We transferred ownership of the original allocation out of + // `parts` before reconstructing the Vec. If `reserve` unwinds, the Vec + // owns and frees that allocation while `parts` remains a valid empty Vec. unsafe { - let mut vec = parts.into_vec::(); + let mut vec = original_parts.into_vec::(); vec.reserve(additional); let new_parts = VecParts::from_vec(vec); - debug_assert_eq!(new_parts.len, parts.len); + debug_assert_eq!(new_parts.len, original_parts.len); *parts = new_parts; }