allocator: refactor for stabilisation by nia-e · Pull Request #157428 · rust-lang/rust

nia-e · 2026-06-04T11:47:58Z

Adds my current proposal per the doc in #156882 and follow-up Zulip conversations (notably for dyn-compat) unstably.

r? libs

nia-e · 2026-06-04T14:08:19Z

Note that the no-panic bounds introduced close #156490 and #155746. However, if we want to relax them in the future, we may need to adjust our collection types to be more resilient.

clarfonthey · 2026-06-04T18:36:38Z

+///  - the allocator is mutated through public API taking `&mut` access (notably,
+///    running the allocator's destructor is such a mutation), or


This guarantee seems fine on the surface, but I'm trying to wrap my head around what's actually being guaranteed here. Like, clearly, it'd be wildly unsafe to offer an invalidate_everything method on an allocator that just deletes the backing memory without requiring any of the things that are using it to be dropped, but this feels like it's opening the door for that kind of method "as long as you're careful" which, doesn't make a lot of sense.

Like, I'm trying to gauge what value is being gained by this guarantee and it mostly just feels like it's making things more confusing without actually helping.

View changes since the review

we're saying that such an invalidate_everything method is allowed to exist, and you can't rely on the allocator having not yet been dropped for soundness. in other words, so long as you hold a &alloc (thus preventing a &mut alloc from being created), you can trust the memory you have is fine; but if you lose the &alloc and get a new one back, your memory might have been scribbled over and you must act as such

Okay, but wouldn't that be the same thing as the lifetime expiring as before? Technically, even though both of them are written as &A, you've gotten a new &A lifetime in that case.

i think the extra guarantee here is that if you do hold a &mut alloc, you can call methods that take alloc by-shared-ref without worry but you can't pass the actual &mut to an untrusted function and expect your allocator to be okay at the end. but i agree that's not obviously guaranteed

Isn't that just totally breaking the aliasing guarantees, though? Since that &mut reference wouldn't be unique.

...you know, you make a good point. i'll revisit the reasoning for this, i recall adding this in response to something being brought up

nvm, i'm being stupid. the following is the reason:

let mut alloc = SomeAllocator::new(); let ptr = alloc.allocate(...); alloc.something_by_shared_ref(); // ptr is still guaranteed to be valid alloc.trusted_method_by_unique_ref(); // ptr is still valid because we know for sure the method is trusted not to mess w/ allocator state alloc.untrusted_method_by_unique_ref(); // ptr must be assumed to be maybe-invalid even if the lifetime of alloc is not expired and ptr hasn't yet been deallocated

I guess that I was technically thinking of Box whose lifecycle is intrinsically tied to the lifetime of the allocator parameter, whereas in this case if you just call alloc and dealloc manually the "lifetime" is not really tracked at all. So, yes, mutable borrows can happen on the allocator and it's fine, and you guarantee this doesn't happen by taking a non-mutable borrow.

Seeing this thread from @RalfJung: #157428 (comment)

I think we probably also need to be careful about how we define the relationship between these rules and StaticAllocator, since "lifetime expiration" in those cases refers to the allocator value and not references in that case.

clarfonthey · 2026-06-04T19:19:49Z

@rustbot author

