about summary refs log tree commit diff
path: root/library/alloc/src/collections
diff options
context:
space:
mode:
authorSidney Cammeresi <sac@cheesecake.org>2025-09-20 16:54:51 -0700
committerSidney Cammeresi <sac@cheesecake.org>2025-09-24 18:08:32 -0700
commit137d4eaf12139af894da6c0b6fe74195d6b7b98c (patch)
tree993fa064a835b53ada0aa08401ba0c89ea1bde4c /library/alloc/src/collections
parent1d23da6b7304d9e2a2c3dcb1b0aaa709cb9bc4ad (diff)
downloadrust-137d4eaf12139af894da6c0b6fe74195d6b7b98c.tar.gz
rust-137d4eaf12139af894da6c0b6fe74195d6b7b98c.zip
BTreeMap: Don't leak allocators when initializing nodes
Memory was allocated via `Box::leak` and thence intended to be tracked
and deallocated manually, but the allocator was also leaked, not
tracked, and never dropped.  Now it is dropped immediately.

According to my reading of the `Allocator` trait, if a copy of the
allocator remains live, then its allocations must remain live.  Since
the B-tree has a copy of the allocator that will only be dropped after
the nodes, it's safe to not store the allocator in each node (which
would be a much more intrusive change).
Diffstat (limited to 'library/alloc/src/collections')
-rw-r--r--library/alloc/src/collections/btree/map.rs3
-rw-r--r--library/alloc/src/collections/btree/node.rs12
2 files changed, 13 insertions, 2 deletions
diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs
index cebd25c6039..e3b53b2fe29 100644
--- a/library/alloc/src/collections/btree/map.rs
+++ b/library/alloc/src/collections/btree/map.rs
@@ -194,6 +194,9 @@ pub struct BTreeMap<
     root: Option<Root<K, V>>,
     length: usize,
     /// `ManuallyDrop` to control drop order (needs to be dropped after all the nodes).
+    // Although some of the accessory types store a copy of the allocator, the nodes do not.
+    // Because allocations will remain live as long as any copy (like this one) of the allocator
+    // is live, it's unnecessary to store the allocator in each node.
     pub(super) alloc: ManuallyDrop<A>,
     // For dropck; the `Box` avoids making the `Unpin` impl more strict than before
     _marker: PhantomData<crate::boxed::Box<(K, V), A>>,
diff --git a/library/alloc/src/collections/btree/node.rs b/library/alloc/src/collections/btree/node.rs
index b233e1740b7..77bd90f2c75 100644
--- a/library/alloc/src/collections/btree/node.rs
+++ b/library/alloc/src/collections/btree/node.rs
@@ -224,7 +224,11 @@ impl<K, V> NodeRef<marker::Owned, K, V, marker::Leaf> {
     }
 
     fn from_new_leaf<A: Allocator + Clone>(leaf: Box<LeafNode<K, V>, A>) -> Self {
-        NodeRef { height: 0, node: NonNull::from(Box::leak(leaf)), _marker: PhantomData }
+        // The allocator must be dropped, not leaked.  See also `BTreeMap::alloc`.
+        let (leaf, _alloc) = Box::into_raw_with_allocator(leaf);
+        // SAFETY: the node was just allocated.
+        let node = unsafe { NonNull::new_unchecked(leaf) };
+        NodeRef { height: 0, node, _marker: PhantomData }
     }
 }
 
@@ -242,7 +246,11 @@ impl<K, V> NodeRef<marker::Owned, K, V, marker::Internal> {
         height: usize,
     ) -> Self {
         debug_assert!(height > 0);
-        let node = NonNull::from(Box::leak(internal)).cast();
+        // The allocator must be dropped, not leaked.  See also `BTreeMap::alloc`.
+        let (internal, _alloc) = Box::into_raw_with_allocator(internal);
+        // SAFETY: the node was just allocated.
+        let internal = unsafe { NonNull::new_unchecked(internal) };
+        let node = internal.cast();
         let mut this = NodeRef { height, node, _marker: PhantomData };
         this.borrow_mut().correct_all_childrens_parent_links();
         this