Repository navigation
Abort on some large allocation requests, Panic on other #26951
Description
Compare these two:
let v = vec![0u8; !0];
let u = vec![0u16; !0];
We request a vector with 18446744073709551615 elements.
- For
u8we receive out of memory (null) from the allocator, and callalloc::oom, which aborts the program:application terminated abnormally with signal 4 (Illegal instruction) - For
u16, we get an assertion:thread '<main>' panicked at 'capacity overflow', ../src/libcore/option.rs:330
This is inconsistent. Why don't we abort on both of these cases? We abort on too large allocation requests, so those that are even larger could abort too?
Activity
arielb1 commentedon Jul 11, 2015
@bluss
For the record, your code is the equivalent of
In the
u16case, the amount of memory that is supposed to be allocated is2*(2^64 - 1), which exceeds the size ofusize. That is checked for rather explicitly inVec::with_capacity. Maybe we should abort instead of panicking in that case too.bluss commentedon Jul 11, 2015
Additional info: already
!0bytes is more than we can safely allow allocating (due to known fact #22104). We should treat that as capacity overflow, but decline to do so, because it will abort anyway.steveklabnik commentedon Jul 16, 2015
/cc @rust-lang/compiler , I think?
Gankra commentedon Jul 16, 2015
This is a @rust-lang/libs problem.
I am of the opinion that aborts should only occur on problems that unwinding can't address. That is, if you overflow the capacity we have to check for that, and we can panic right away. Everything will be cleaned up and the world will be happy.
However if an allocation makes it past our checks and then fails in the allocator, we have historically conservatively assumed that you are legitimately OOM and abort; unwinding can lead to arbitrary other allocations which is presumably bad to do during OOM.
In practice, the platforms we support basically can't OOM naturally (by which I mean, you're asking for the last few bits of free memory), so I reckon if you ever managed to trigger an OOM it's because you managed to request All The Memory, in which case there's plenty of slack space for unwinding. If it's a legit OOM then we'll double panic and crash anyway.
However #14674 mentions that aborting on OOM enables LLVM to better optimize around allocations. I don't know the details.
Edit: Regardless, I do not want to add any kind of "heuristic checks" for "will probably OOM" or "was probably a real OOM".
Aatch commentedon Jul 17, 2015
I think that, ultimately, abort vs. unwind behaviour should be left up to the implementation, but the standard library should abort on failed allocations. This particular example is just a red herring. It's only because
u8is a single byte that it doesn't trigger any kind of arithmetic overflow behaviour. It's an interesting quirk, but doesn't really reflect any real inconsistent behaviour.Gankra commentedon Jul 17, 2015
Also to be completely clear to those not super familiar with the issue: the "odd" behaviour described in this issue is completely by design, and not a mistake. Rather this issue is positing that the behaviour should be changed to be more consistent.
bluss commentedon Jul 24, 2015
Of course. I don't think it's completely by design, it's patched up to be this way. For example, if we know a capacity of
> isize::MAXis always too large, why does that abort and not panic?Gankra commentedon Jul 24, 2015
@bluss any time we detect this, I believe we panic. Only
alloc(..) == nullaborts to my knowledge: https://github.com/rust-lang/rust/blob/master/src/liballoc/raw_vec.rsWe simply don't even check in some cases where we know any degeneracies will be caught by the allocator (e.g. when doubling capacity on 64-bit).
Gankra commentedon Jul 24, 2015
Note that checking for
> isize::MAXon 64-bit is almost surely useless since your system will OOM you far before that point using normal growth. If you explicit ask for> isize::MAXthen you're basically arbitrarily "lucky". It's almost inconsistent to check, in that regard.nagisa commentedon Jul 24, 2015
That’s what they said about 32 bits timestamps and addressing too.
Gankra commentedon Jul 24, 2015
@nagisa Today this is simply a hard hardware issue: you only get 48 bits of the address space.
nagisa commentedon Jul 24, 2015
Yes, but it is always useful to be aware of future prospects.
Gankra commentedon Jul 24, 2015
@nagisa I have thankfully architected the current design to make such an upgrade trivial. In fact, all you have to do is delete some code!
https://github.com/rust-lang/rust/blob/master/src/liballoc/raw_vec.rs#L445-L453
steveklabnik commentedon Aug 4, 2015
So is this not a bug?
Gankra commentedon Aug 4, 2015
It's not an accident, at least.
On Tue, Aug 4, 2015 at 2:54 PM, Steve Klabnik notifications@github.com
wrote:
27 remaining items
lilith commentedon Aug 28, 2016
@pythonesque Why? If it's not possible to deal with OOM in Rust, I can't use Rust. See https://internals.rust-lang.org/t/could-we-support-unwinding-from-oom-at-least-for-collections/3673/21 for an exploration of why handling OOM is important.
eddyb commentedon Aug 28, 2016
@nathanaeljones IMO the real problem here is that we don't have an allocator abstraction in
libstdyet.steveklabnik commentedon Aug 29, 2016
@nathanaeljones the default, std allocator aborts, but given Rust's low-level nature, you could replace it with one that doesn't. It is a lot more work right now, but as @eddyb said, this is something that will come along in time.
lilith commentedon Aug 29, 2016
@steveklabnik Thanks; I'm not too concerned with 'lots of work', but rather if I can produce well-behaved binaries for both FFI and server use.
I started porting Imageflow over to Rust in June, but failed to verify that OOM panics (as widely reported) instead of aborting. I'd assumed that the stabilization of
std::panic::catch_unwindmeant Rust was ready for my use case. I decided to write a test today and, well, panicked.It's very unclear to me where the anti-OOM-panic sentiment arises from. Double panics abort.
I've been trying to track down blockers around this, but haven't been able to discover the reasons why movement on graceful OOM handling has been slow. I think I ?might? have found the right issue to discuss: #27700 (comment)
steveklabnik commentedon Dec 25, 2019
Triage: still blocked on a libs team decision.
ehsanul commentedon May 11, 2020
Looks like there is now an unstable
alloc_error_handlerfeature gate. Tracking issue: #51540Mentioned in that issue, there is also an accepted RFC to set a cargo attribute for
oom=panic, tracking issue: #43596And another issue to make the default for
handle_alloc_errorpanic, which has survived the FCP with disposition to merge: #66741Amanieu commentedon Sep 22, 2021
We discussed this in the Libs API meeting today: the consensus was that we should pipe "invalid layout" errors through to
alloc_error_handlerby making it take aenumwith eitherAllocErrandLayoutErr. This would be similar to the existing unstableTryReserveErrortype, and in fact we could reuse the same type in both places.