(mostly so you can more clearly signal when you think things are ready; I've commented here already so I'll see any additional changes for review as they're made)

nia-e · 2026-06-04T19:48:25Z

+///
+/// [`Pin`]: ../../core/pin/struct.Pin.html
+#[unstable(feature = "allocator_api", issue = "32838")]
+pub unsafe trait StaticAllocator: Allocator {}


I dropped the 'static bound here (and thus implicitly in Box::pin_in, etc.); since this trait is about being able to be lifetime-subtyped safely, it would mean that you need to be able to coerce to a StaticAllocator + 'a so the whole guarantee about "this is Actually Static I Promise" has weight. cc @rust-lang/opsem in case i did a bad here

View changes since the review

That's more of a @rust-lang/types question

maxdexh · 2026-07-06T18:20:32Z

If AllocatorClone requires that clone does not panic, then what happens when Box::<A>::clone fails and calls handle_alloc_error? handle_alloc_error is allowed to unwind, so the impl AllocatorClone for Box seems incorrect. More generally, I feel like this is a huge footgun.

clarfonthey · 2026-07-06T18:38:49Z

handle_alloc_error is explicitly not allowed to unwind, or at least, that is the plan for stabilisation.

maxdexh · 2026-07-06T18:42:50Z

There is currently no way for it to unwind, but that may change with features like alloc_error_hook, which is independent from allocator_api in my eyes?

Just to be clear: I mean the global handle_alloc_error function, which is used generically to handle allocation errors, and specifically states in its documentation that it may unwind.

clarfonthey · 2026-07-06T18:54:56Z

Yeah, but I believe that it's been deemed unsound if handle_alloc_error unwinds. There may be support for this in the future but the status quo is that it always aborts on stable.

See #51245

maxdexh · 2026-07-06T19:03:13Z

That is the status quo, but that only matters to the standard library. Noone else can rely on this; a downstream user who wants to allocate in the clone of their allocator must not call handle_alloc_error, as that method specifically states that it may unwind, and AllocatorClone has a safety requirement to never unwind.

That is why I consider this a huge footgun.

@nia-e

…e, r=Amanieu Rewrite safety requirements for `Allocator` impls This PR supersedes rust-lang#156544. cc rust-lang#157428, rust-lang#156920 cc @nia-e r? @Amanieu (reviewer of rust-lang#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.)

@nia-e

…e, r=Amanieu Rewrite safety requirements for `Allocator` impls This PR supersedes rust-lang#156544. cc rust-lang#157428, rust-lang#156920 cc @nia-e r? @Amanieu (reviewer of rust-lang#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.)

@nia-e

…e, r=Amanieu Rewrite safety requirements for `Allocator` impls This PR supersedes rust-lang#156544. cc rust-lang#157428, rust-lang#156920 cc @nia-e r? @Amanieu (reviewer of rust-lang#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.)

rust-bors · 2026-07-08T00:03:26Z

☔ The latest upstream changes (presumably #158924) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

@nia-e

Rollup merge of #157801 - theemathas:allocator-safety-rewrite, r=Amanieu Rewrite safety requirements for `Allocator` impls 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.)

nia-e · 2026-07-08T14:33:16Z

Allocators themselves should not use handle_alloc_error internally. Callers are free to do so, though, if unwinding is fine. I don't believe this is an issue so long as docs are clear

maxdexh · 2026-07-08T15:00:28Z

The discussion was about clone calling handle_alloc_error. Such allocators calling it in their clone already exist, namely Box<A>.

clarfonthey · 2026-07-08T15:17:17Z

That might be a valid reason to create custom methods for allocator cloning instead of using Clone, which is one alternative I provided. We could make allocator cloning explicitly falliable to ensure that errors when allocating clones of allocators are explicitly handled without unwinding.

maxdexh · 2026-07-08T15:34:13Z

I think the restriction on panicking should be lifted regardless of the mechanism used to duplicate allocators.
IIUC, the restriction is just because of some badly written std code?

RalfJung · 2026-07-08T16:06:32Z

The fact that even std, which has a pretty high review bar, is unsound under panicking allocation indicates that this is too high a bar for the entire ecosystem to uphold, and we'd be better off saying that allocation must never panic.

maxdexh · 2026-07-08T16:11:02Z

Well, the code in Arc::make_mut also has a memory leak when using allocators, as well as having the same panic-safety issue with handle_alloc_error. It is very clear that the method was written with very different assumptions originally and that allocators were kind of crammed into it (the affected branch has 3 bugs in 4 lines, see #158876).

The other one was in BTreeMap, but idk enough about the code there.

maxdexh · 2026-07-08T16:19:56Z

More generally, code that deals with allocations, unless it uses std::alloc::alloc directly, is already calling into std code that can panic through handle_alloc_error, or is calling it in its abstractions, like Arc is.

nia-e · 2026-07-08T16:33:19Z

i see. in that case i'd nominate the tracking issue for alloc_error_hook for discussion on whether we want to add the requirement that it never unwind (and, implicitly, add that guarantee to handle_alloc_error). there was some discussion about this on zulip already i believe

RalfJung · 2026-07-08T16:36:05Z

Note that there's also the default error hook defined in liballoc, which panics. This can obviously unwind, and it can even invoke arbitrary user-defined code via a custom #[panic_handler]. Maybe that should be changed to a non-unwinding panic.

RalfJung · 2026-07-08T16:46:04Z

Never mind, that is already using a non-unwinding panic.

rust/library/alloc/src/alloc.rs

Lines 572 to 577 in 4294891

    
           pub unsafe fn __rdl_alloc_error_handler(size: usize, _align: usize) -> ! { 
        
               core::panicking::panic_nounwind_fmt( 
        
                   format_args!("memory allocation of {size} bytes failed"), 
        
                   /* force_no_backtrace */ false, 
        
               ) 
        
           }

And setting a custom #[alloc_error_handler] is unstable. So we should be good on that front.

maxdexh · 2026-07-08T17:11:12Z

What about allocators panicking in their drop?

clarfonthey · 2026-07-08T19:25:25Z

Never mind, that is already using a non-unwinding panic.

Note: this is why I mentioned that the status quo is always an abort, since libstd uses this handler as well.

What about allocators panicking in their drop?

I could have sworn that panics in destructors were always converted to unconditional aborts, but I can't find any link in the reference that says this. At least, panics in destructors are risky because they're run during unwinding, and a panic while unwinding is always an abort.

There is pretty strong justification for wanting an unwinding alloc_error_handler because it allows recovering from OOM conditions in some cases, but it also comes with a lot of potential footguns, at least similar to mutex poisoning and the like.

RalfJung · 2026-07-08T19:27:52Z

I could have sworn that panics in destructors were always converted to unconditional aborts, but I can't find any link in the reference that says this. At least, panics in destructors are risky because they're run during unwinding, and a panic while unwinding is always an abort.

The RFC didn't make it (yet): rust-lang/rfcs#3288

RalfJung · 2026-07-08T19:28:34Z

There is pretty strong justification for wanting an unwinding alloc_error_handler because it allows recovering from OOM conditions in some cases, but it also comes with a lot of potential footguns, at least similar to mutex poisoning and the like.

I think the way to deal with that is to use APIs that don't call alloc_error_handler, like Vec::try_reserve. "unwind + catch" is not an error handling strategy we usually endorse in Rust.

maxdexh · 2026-07-08T19:33:55Z

I could have sworn that panics in destructors were always converted to unconditional aborts, but I can't find any link in the reference that says this.

I keep mistakenly thinking the same thing, even though I have used panicking destructors for soundness holes in crates in the past. This is one of the biggest footguns in unsafe code; you need to make sure that you do not drop values of generic type unless you can deal with an unwind.

maxdexh · 2026-07-08T19:39:31Z

And in my PR to fix Arc::make_mut, I fell for this as well before noticing it. I think allocators are uniquely susceptible to this because they introduce owned generics into existing unsafe code when they are added to code bases.

clarfonthey · 2026-07-08T19:39:43Z

I think the way to deal with that is to use APIs that don't call alloc_error_handler, like Vec::try_reserve. "unwind + catch" is not an error handling strategy we usually endorse in Rust.

While I agree, I also know that catch+unwind is exactly how threads work, and this is the basis for why async runtimes like tokio also depend on this model.

I would imagine that most cases that depend on catch+unwind here are not planning to actually mitigate the error, but require a bare-minimum cleanup in the error handler, like saving some important data or logging the state during the issue. You could argue that these things are best put in the alloc error handler, but since it's hard to ensure a program is panic-free without some cursed linker shenanigans or daemonic rituals, folks might just argue for being able to catch the allocation error instead, with the caveat that any issues during unwinding still cause an abort anyway.

maxdexh · 2026-07-08T19:41:10Z

Some methods also don't have try_ equivalents, especially ones from traits, like clone and extend

rustbot assigned clarfonthey Jun 4, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 4, 2026

nia-e added the A-allocators Area: Custom and system allocators label Jun 4, 2026

Urgau reviewed Jun 4, 2026

View reviewed changes

Comment thread library/core/src/alloc/mod.rs Outdated

This comment has been minimized.

Sign in to view

This comment was marked as resolved.

Sign in to view

This comment has been minimized.

Sign in to view