Skip to content

BTreeMap: Don't leak allocators when initializing nodes#146859

Merged
bors merged 1 commit intorust-lang:masterfrom
cammeresi:btree-alloc-20250920
Sep 25, 2025
Merged

BTreeMap: Don't leak allocators when initializing nodes#146859
bors merged 1 commit intorust-lang:masterfrom
cammeresi:btree-alloc-20250920

Conversation

@cammeresi
Copy link
Contributor

@cammeresi cammeresi commented Sep 21, 2025 •
edited by rustbot
Loading

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).

Fixes: #106203

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 21, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 21, 2025

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

joboet
joboet requested changes Sep 24, 2025
View reviewed changes
Copy link
Member

@joboet joboet left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks correct, but could you add a comment explaining the allocator behaviour you outlined in the PR description?

View changes since this review

@joboet joboet assigned joboet and unassigned ibraheemdev Sep 24, 2025
@cammeresi
BTreeMap: Don't leak allocators when initializing nodes
137d4ea 
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).
@joboet
Copy link
Member

joboet commented Sep 25, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 25, 2025

📌 Commit 137d4ea has been approved by joboet

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 25, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 25, 2025
@matthiaskrgr
Rollup merge of rust-lang#146859 - cammeresi:btree-alloc-20250920, r=…
468d12f 
…joboet

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).

Fixes: rust-lang#106203
This was referenced Sep 25, 2025
Closed
Merged
bors added a commit that referenced this pull request Sep 25, 2025
@bors
Auto merge of #147037 - matthiaskrgr:rollup-xtgqzuu, r=matthiaskrgr
7ac0330 
Rollup of 8 pull requests

Successful merges:

 - #116882 (rustdoc: hide `#[repr]` if it isn't part of the public ABI)
 - #135771 ([rustdoc] Add support for associated items in "jump to def" feature)
 - #141032 (avoid violating `slice::from_raw_parts` safety contract in `Vec::extract_if`)
 - #142401 (Add proper name mangling for pattern types)
 - #146293 (feat: non-panicking `Vec::try_remove`)
 - #146859 (BTreeMap: Don't leak allocators when initializing nodes)
 - #146924 (Add doc for `NonZero*` const creation)
 - #146933 (Make `render_example_with_highlighting` return an `impl fmt::Display`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 83cf8f9 into rust-lang:master Sep 25, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Sep 25, 2025
rust-timer added a commit that referenced this pull request Sep 25, 2025
@rust-timer
Unrolled build for #146859
04136f6 
Rollup merge of #146859 - cammeresi:btree-alloc-20250920, r=joboet

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).

Fixes: #106203
@cammeresi cammeresi deleted the btree-alloc-20250920 branch September 26, 2025 00:02
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Sep 26, 2025
@bors
Auto merge of #147037 - matthiaskrgr:rollup-xtgqzuu, r=matthiaskrgr
3333c7e 
Rollup of 8 pull requests

Successful merges:

 - rust-lang/rust#116882 (rustdoc: hide `#[repr]` if it isn't part of the public ABI)
 - rust-lang/rust#135771 ([rustdoc] Add support for associated items in "jump to def" feature)
 - rust-lang/rust#141032 (avoid violating `slice::from_raw_parts` safety contract in `Vec::extract_if`)
 - rust-lang/rust#142401 (Add proper name mangling for pattern types)
 - rust-lang/rust#146293 (feat: non-panicking `Vec::try_remove`)
 - rust-lang/rust#146859 (BTreeMap: Don't leak allocators when initializing nodes)
 - rust-lang/rust#146924 (Add doc for `NonZero*` const creation)
 - rust-lang/rust#146933 (Make `render_example_with_highlighting` return an `impl fmt::Display`)

r? `@ghost`
`@rustbot` modify labels: rollup
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Oct 9, 2025
@matthiaskrgr
Rollup merge of rust-lang#146859 - cammeresi:btree-alloc-20250920, r=…
d244aa7 
…joboet

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).

Fixes: rust-lang#106203
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Oct 9, 2025
@bors
Auto merge of rust-lang#147037 - matthiaskrgr:rollup-xtgqzuu, r=matth…
0915c6f 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#116882 (rustdoc: hide `#[repr]` if it isn't part of the public ABI)
 - rust-lang#135771 ([rustdoc] Add support for associated items in "jump to def" feature)
 - rust-lang#141032 (avoid violating `slice::from_raw_parts` safety contract in `Vec::extract_if`)
 - rust-lang#142401 (Add proper name mangling for pattern types)
 - rust-lang#146293 (feat: non-panicking `Vec::try_remove`)
 - rust-lang#146859 (BTreeMap: Don't leak allocators when initializing nodes)
 - rust-lang#146924 (Add doc for `NonZero*` const creation)
 - rust-lang#146933 (Make `render_example_with_highlighting` return an `impl fmt::Display`)

r? `@ghost`
`@rustbot` modify labels: rollup
Kobzol pushed a commit to Kobzol/rustc_codegen_gcc that referenced this pull request Dec 21, 2025
@bors
Auto merge of #147037 - matthiaskrgr:rollup-xtgqzuu, r=matthiaskrgr
968693f 
Rollup of 8 pull requests

Successful merges:

 - rust-lang/rust#116882 (rustdoc: hide `#[repr]` if it isn't part of the public ABI)
 - rust-lang/rust#135771 ([rustdoc] Add support for associated items in "jump to def" feature)
 - rust-lang/rust#141032 (avoid violating `slice::from_raw_parts` safety contract in `Vec::extract_if`)
 - rust-lang/rust#142401 (Add proper name mangling for pattern types)
 - rust-lang/rust#146293 (feat: non-panicking `Vec::try_remove`)
 - rust-lang/rust#146859 (BTreeMap: Don't leak allocators when initializing nodes)
 - rust-lang/rust#146924 (Add doc for `NonZero*` const creation)
 - rust-lang/rust#146933 (Make `render_example_with_highlighting` return an `impl fmt::Display`)

r? `@ghost`
`@rustbot` modify labels: rollup
Kobzol pushed a commit to Kobzol/rustc_codegen_cranelift that referenced this pull request Dec 29, 2025
@bors
Auto merge of #147037 - matthiaskrgr:rollup-xtgqzuu, r=matthiaskrgr
7e272c0 
Rollup of 8 pull requests

Successful merges:

 - rust-lang/rust#116882 (rustdoc: hide `#[repr]` if it isn't part of the public ABI)
 - rust-lang/rust#135771 ([rustdoc] Add support for associated items in "jump to def" feature)
 - rust-lang/rust#141032 (avoid violating `slice::from_raw_parts` safety contract in `Vec::extract_if`)
 - rust-lang/rust#142401 (Add proper name mangling for pattern types)
 - rust-lang/rust#146293 (feat: non-panicking `Vec::try_remove`)
 - rust-lang/rust#146859 (BTreeMap: Don't leak allocators when initializing nodes)
 - rust-lang/rust#146924 (Add doc for `NonZero*` const creation)
 - rust-lang/rust#146933 (Make `render_example_with_highlighting` return an `impl fmt::Display`)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joboet joboet joboet requested changes

Assignees

@joboet joboet

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.92.0

Development

Successfully merging this pull request may close these issues.

Memory leaks in BTreeMap with allocator_api enabled

5 participants

@cammeresi @rustbot @joboet @bors @ibraheemdev