Skip to content

Commit 137d4ea

Browse files
committed
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).
1 parent 1d23da6 commit 137d4ea

File tree

2 files changed

+13
-2
lines changed

2 files changed

+13
-2
lines changed

library/alloc/src/collections/btree/map.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,9 @@ pub struct BTreeMap<
194194
root: Option<Root<K, V>>,
195195
length: usize,
196196
/// `ManuallyDrop` to control drop order (needs to be dropped after all the nodes).
197+
// Although some of the accessory types store a copy of the allocator, the nodes do not.
198+
// Because allocations will remain live as long as any copy (like this one) of the allocator
199+
// is live, it's unnecessary to store the allocator in each node.
197200
pub(super) alloc: ManuallyDrop<A>,
198201
// For dropck; the `Box` avoids making the `Unpin` impl more strict than before
199202
_marker: PhantomData<crate::boxed::Box<(K, V), A>>,

library/alloc/src/collections/btree/node.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,11 @@ impl<K, V> NodeRef<marker::Owned, K, V, marker::Leaf> {
224224
}
225225

226226
fn from_new_leaf<A: Allocator + Clone>(leaf: Box<LeafNode<K, V>, A>) -> Self {
227-
NodeRef { height: 0, node: NonNull::from(Box::leak(leaf)), _marker: PhantomData }
227+
// The allocator must be dropped, not leaked. See also `BTreeMap::alloc`.
228+
let (leaf, _alloc) = Box::into_raw_with_allocator(leaf);
229+
// SAFETY: the node was just allocated.
230+
let node = unsafe { NonNull::new_unchecked(leaf) };
231+
NodeRef { height: 0, node, _marker: PhantomData }
228232
}
229233
}
230234

@@ -242,7 +246,11 @@ impl<K, V> NodeRef<marker::Owned, K, V, marker::Internal> {
242246
height: usize,
243247
) -> Self {
244248
debug_assert!(height > 0);
245-
let node = NonNull::from(Box::leak(internal)).cast();
249+
// The allocator must be dropped, not leaked. See also `BTreeMap::alloc`.
250+
let (internal, _alloc) = Box::into_raw_with_allocator(internal);
251+
// SAFETY: the node was just allocated.
252+
let internal = unsafe { NonNull::new_unchecked(internal) };
253+
let node = internal.cast();
246254
let mut this = NodeRef { height, node, _marker: PhantomData };
247255
this.borrow_mut().correct_all_childrens_parent_links();
248256
this

0 commit comments

Comments
 (0)