From ec6700a96b22dab7b6a4693d3d8fcccf9f28c932 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= <sosthene@nitrokey.com>
Date: Fri, 5 Apr 2024 11:31:59 +0200
Subject: [PATCH 1/2] Fix memory leak on VecView drop

Drop must also be implemented on `VecView` for the cases wher the `VecView` is owned
even if it is `!Sized`.

This can happen when it is boxed.
---
 src/vec.rs | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/src/vec.rs b/src/vec.rs
index f7c3efbb66..bb972336bc 100644
--- a/src/vec.rs
+++ b/src/vec.rs
@@ -15,12 +15,6 @@ pub trait VecDrop {
 }
 
 impl<T> VecDrop for [MaybeUninit<T>] {
-    unsafe fn drop_with_len(&mut self, _len: usize) {
-        // Case of a view, drop does nothing
-    }
-}
-
-impl<T, const N: usize> VecDrop for [MaybeUninit<T>; N] {
     unsafe fn drop_with_len(&mut self, len: usize) {
         // NOTE(unsafe) avoid bound checks in the slicing operation
         // &mut buffer[..len]
@@ -31,6 +25,12 @@ impl<T, const N: usize> VecDrop for [MaybeUninit<T>; N] {
     }
 }
 
+impl<T, const N: usize> VecDrop for [MaybeUninit<T>; N] {
+    unsafe fn drop_with_len(&mut self, len: usize) {
+        VecDrop::drop_with_len(self.as_mut_slice(), len)
+    }
+}
+
 /// <div class="warn">This is private API and should not be used</div>
 pub struct VecInner<B: ?Sized + VecDrop> {
     len: usize,
@@ -1953,7 +1953,7 @@ mod tests {
 
     use static_assertions::assert_not_impl_any;
 
-    use crate::Vec;
+    use super::{Vec, VecView};
 
     // Ensure a `Vec` containing `!Send` values stays `!Send` itself.
     assert_not_impl_any!(Vec<*const (), 4>: Send);
@@ -2014,6 +2014,33 @@ mod tests {
         assert_eq!(Droppable::count(), 0);
     }
 
+    #[test]
+    fn drop_vecview() {
+        droppable!();
+
+        {
+            let v: Vec<Droppable, 2> = Vec::new();
+            let mut v: Box<VecView<Droppable>> = Box::new(v);
+            v.push(Droppable::new()).ok().unwrap();
+            v.push(Droppable::new()).ok().unwrap();
+            assert_eq!(Droppable::count(), 2);
+            v.pop().unwrap();
+            assert_eq!(Droppable::count(), 1);
+        }
+
+        assert_eq!(Droppable::count(), 0);
+
+        {
+            let v: Vec<Droppable, 2> = Vec::new();
+            let mut v: Box<VecView<Droppable>> = Box::new(v);
+            v.push(Droppable::new()).ok().unwrap();
+            v.push(Droppable::new()).ok().unwrap();
+            assert_eq!(Droppable::count(), 2);
+        }
+
+        assert_eq!(Droppable::count(), 0);
+    }
+
     #[test]
     fn eq() {
         let mut xs: Vec<i32, 4> = Vec::new();

From fc133a9f48f11ff260b3adb7e21099080d2b1ad4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= <sosthene@nitrokey.com>
Date: Fri, 5 Apr 2024 15:24:45 +0200
Subject: [PATCH 2/2] Rename VecDrop to VecBuffer

This represent more clearly the role of the trait now that it's not used
to work around drop specialization.
---
 src/vec.rs | 47 ++++++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/src/vec.rs b/src/vec.rs
index bb972336bc..1db505bb8f 100644
--- a/src/vec.rs
+++ b/src/vec.rs
@@ -5,34 +5,37 @@ use core::{
     ops, ptr, slice,
 };
 
-/// Workaround forbidden specialization of Drop
-pub trait VecDrop {
-    // SAFETY: drop_with_len will be called to call drop in place the first `len` elements of the buffer.
-    // Only the Owned buffer (`[MaybeUninit<T>; N]`) must drop the items
-    // and the view (`[MaybeUninit<T>]`) drops nothing.
-    // `drop_with_len `assumes that the buffer can contain `len` elements.
-    unsafe fn drop_with_len(&mut self, len: usize);
+pub trait VecBuffer {
+    type T;
+
+    fn as_vecview(vec: &VecInner<Self>) -> &VecView<Self::T>;
+    fn as_mut_vecview(vec: &mut VecInner<Self>) -> &mut VecView<Self::T>;
 }
 
-impl<T> VecDrop for [MaybeUninit<T>] {
-    unsafe fn drop_with_len(&mut self, len: usize) {
-        // NOTE(unsafe) avoid bound checks in the slicing operation
-        // &mut buffer[..len]
-        // SAFETY: buffer[..len] must be valid to drop given the safety requirement of the trait definition.
-        let mut_slice = slice::from_raw_parts_mut(self.as_mut_ptr() as *mut T, len);
-        // We drop each element used in the vector by turning into a `&mut [T]`.
-        ptr::drop_in_place(mut_slice);
+impl<T, const N: usize> VecBuffer for [MaybeUninit<T>; N] {
+    type T = T;
+
+    fn as_vecview(vec: &VecInner<Self>) -> &VecView<Self::T> {
+        vec
+    }
+    fn as_mut_vecview(vec: &mut VecInner<Self>) -> &mut VecView<Self::T> {
+        vec
     }
 }
 
-impl<T, const N: usize> VecDrop for [MaybeUninit<T>; N] {
-    unsafe fn drop_with_len(&mut self, len: usize) {
-        VecDrop::drop_with_len(self.as_mut_slice(), len)
+impl<T> VecBuffer for [MaybeUninit<T>] {
+    type T = T;
+
+    fn as_vecview(vec: &VecInner<Self>) -> &VecView<Self::T> {
+        vec
+    }
+    fn as_mut_vecview(vec: &mut VecInner<Self>) -> &mut VecView<Self::T> {
+        vec
     }
 }
 
 /// <div class="warn">This is private API and should not be used</div>
-pub struct VecInner<B: ?Sized + VecDrop> {
+pub struct VecInner<B: ?Sized + VecBuffer> {
     len: usize,
     buffer: B,
 }
@@ -1572,10 +1575,12 @@ impl<T, const N: usize, const M: usize> From<[T; M]> for Vec<T, N> {
     }
 }
 
-impl<T: ?Sized + VecDrop> Drop for VecInner<T> {
+impl<T: ?Sized + VecBuffer> Drop for VecInner<T> {
     fn drop(&mut self) {
+        let mut_slice = VecBuffer::as_mut_vecview(self).as_mut_slice();
+        // We drop each element used in the vector by turning into a `&mut [T]`.
         // SAFETY: the buffer contains initialized data for the range 0..self.len
-        unsafe { self.buffer.drop_with_len(self.len) }
+        unsafe { ptr::drop_in_place(mut_slice) }
     }
 }