diff options
| author | Sidney Cammeresi <sac@cheesecake.org> | 2025-09-20 16:54:51 -0700 |
|---|---|---|
| committer | Sidney Cammeresi <sac@cheesecake.org> | 2025-09-24 18:08:32 -0700 |
| commit | 137d4eaf12139af894da6c0b6fe74195d6b7b98c (patch) | |
| tree | 993fa064a835b53ada0aa08401ba0c89ea1bde4c /library/alloc/src/collections | |
| parent | 1d23da6b7304d9e2a2c3dcb1b0aaa709cb9bc4ad (diff) | |
| download | rust-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.rs | 3 | ||||
| -rw-r--r-- | library/alloc/src/collections/btree/node.rs | 12 |
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 |
