From 5c39f5c7ee2e87a2a431e3c8390d71ecabaf6d65 Mon Sep 17 00:00:00 2001
From: Jorge Aparicio <jorge.aparicio@ferrous-systems.com>
Date: Mon, 9 May 2022 15:07:03 +0200
Subject: [PATCH] fix: BinaryHeap elements are dropped twice

---
 CHANGELOG.md        |  1 +
 src/binary_heap.rs  | 45 +++++++++++++++++++++++++++++++++++++++------
 src/lib.rs          |  4 ++++
 src/test_helpers.rs | 23 +++++++++++++++++++++++
 src/vec.rs          | 23 -----------------------
 5 files changed, 67 insertions(+), 29 deletions(-)
 create mode 100644 src/test_helpers.rs

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 93e0090b..4abcb2f2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 * Fixed `pool` example in docstring.
 * Fixed undefined behavior in `Vec::truncate()`, `Vec::swap_remove_unchecked()`,
   and `Hole::move_to()` (internal to the binary heap implementation).
+* Fixed `BinaryHeap` elements are being dropped twice
 
 ### Added
 
diff --git a/src/binary_heap.rs b/src/binary_heap.rs
index dc9ea590..5bf01827 100644
--- a/src/binary_heap.rs
+++ b/src/binary_heap.rs
@@ -537,12 +537,6 @@ where
     }
 }
 
-impl<T, K, const N: usize> Drop for BinaryHeap<T, K, N> {
-    fn drop(&mut self) {
-        unsafe { ptr::drop_in_place(self.data.as_mut_slice()) }
-    }
-}
-
 impl<T, K, const N: usize> fmt::Debug for BinaryHeap<T, K, N>
 where
     K: Kind,
@@ -577,6 +571,45 @@ mod tests {
         static mut _B: BinaryHeap<i32, Min, 16> = BinaryHeap::new();
     }
 
+    #[test]
+    fn drop() {
+        droppable!();
+
+        {
+            let mut v: BinaryHeap<Droppable, Max, 2> = BinaryHeap::new();
+            v.push(Droppable::new()).ok().unwrap();
+            v.push(Droppable::new()).ok().unwrap();
+            v.pop().unwrap();
+        }
+
+        assert_eq!(unsafe { COUNT }, 0);
+
+        {
+            let mut v: BinaryHeap<Droppable, Max, 2> = BinaryHeap::new();
+            v.push(Droppable::new()).ok().unwrap();
+            v.push(Droppable::new()).ok().unwrap();
+        }
+
+        assert_eq!(unsafe { COUNT }, 0);
+
+        {
+            let mut v: BinaryHeap<Droppable, Min, 2> = BinaryHeap::new();
+            v.push(Droppable::new()).ok().unwrap();
+            v.push(Droppable::new()).ok().unwrap();
+            v.pop().unwrap();
+        }
+
+        assert_eq!(unsafe { COUNT }, 0);
+
+        {
+            let mut v: BinaryHeap<Droppable, Min, 2> = BinaryHeap::new();
+            v.push(Droppable::new()).ok().unwrap();
+            v.push(Droppable::new()).ok().unwrap();
+        }
+
+        assert_eq!(unsafe { COUNT }, 0);
+    }
+
     #[test]
     fn min() {
         let mut heap = BinaryHeap::<_, Min, 16>::new();
diff --git a/src/lib.rs b/src/lib.rs
index a8b52a3f..a35aebee 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -85,6 +85,10 @@ pub use pool::singleton::arc::Arc;
 pub use string::String;
 pub use vec::Vec;
 
+#[macro_use]
+#[cfg(test)]
+mod test_helpers;
+
 mod deque;
 mod histbuf;
 mod indexmap;
diff --git a/src/test_helpers.rs b/src/test_helpers.rs
new file mode 100644
index 00000000..8967f3fb
--- /dev/null
+++ b/src/test_helpers.rs
@@ -0,0 +1,23 @@
+macro_rules! droppable {
+    () => {
+        #[derive(Eq, Ord, PartialEq, PartialOrd)]
+        struct Droppable(i32);
+        impl Droppable {
+            fn new() -> Self {
+                unsafe {
+                    COUNT += 1;
+                    Droppable(COUNT)
+                }
+            }
+        }
+        impl Drop for Droppable {
+            fn drop(&mut self) {
+                unsafe {
+                    COUNT -= 1;
+                }
+            }
+        }
+
+        static mut COUNT: i32 = 0;
+    };
+}
diff --git a/src/vec.rs b/src/vec.rs
index cc8202b7..909e69d6 100644
--- a/src/vec.rs
+++ b/src/vec.rs
@@ -901,29 +901,6 @@ mod tests {
         assert!(v.is_full());
     }
 
-    macro_rules! droppable {
-        () => {
-            struct Droppable;
-            impl Droppable {
-                fn new() -> Self {
-                    unsafe {
-                        COUNT += 1;
-                    }
-                    Droppable
-                }
-            }
-            impl Drop for Droppable {
-                fn drop(&mut self) {
-                    unsafe {
-                        COUNT -= 1;
-                    }
-                }
-            }
-
-            static mut COUNT: i32 = 0;
-        };
-    }
-
     #[test]
     fn drop() {
         droppable!();