Repository navigation
Box::leak will not call allocator destruction function #106207
Description
I tried this code:
#![feature(btreemap_alloc)]
#![feature(allocator_api)]
use std::collections::BTreeMap;
use std::sync::Arc;
use std::alloc::Allocator;
use std::ptr::{self, NonNull};
#[derive(Clone)]
pub struct AllocWrapper<T>(pub Arc<T>);
unsafe impl<T: Allocator> Allocator for AllocWrapper<T> {
fn allocate(
&self,
layout: std::alloc::Layout,
) -> Result<std::ptr::NonNull<[u8]>, std::alloc::AllocError> {
self.0.allocate(layout)
}
unsafe fn deallocate(&self, ptr: std::ptr::NonNull<u8>, layout: std::alloc::Layout) {
self.0.deallocate(ptr, layout)
}
fn allocate_zeroed(
&self,
layout: std::alloc::Layout,
) -> Result<std::ptr::NonNull<[u8]>, std::alloc::AllocError> {
self.0.allocate_zeroed(layout)
}
unsafe fn grow(
&self,
ptr: std::ptr::NonNull<u8>,
old_layout: std::alloc::Layout,
new_layout: std::alloc::Layout,
) -> Result<std::ptr::NonNull<[u8]>, std::alloc::AllocError> {
self.0.grow(ptr, old_layout, new_layout)
}
unsafe fn grow_zeroed(
&self,
ptr: std::ptr::NonNull<u8>,
old_layout: std::alloc::Layout,
new_layout: std::alloc::Layout,
) -> Result<std::ptr::NonNull<[u8]>, std::alloc::AllocError> {
self.0.grow_zeroed(ptr, old_layout, new_layout)
}
unsafe fn shrink(
&self,
ptr: std::ptr::NonNull<u8>,
old_layout: std::alloc::Layout,
new_layout: std::alloc::Layout,
) -> Result<std::ptr::NonNull<[u8]>, std::alloc::AllocError> {
self.0.shrink(ptr, old_layout, new_layout)
}
}
fn main() {
let allocator = AllocWrapper(Arc::new(std::alloc::System));
println!("count = {}", Arc::strong_count(&allocator.0));
let x = Box::new_in(5, allocator.clone());
println!("count = {}", Arc::strong_count(&allocator.0));
let _ = NonNull::from(Box::leak(x));
println!("count = {}", Arc::strong_count(&allocator.0));
}I expected to see this happen:
- When the
xget dropped, the ref count decreased to 1
Instead, this happened:
- When the
xget dropped, the ref count dose not change
The current implementation of Box::leak will ManuallyDrop the whole Box but only leak the data pointer. The destruction function of the allocator will not be called. I'm not sure if it the expected behavior.
pub const fn leak<'a>(b: Self) -> &'a mut T
where
A: 'a,
{
unsafe { &mut *mem::ManuallyDrop::new(b).0.as_ptr() }
}Meta
rustc --version --verbose:
rustc 1.68.0-nightly (bdb07a8ec 2022-12-11)
binary: rustc
commit-hash: bdb07a8ec8e77aa10fb84fae1d4ff71c21180bb4
commit-date: 2022-12-11
host: aarch64-apple-darwin
release: 1.68.0-nightly
LLVM version: 15.0.6
Backtrace
<backtrace>
related to : #106203
Activity
jschwe commentedon Dec 28, 2022
x is never dropped though - it is leaked.
The Box is wrapped in ManuallyDrop. This does not mean that the value is dropped! Instead, it means that the value will not be dropped automatically by the compiler. The destructor not running seems like the intended behaviour to me.
compiler-errors commentedon Dec 28, 2022
Hm, seems a bit inconsistent if we compare to the
into_rawimplementation, which does drop the allocator: https://doc.rust-lang.org/src/alloc/boxed.rs.html#1057fuyufjh commentedon Dec 29, 2022
https://doc.rust-lang.org/src/alloc/boxed.rs.html#197-200
Yeah, obviously the destructor of value (
T) should not be called, but how about the memory allocator (A)? Especially in cases of a customized allocator wrapped inArc.yuhao-su commentedon Dec 29, 2022
Sorry for using the wrong phrase
drop. What I want to say isleakask the compiler not to automatically drop the Boxed value and the allocator, but it only returns the pointer to the value while the allocator is ignored. Maybe we need something likeleak_with_allocator?thomcc commentedon Dec 29, 2022
I think this failing to drop the allocator is pretty surprising and is inconsistent with most uses of
Box::leak.Really, it's hard for me to see a case where this is the desired behavior...
BugenZhao commentedon Dec 29, 2022
While
Vec::into_raw_partsdoes not drop the allocator as well. 🤯 I guess we may need to clarify the semantics of theseinto_raws when the allocator API is supported.https://doc.rust-lang.org/stable/src/alloc/vec/mod.rs.html#818
jschwe commentedon Dec 29, 2022
In case of a multi-tiered allocator approach, where the last tier is something super simple, e.g. a bump allocator, I could see a use case where a child allocator returns the memory owned by itself to a parent allocator when its last instance is dropped. This obviously would be unsound if allocator instances can be dropped when leaking memory.
The Safety comment of the allocator trait says (emphasis mine):
The safety comment could of course be adjusted to put the onus on the implementor to check for any leaks when the alst instance is dropped, but that prevents very simple allocator designs, since some book-keeping is needed.
thomcc commentedon Jul 4, 2023
So, I think I was completely mistaken here. Or rather, I think it is surprising behavior, and I think it will usually not be desired, but it's also probably the correct behavior if we're going to offer this function for
Box<T, A>.My guess is that there are going to be plenty of
As which areGlobal-like (that is, ZSTs with no drop) where this causes no problems, thatBox::leakis worth having, but we should probably call out this behavior in the documentation with some sort of caveat.Change `{Box,Arc,Rc,Weak}::into_raw` to only work with `A = Global`
Amanieu commentedon May 18, 2025
The correct behavior is actually not to leak the allocator, otherwise
leakis unsound. This is because theAllocatortrait only guarantees that allocations remain valid until the allocator is dropped.#141219 moves
into_rawto only be usable with the global allocator, requiring the use ofinto_raw_with_allocwhen using custom allocators. This avoids any confusing by requiring users ofinto_rawto always explicitly handle the allocator.Since this is behaving as intended, I'm going to close this issue.