Skip to content

Rewrite safety requirements for Allocator impls#157801

Open
theemathas wants to merge 2 commits into
rust-lang:mainfrom
theemathas:allocator-safety-rewrite
Open

Rewrite safety requirements for Allocator impls#157801
theemathas wants to merge 2 commits into
rust-lang:mainfrom
theemathas:allocator-safety-rewrite

Conversation

@theemathas

@theemathas theemathas commented Jun 12, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

View all comments

This PR supersedes #156544.

cc #157428, #156920

cc @nia-e

r? @Amanieu (reviewer of #156544)

I define the concept of "equivalent allocators", in order to be able to talk about cloning allocators, and to give commonsense guarantees about stdlib Allocator impls.

I define the concept of "invalidating a memory block" in order to be able to talk about users not being allowed to use a block of allocated memory after its allocator is "gone".

An Allocator implementation is now allowed to invalidate its allocations when the allocator is mutated or when a lifetime in the allocator type expires.

Mutation of an Allocator should sensibly be allowed to invalidate its allocations. For example, the bumpalo crates has a Bump::reset method that takes &mut self and invalidates all past allocations. Accesses via & still must not invalidate past allocations since, for example, Box provides & access to the allocator.

I still had the "allocator destructor" clause as a separate clause from the &mut clause, to avoid questions about whether drop glue of types that don't implement Drop but have fields that implement Drop counts as creating a &mut to the whole thing.

The "lifetime expiry" clause closes a hole/ambiguity on when an allocator is considered to be "dropped" if it does not have a destructor. Additionally, this clause matches what is required for Box::into_pin and {Rc, Arc}::pin to be sound. (Those methods have an A: 'static bound to prevent allocating via a &MyAllocator and then running MyAllocator's destructor.)

@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 Jun 12, 2026
@theemathas theemathas mentioned this pull request Jun 12, 2026
Draft
@theemathas theemathas added A-allocators Area: Custom and system allocators T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 12, 2026
theemathas
theemathas commented Jun 12, 2026
View reviewed changes
Comment thread library/core/src/alloc/mod.rs
Comment on lines +152 to +155
/// until that memory block is [*invalidated*]. The implementor must also
/// not violate this invariant of `Allocator` via allocator equivalences
/// that are in the implementor's control (e.g., via a misbehaving
/// `impl Clone for Box<MyAllocator>`).

@theemathas theemathas Jun 12, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Any ideas on how to better word this?

View changes since the review

@theemathas
Rewrite safety requirements for Allocator impls
d9b90b0 
I define the concept of "equivalent allocators", in order to be able
to talk about cloning allocators, and to give commonsense guarantees
about stdlib `Allocator` impls.

I define the concept of "invalidating a memory block" in order to be
able to talk about users not being allowed to use a block of allocated
memory after its allocator is "gone".

An `Allocator` implementation is now allowed to invalidate its
allocations when the allocator is mutated or when a lifetime in the
allocator type expires.

Mutation of an `Allocator` should sensibly be allowed to invalidate its
allocations. For example, the `bumpalo` crates has a `Bump::reset`
method that takes `&mut self` and invalidates all past allocations.
Accesses via `&` still must not invalidate past allocations since,
for example, `Box` provides `&` access to the allocator.

I still had the "allocator destructor" clause as a separate clause from
the `&mut` clause, to avoid questions about whether drop glue of types
that don't implement `Drop` but have fields that implement `Drop` counts
as creating a `&mut` to the whole thing.

The "lifetime expiry" clause closes a hole/ambiguity on when an
allocator is considered to be "dropped" if it does not have a
destructor. Additionally, this clause matches what is required for
`Box::into_pin` and `{Rc, Arc}::pin` to be sound. (Those methods have an
`A: 'static` bound to prevent allocating via a `&MyAllocator` and then
running `MyAllocator`'s destructor.)
@theemathas theemathas force-pushed the allocator-safety-rewrite branch from 2861839 to d9b90b0 Compare June 12, 2026 12:23
@nia-e

This comment was marked as resolved.

Sign in to view
@theemathas

This comment was marked as resolved.

Sign in to view
@theemathas

This comment was marked as resolved.

Sign in to view
nia-e
nia-e reviewed Jun 12, 2026
View reviewed changes
Comment thread library/core/src/alloc/mod.rs
Comment thread library/core/src/alloc/mod.rs
Comment thread library/core/src/alloc/mod.rs Outdated
Comment thread library/core/src/alloc/mod.rs Outdated
Comment thread library/core/src/alloc/mod.rs Outdated
Comment thread library/core/src/alloc/mod.rs
Comment thread library/core/src/alloc/mod.rs
Comment thread library/core/src/alloc/mod.rs Outdated
Comment thread library/core/src/alloc/mod.rs Outdated
@nia-e

This comment was marked as resolved.

Sign in to view
@theemathas

theemathas commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Talked in DMs with @nia-e, and we agreed that we should use a symmetric relationship, since that is easier to explain to users. If a library needs an asymmetric relationship, they can define their own concept with their own name, and that library can add that extra promise for their allocators on top of the requirements of Allocator.

@theemathas theemathas added needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. and removed needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. labels Jun 12, 2026
clarfonthey
clarfonthey reviewed Jun 12, 2026
View reviewed changes
Comment thread library/core/src/alloc/mod.rs
Comment on lines +90 to +92
/// Note: Currently, the interaction between cloning and unsize-coercing allocators
/// is unsound, and there is ongoing discussion on how to revise the `Allocator` trait
/// to fix this. See [#156920].

@clarfonthey clarfonthey Jun 12, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Personally, I would be in favour of just incorporating AllocatorEq and AllocatorClone into whichever PR does these wording changes, since that fixes the problem and feels like a more reasonable way to do it.

That way, we require implementing those two traits to prove sound equivalence.

View changes since the review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clarfonthey clarfonthey clarfonthey left review comments

@nia-e nia-e nia-e approved these changes

Assignees

@Amanieu Amanieu

Labels

A-allocators Area: Custom and system allocators S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@theemathas @nia-e @clarfonthey @Amanieu @rustbot