Tracking issue for `try_reserve`: RFC 2116 fallible collection allocation

Opened by Aaron Turon at 2024-03-13 14:23:10

This is a tracking issue for the RFC "fallible collection allocation" (rust-lang/rfcs#2116).

Steps:

[x] Implement the RFC #48648
[ ] Adjust documentation (see instructions on forge)
[ ] Stabilization PR (see instructions on forge)

This is a tracking issue for the try_reserve part of the RFC "fallible collection allocation" (rust-lang/rfcs#2116).

Steps:

[x] Implement the RFC #48648
- [x] Add HashSet::try_reserve: https://github.com/rust-lang/rust/pull/58623
[x] Finalize the error type
[ ] Adjust documentation (see instructions on forge)
[ ] Stabilization PR (see instructions on forge)

API:

impl /* each of String, Vec<T>, VecDeque<T> */ {
    pub fn try_reserve(&mut self, additional: usize) -> Result<(), TryReserveError> {…}
    pub fn try_reserve_exact(&mut self, additional: usize) -> Result<(), TryReserveError> {…}
}

impl /* each of HashMap<K, V> and HashSet<T> */ {
    pub fn try_reserve(&mut self, additional: usize) -> Result<(), TryReserveError> {…}
}

/// The error type for `try_reserve` methods.
#[derive(Clone, PartialEq, Eq, Debug)]
pub enum TryReserveError { // in std::collections
    /// Error due to the computed capacity exceeding the collection's maximum
    /// (usually `isize::MAX` bytes).
    CapacityOverflow,

    /// The memory allocator returned an error
    AllocError {
        /// The layout of allocation request that failed
        layout: Layout,

        #[doc(hidden)]
        #[unstable(feature = "container_error_extra", issue = "0", reason = "\
            Enable exposing the allocator’s custom error value \
            if an associated type is added in the future: \
            https://github.com/rust-lang/wg-allocators/issues/23")]
        non_exhaustive: (),
    },
}

impl From<LayoutErr> for TryReserveError {
    fn from(_: LayoutErr) -> Self {
        TryReserveError::CapacityOverflow
    }
}

Simon Sapin at 2018-06-27 12:42:31

Hi, this feature will require oom and try_reserve to be implemented. I'm needing try_reserve and I'm willing to work (slowly) on it if noone else does. I see that the previous commit by @Gankro is here and seems to match the RFC description. Is there anything else I should consider before rebasing that PR?
Sebastian N. Fernandez at 2018-02-22 15:15:32
I can use some mentorship on what's the best way to implement oom=panic/abort . Should I emit a global variable through rustc depending on this case?

I was taking inspiration from https://github.com/rust-lang/rust/pull/32900 but there might be an easier way to emit a boolean global variable.
Sebastian N. Fernandez at 2018-03-03 23:19:22
cc @Gankro, can you help mentor?
Aaron Turon at 2018-03-05 14:06:34
try_reserve methods are implemented in Nightly, but the accepted RFC also includes the ability to make allocation failure cause a panic rather than abort and that part is not implemented. Unfortunately the RFC does not discuss how this would be implemented, other than being controlled by a flag in Cargo.toml. It’s not clear to me how this can work.
Simon Sapin at 2018-04-05 07:32:27
Unfortunately the RFC does not discuss how this would be implemented, other than being controlled by a flag in Cargo.toml. It’s not clear to me how this can work.

We've been talking about panic::set_hook-like hooking for the new oom lang item in #49668, this could be how this works.
Mike Hommey at 2018-05-04 05:20:52
@alexcrichton, how does https://github.com/rust-lang/rust/pull/51041 fit with the goal of implementing some way to opt into oom=panic?
Simon Sapin at 2018-05-29 14:16:51
@SimonSapin we'd decide that the regression is acceptable, and we'd revert the change.
Alex Crichton at 2018-05-29 16:43:24
You use a conditional tense, but oom=panic is already in an accepted RFC (though the implementation strategy is not quite settled).
Simon Sapin at 2018-05-29 17:35:45
Ok sure, we can figure it out in the implementation details? AFAIK there's no need to revert the change as it has basically no benefit today. It's a trivial change to revert as well, it's one line in the standard library
Alex Crichton at 2018-05-29 17:44:06
This example in playground asks for 1024*1024*1024*1000 bytes on a server, via v.try_reserve(len)? without giving an error.

Is this RFC implementation is not at the point, at which errors in such cases will start to show up? Or, is this a gross over-commit by underlying OS, and this state of affairs will stay like this? Can try_reserve in principle reserve only realistic things?
3n-mb at 2018-06-24 23:16:19
When an error is returned is entirely dependent of the underlying allocator. As far as I know Linux usually overcommits, Windows doesn’t.

https://play.rust-lang.org/?gist=aa668f38117983d1951b58307df04880&version=nightly allocates 120 PB but fails at 130 PB, it is exhausting 48 bits of address space.

Even on Linux though, this API can be useful for 32-bit programs when exhausting the address space is much more realistic.
Simon Sapin at 2018-06-24 23:44:41
https://github.com/rust-lang/rust/issues/43596 was the pre-RFC feature request for oom=panic. I’ve repurposed it as the tracking issue for that feature, since it and try_reserved are likely to be stabilized at different times. This leaves this issue as tracking only try_reserve.
Simon Sapin at 2018-06-27 12:43:57
https://github.com/rust-lang/rust/pull/52420 will change CollectionAllocErr to CollectionAllocErr<AllocError>
Sebastian N. Fernandez at 2018-08-01 14:45:27
Will, or would. I’ve comment there that such a change in public API design merits being discussed separately from a large PR.
Simon Sapin at 2018-08-01 15:43:37
Yeah definitely. Even if it weren't for the 2018 delaying considering these things, I wouldn't expect it to be merged quick. I just make the PR to prove the feasibility and make my ideas clearer to / interactive for others.

Ignoring the benefits of allocator polymorphism itself (that would be https://github.com/rust-lang/rust/issues/42774), I see 4 big benefits:
- Self discipline: Applications can prevent themselves statically from forgetting to handled allocations failures.
- Race freedom: allocation divergence due to try_reserving insufficient space are statically prevented.
- Expressiveness: Unclear how to do try_reserve-like things for "deep" data structures (those containing indirecation).
- Library flexibility: Implement functionality just once with a parameteric error type, and you get fallibility polymorphism: downstream can instantiate with ! to get today's divergence, or AllocError to allow them to be explicitly handled.
I thus advocating going with error polymorphism and abandoning try_reverve (I rather reuse try_ for the Result<(), E> copies of every method we'll need for backwards compat). (But, my PR keeps the existing meaning of try_reserve so as to allow a transition period where people can try both approaches.)
John Ericson at 2018-08-02 15:29:17
Firefox needs try_reserve badly enough that they forked Vec and HashMap over it. It’d be nice to stabilize.

Some of the discussion above is about oom=panic whose tracking has moved to https://github.com/rust-lang/rust/issues/43596. This leaves two unresolved question:
- Whether CollectionAllocErr should gain a type parameter, with the AllocErr variant holding a value of that type, in preparation for future allocation traits having an associated type for errors. I think we can still make that change after stabilization, if the type parameter has a default. @rust-lang/libs, does that sound accurate?
- @Ericson2314, it sounds like you’re objecting to having try_reserve at all, because instead we should have try_* variations of every existing method that allocates. However reserve is such a method so this proposal is a subset of yours, not contradictory. Am I missing something?
Simon Sapin at 2018-10-07 12:08:36
@SimonSapin Yes if things were done my way Firefox wouldn't be try_reserving or reserving at all, but just fallibly allocate the memory they need when they need it. That's much simpler and more robust.

(There's also the more minor quibble that under the simplest version of my plan, the Self types are different and try_reseve wouldn't be callable for the regular infallible collections. But that can be changed at the cost of more associated type magic.)

The libs team paused associated allocators to prioritize the 2018 edition. That's fine with me, but then I think it only fair that Firefox wait too. I promise as soon as the gates are open I'll rush to revive @glandium's and my PRs.
John Ericson at 2018-10-07 18:09:22
Right, try_insert for example would be preferable to try_reserve + insert, but it doesn’t exist yet. You know as well as I do that large changes or additions are harder to get through the process, https://github.com/rust-lang/rfcs/pull/2116 was designed deliberately to be minimal.

I think it only fair that Firefox wait too

It’s not about who gets candy first. RFC 2116 was submitted 14 months ago and accepted 8 months ago. The try_reserve part has been implemented 7 months ago, stabilizing it now can be a matter of flipping a switch. Allocator-generic collections however are still in the design phase with a number of open questions, they are still gonna take significant work from multiple people.

That said, there is no real urgency for try_reserve. Firefox using a forked HashMap is unfortunate in my opinion, but it works. Stabilizing try_reserve only seems to me like easy incremental improvement.
Simon Sapin at 2018-10-07 18:52:28
Well it sounds like we are in something closer to agreement than usual. :)

Allocator-generic collections however are still in the design phase with a number of open questions

The code has been written and if I recall correctly works (there was one ancillary issue with trying to make default impls work well, but it is not fundamental and I believe I found a workaround beyond just requiring more boilerplate). It just needs to be read and debated.
John Ericson at 2018-10-07 19:06:44
@SimonSapin it's been awhile since I took a look at this, but is the idea that (in the long run) every fallible method on a collection returns CollectionAllocErr<T> and the T is specialized per-method to be the "most appropriate"? In the meantime, though, we're only thinking of the try_reserve methods which would effectively return CollectionAllocErr<()> and you're thinking that we can add <T = ()> to the enum definition later?

One possible alternative to this is to avoid defining this type at all I think? For example unlike as we may have originally though AllocErr wasn't stabilized with global allocators. To that end it increases the surface area of what's to be stabilized to stabilize this feature, and it also means we're not necessarily sold on having AllocErr yet. These methods could, perhaps, return a bool for whether they succeeded or not. (this sort of depends if we think it's crucial to switch on why reservation failed, allocation and/or capacity)

I think I'd personally lean towards what you're thinking @SimonSapin by having CollectionAllocErr with a defaulted type parameter later on, if necessary. I might also advocate, as well, having either a non-exhaustive enum or an opaque struct with boolean accessors.
Alex Crichton at 2018-10-08 17:31:45
Oh, I forgot that AllocErr was not stable yet. I’m fine with changing CollectionAllocErr to an opaque type for now. (A struct with a private field, to leave us more options later.) Even zero-size, though I’d still prefer Result<(), _> over bool even if they are semantically equivalent.

At the moment I don’t really have an opinion on whether CollectionAllocErr should have a type parameter. I was only responding to https://github.com/rust-lang/rust/issues/48043#issuecomment-409600561 by saying we’ll still be able to do that later if we want. (With the RFC process hat on, to make sure possible objections are not ignored.)

This sort of depends if we think it's crucial to switch on why reservation failed, allocation and/or capacity

It looks like at least some of the code paths in Firefox track the size (and alignment) of the allocation request that failed, so the current enum might be insufficient anyway.

Alright, on further thought it may not make sense to pursue this just yet, until we figure out the rest of the story for allocation traits and error types.
Simon Sapin at 2018-10-08 18:24:23
@alexcrichton So in https://github.com/rust-lang/rust/pull/52420/files I did make a CollectionAllocErr, but that's not really the important part, the associated error type (which would be the parameter) is.

Honestly, I'm not even sure CollectionAllocErr is worth it. It creates two axis with the associated type: do we hide allocation errors, and do we hide size errors? I can't imagine a case where one should cause an panic/abort and the other shouldn't. I rather have just AllocErr as is, or have AllocErr have both variants (In the memory hierarchy we can imagine that at any layer some collection/allocator gets too big, or it cannot get memory from it's backing allocator).

All this another reason, and one I initially forgot (but now remember thinking of when I rebased my PR over try_reserve was added), that stabilizing try_reserve now will tie our hands later.
John Ericson at 2018-10-08 19:12:16
I think this is a feature that we need semi-urgently. There are projects out there that require try_reserve and this is not something that can be quickly replaced by a custom crate. I like the idea of CollectionAllocErr with a private field but I'd like to know if anyone can think of a limitation before proceeding in implementing it.
Sebastian N. Fernandez at 2018-10-21 17:08:12
As far as I can tell, the error type is the only remaining blocker. How about something like this?
```
pub struct ContainerAllocError {
    layout_of_failed_allocation: Option<Layout>,
}

impl ContainerAllocError {
    pub fn layout_of_failed_allocation(&self) -> Option<Layout> { self.layout_of_failed_allocation }
}
```
- A struct with private fields (and no public fields), so we could later add more fields or even turn it back into an enum
- No type parameter, but we can add one later (with a default) if the Alloc trait grows an associated type for errors and we want to propagate custom error values
- Container instead of Collection in the name, in case we want to add Box::try_new at some point.
  - Should it stay in std::collections, or move to another module? Which one?
- Can contain the Layout that was passed e.g. to the alloc() called that failed. Having this info (or at least the size) is a requirement for Firefox, but for example the allocation strategy of HashMap is an implementation detail that users should not have to guess at based on HashMap::capacity alone.
  - Name of the accessor method to be bikeshedded. Possibly just layout, with an explanation in the doc-comment.
- Speaking of bikesheds, *Err or *Error? We stabilized LayoutErr, but the precedent for types named FooError is much stronger in https://doc.rust-lang.org/std/error/trait.Error.html#implementors
Simon Sapin at 2019-06-11 12:55:31

Alternative definition:

pub enum ContainerAllocError {
    CapacityOverflow,
    AllocErr {
        layout: Layout,
        #[unstable(feature = "container_alloc_error_extra")] error: AllocErr,
    },
}

Having a public field for a zero-size type is a bit awkward, but the key idea is that it can stay unstable while the rest of the enum is stabilized (together with try_reserve methods that return it).

When https://github.com/rust-lang/wg-allocators/issues/23 is resolved, based on the outcome we either remove that field:

pub enum ContainerAllocError {
    CapacityOverflow,
    AllocErr { layout: Layout },
}

… or make its type be a new type parameter of the enum:

pub enum ContainerAllocError<E = AllocErr> {
    CapacityOverflow,
    AllocErr {
        layout: Layout,
        error: E,
    },
}

impl SomeContainer<T, A: Alloc> {
    pub fn try_reserve(n: usize) -> Result<(), ContainerAllocError<A::Err>> {…}
}

Simon Sapin at 2019-06-11 15:06:13

@SimonSapin and I disagree on whether it is superior, but my fallibility-polymorphism in https://github.com/rust-lang/rust/pull/60703 is almost ready (see how https://github.com/rust-lang/rust/pull/58457 was just rebased, I just rebased).
John Ericson at 2019-06-11 19:13:49
Oh, thanks @SimonSapin for mentioning my issue in the second comment I did not see that the first time sorry. For the record, #60703 goes with the pub enum ContainerAllocError<E = AllocErr> approach, which is more useful because ContainerAllocError<!> correctly ensures only CapacityOverflow is possible since all all allocation failures will panic/abort.
John Ericson at 2019-06-12 00:40:30
(At a risk of re-hashing arguments already made elsewhere, my view is that when panic/abort on allocation error is desired there is no need for try_reserve over reserve. So ContainerAllocError<!> is not a useful construct IMO. But the proposed unstable field to force non-exhaustive matching leaves the door open either way, so this doen’t need to be decided in this thread.)
Simon Sapin at 2019-06-12 16:55:10
PR for the above: https://github.com/rust-lang/rust/pull/61780
Simon Sapin at 2019-06-12 17:13:06
The PR takes into account my concerns and so looks fine to me. Layout is a good idea.

@SimonSapin

(At a risk of re-hashing arguments already made elsewhere,

I think that debate should happen in this issue? After all with my plan for fallibility/allocator polymorphism doesn't really point to try_reserve as written in the RFC being a good idea, even if it is technically possible to support it.

My view is that when panic/abort on allocation error is desired there is no need for try_reserve over reserve.

My argument hinges on one key observation: the code using reserve/try_reserve may not want to decide whether to panic/abort or not on allocation failure, but rather leave that decision to to the user of that code.

In general, we could have arbitrary amounts of collection and and other library code that leaves that decision to only the final executable. There's no good way to write that today without exposing try_ and panicking versions of every method---maybe the stdlib collections have to do that anyways for backwards compat, but lets not put that burden on every other library. You can just expose the try versions and rely on users panicking themselves, but that's very annoying and tedious, and users are likely to make mistakes like panicking on too much if other non-allocation errors are possible. Put shortly, we should never tell library authors to write 2 copies of every function, or anyone to unwrap.

With the polymorphic alloc error, and a safe Result<T, !>::void_unwrap function, library authors can write just one function, and users can do something that, while a bit tedious, is more importantly completely safe. And it lives open the door to even less fiction with e.g. a Result<T, !> <=> T coercion.
John Ericson at 2019-06-12 19:11:59
https://github.com/rust-lang/rust/pull/61780 has merged. I believe it resolves the remaining issues. I’ve updated the API signatures in the issue description.

Is there anything else to tweak here before stabilization? CC @Manishearth for the hashglobe crate, @rillian for mp4parse_fallible

@rfcbot fcp merge
Simon Sapin at 2019-08-19 16:03:36
Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged team members:
- [x] @Amanieu
- [ ] @Kimundi
- [x] @SimonSapin
- [ ] @alexcrichton
- [ ] @dtolnay
- [ ] @sfackler
- [ ] @withoutboats
No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.
Rust RFC bot at 2019-08-19 16:03:37
What's the deal with the possible custom error value in TryReserveError::AllocError?

https://github.com/rust-lang/rust/blob/bdfd698f37184da42254a03ed466ab1f90e6fb6c/src/liballoc/collections/mod.rs#L59-L64

Does that involve later possibly adding a type parameter with a default value to TryReserveError? If so, that would break code that constructs CapacityOverflow because default type parameters only come into play in type position not expression position.
```
let _ = TryReserveError::CapacityOverflow;
```
```
error[E0282]: type annotations needed for `TryReserveError<E>`
  --> src/main.rs:22:13
   |
22 |     let _ = TryReserveError::CapacityOverflow;
   |         -   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type for `E`
   |         |
   |         consider giving this pattern the explicit type `TryReserveError<E>`, where the type parameter `E` is specified
```
David Tolnay at 2019-08-19 18:20:03
Does that involve later possibly adding a type parameter with a default value to TryReserveError?

Correct.

If so, that would break code that constructs CapacityOverflow because default type parameters only come into play in type position not expression position.

Ugh, that’s a good point. Back to the drawing board I guess.

@rfcbot fcp cancel
Simon Sapin at 2019-08-19 20:27:47
@SimonSapin proposal cancelled.
Rust RFC bot at 2019-08-19 20:27:48
Another concern: If we decide to forbid zero sized allocations (https://github.com/rust-lang/wg-allocators/issues/16) the allocator would probably take a NonZeroLayout or similar. In order to not block this issue even more, I propose to mark layout in AllocError as unstable, too, until the WG found a consens.
Tim Diekmann at 2019-10-04 13:52:54
At this point I think there are three options here:
1. Keep blocking stabilization of this feature (which has been implemented for 19 months now) for an unbounded amount of time until the Allocators WG decides whether allocators should have an associated error type https://github.com/rust-lang/wg-allocators/issues/23. Note that WG’s repository has had very little activity in the last 5 months, after a brief surge when in was created.
2. Or take a bet that standard library collections will eventually be made allocator-generic with an associated error type, and stabilize try_reserve with a generic payload field in TryReserveError::AllocError that defaults to ().
  
  The risk is that if the bet turns out wrong, we’ll have a useless field and type parameter that is always (), in a public type.
3. Or take a bet that standard library collections will not be made allocator-generic (that feature might end up on parallel collection types, possibly outside of the standard library), or that they will but with a concrete zero-size error type, and stabilize try_reserve and TryReserveError without a payload field.
  
  The risk is that if the bet turns out wrong, we’ll have try_reserve methods that drop the error value it receives from the allocator.
My preference is for 2 or 3, but I don’t have a strong opinion between them. @rust-lang/libs what do you think?
Simon Sapin at 2019-10-18 21:20:19
```
Note that WG’s repository has had very little activity in the last 5 months, after a brief surge when in was created.
```
It is true that after the repository was opened, a few months was silent. Recently, however, there was an increase in activity again and there is a WIP PR that is currently blocking the WG from pushing anything upstream.

So I would have suggested to make the errortype non-exhaustive. The non-exhaustiveness can still be removed later.
Tim Diekmann at 2019-10-18 21:31:55
I'm personally pretty wary and against stabilizing something just because progress isn't happening on it. Stabilization takes real work and has a cost. Simply because an API exists doesn't mean we should stabilize it, even if it's been sitting for <insert the amount you calculate at the time here> months. I think it's pretty bad design to get things into the standard library, then point out it's sat with no feedback for months and propose stabilization.

Fallible collections are hard in Rust. There's really no way we can escape that. At this point it still seems clear that the design work that needs to be done has not been done. It also seems like no one really knows how to get the design work done.

I agree this would be a great feature to have, and yeah this has sat around forever as unstable. That being said the length of time something has sat is not at all a justification for stabilization in my opinion.
Alex Crichton at 2019-10-21 15:57:59
I think it's pretty bad design to get things into the standard library, then point out it's sat with no feedback for months and propose stabilization.

My understanding was that “Should try_reserve’s error type carry an error value that generic allocators may return” was the only remaining question that needed resolving. I wrote https://github.com/rust-lang/rust/issues/48043#issuecomment-543952770 on that basis.

But it sounds like you have concerns that are larger than that?
Simon Sapin at 2019-10-21 16:04:01
It sounds like your first bullet in that comment has quite a lot of exasperation flowing through it about how slow this process is, and it also sounds like you're drawing attention to emphasize your case about how everyone else should be exasperated about it as well. I understand this process has been slow, and I think it's pretty reasonable that you're exasperated.

I don't think, however, that the exasperation is a justification for stabilization one way or another. We still do not have information about how allocators in the standard library will work out (per collection). That work just simply hasn't been done. You're proposing that we bet one way or another on this API. I think betting is fine, but I want to be very clear that exasperation at the slowness of the process is not a reason to stabilize something. It's can be a reason for stabilization because the functionality is useful enough, but it's generally unproductive in my opinion to vent such exasperation while simultaneously proposing stabilization.

I do not personally have an opinion about how this API should turn out, I don't really want to wade into the design issues of custom allocators on collections personally. I again just want to point out that the exasperation isn't particularly helpful and should never be a reason for stabilization.
Alex Crichton at 2019-10-22 14:23:47
Now that some more work has happened on the design of allocators, I think I would prefer to make all methods of a particular Vec that allocate panic on OOM (instead of aborting), by using something like a PanicOnOOM<MyAlloc> allocator adapter, e.g., like:Vec<T, PanicOnOOMAlloc<MyAlloc>>.

That is, instead of returning a null pointer on OOM from the alloc methods, such an allocator would just panic, allowing the user of Vec to catch the panics. This would make not only a handful of allocating methods fallible, but all of them. And well, if you only want "fallible" allocation for some sections of the code, you can just go from a Vec<T, MyAlloc> to a Vec<T, PanicOnOOM<MyAlloc>> for that particular section and back, and then you are guaranteed that no matter how that code gets refactored (e.g. somebody adds a call to Vec::push somewhere instead of Vec::try_push, you'll get a panic).

This means that Vec::try_push and Vec::try_reserve become maybe a little bit less useful, since if unwinding is available, you can just use Vec::push and Vec::reserve (or extend, or anything really). However, for -C panic=abort, unwinding doesn't work, so this solution won't work there. To support this use case, Vec::try_{push,reserve} are very much necessary, since we need to manually propagate a Result value up the stack to allow recovery.

This approach also has some other implications, e.g., about how the API of the Alloc trait must specify what's valid for an allocator to do on OOM, but I guess that can be simply: if the alloc functions return a non-null pointer, then allocation succeeded, else, allocation failed (but that does not imply that the allocation function returns normally at all: it can abort, panic, execute some callback, etc.). This allows all sort of wrappers that let you "hook" your own "on_alloc_error" callbacks, on a per allocator handle basis.
gnzlbg at 2019-11-28 17:08:02
The RFC that proposed try_reserve also separately proposed an opt-in for panic instead of abort, so that unwinding could be an additional option to recover from OOM:
- RFC: https://rust-lang.github.io/rfcs/2116-alloc-me-maybe.html
- Tracking issue: https://github.com/rust-lang/rust/issues/43596
Under that proposal it’s handle_alloc_error that would change behavior. GlobalAlloc::alloc would still be expected (or encouraged) to return null on errors. It also does not requires allocator-generic containers.
Simon Sapin at 2019-11-28 17:46:04
@SimonSapin any update?
Marco A L Barbosa at 2020-05-19 20:09:44
We now have a renewed effort at https://github.com/rust-lang/rust/pull/72314 so we can experiment with try_* methods across the board. I wish (albeit from the sidelines) that such a PR had landed before so we could already be experimenting with those things, but even though that's gone slower than I hope I still think it's prudent to wait and not stabilize this.
John Ericson at 2020-05-20 02:28:06
@malbarbo I’m not sure what you’re expecting, this is the thread where further discussion about try_reserve should happen. As you can read above, the current status is that stabilization is blocked on deciding what extra data if any TryReserveError::AllocErr should carry, which ties into whether generic allocators should have an associated error types. And more generally there is not good consensus to stabilize before generic allocators overall are more settled.

@Ericson2314 https://github.com/rust-lang/rust/pull/72314 is "Stability annotations on generic parameters", which would help making containers generic on the allocator type, but how is it related to try_* methods? For example Vec can have try_reserve without an extra type parameter.
Simon Sapin at 2020-05-20 07:08:08
ties into whether generic allocators should have an associated error types.

I don't think this is going to be fully resolved until we get some allocators on collections upstream and this is currently blocked by #72314.
Tim Diekmann at 2020-05-20 07:57:29
#77118 has been merged. Can stabilization of this proceed?
Kornel at 2020-11-16 20:18:57
I don't really see, how #77118 is related to this? Where to you want to add an unstable generic annotation?
Tim Diekmann at 2020-11-16 20:31:01
Previous comment mentioned it was blocked on #72314, and #72314 has been replaced by #77118.
Kornel at 2020-11-17 00:01:25
The https://github.com/rust-lang/wg-allocators/issues/48 meta issue's "Allocator aware types" track lays out the progression from #77118 to https://github.com/rust-lang/wg-allocators/issues/39, "try_* methods for containers".

The idea is not that try_* methods themselves inherently need custom allocators, but custom allocators could be leveraged to prevent the use of non-try_* methods, fostering a oom-robust ecosystem just as we have a burgeoning no_std ecosystem.

Also, do note that the only reason we started with try_reserve is that it allows fragile hacking around not having try_* methods more broadly.
John Ericson at 2020-11-17 01:42:13
So, I'm probably missing something, but is there a similar RFC/tracking issue/etc for Box::try_new? Is there some place I can track progress for this?
Manish Goregaokar at 2020-12-17 19:42:39
#77118 has been merged. Can stabilization of this proceed?

In the June 2019 to October 2019 part of this thread, the part of custom allocators that was deemed affecting the design of try_resreve is error types.

That is, if error values for allocation failures (potentially) carry meaningful information then the error type of try_reserve should forward that information to its own callers. In particular if the allocator trait ends up with an associated type for its own errors then the error type of try_reserve should be generic.

By this logic (which my 2019 proposal did not adhere to), try_reserve should only be stabilized after the allocation trait is stabilized. #77118 is only one of the steps required for that.

is there a similar RFC/tracking issue/etc for Box::try_new?

By the letter of RFC 2116 I believe there is not. That RFC only proposed try_reserve for collection types in order to keep its scope small and not get into the design of try_push, try_extend, etc.

In the spirit of that RFC however, try_new is the minimal viable API for fallible allocation in non-collection containers in the same way that try_reserve is for containers. So I’d personally be fine with including it here.
Simon Sapin at 2020-12-18 10:26:47
In the spirit of that RFC however, try_new is the minimal viable API for fallible allocation in non-collection containers in the same way that try_reserve is for containers. So I’d personally be fine with including it here.

I'm talking to some folks doing Linux kernel work in Rust who would like to see this. Should I just open a PR for it, feature gate it with try_new (or try_reserve?) for the same tracking issue?
Manish Goregaokar at 2020-12-18 15:06:25
@Manishearth maybe https://github.com/rust-lang/wg-allocators/issues/7 might help. You could at least see what unstable annotations those PRs used.
John Ericson at 2020-12-18 16:16:16
I'm talking to some folks doing Linux kernel work in Rust who would like to see this. Should I just open a PR for it, feature gate it with try_new (or try_reserve?) for the same tracking issue?

In the meantime, you may be interested in (or already know about) https://crates.io/crates/fallible_collections. We've been using (and contributing to) that crate for awhile now to support the rust MP4 parser that Gecko uses. I've found it quite nice to work with.
Jon Bauman at 2020-12-18 18:23:03
Opened https://github.com/rust-lang/rust/pull/80310
Manish Goregaokar at 2020-12-22 21:51:01
BTW, doesn't this could also add:
- try_with_capacity()
- try_push()
- try_append()
- try_insert()
- try_split_off()
- try_resize_with()
- try_resize()
- try_extend_from_slice()
I guess I miss some, this will add a lot of method on Vec but I think there is no reason they should not follow. I wonder if we could put all of this in a trait just for Vec.
Stargateur at 2021-03-22 17:58:22
@Stargateur it does indeed work, and is nice and ergonomic with ?.
John Ericson at 2021-03-22 20:59:31
That is, if error values for allocation failures (potentially) carry meaningful information then the error type of try_reserve should forward that information to its own callers. In particular if the allocator trait ends up with an associated type for its own errors then the error type of try_reserve should be generic.

By this logic (which my 2019 proposal did not adhere to), try_reserve should only be stabilized after the allocation trait is stabilized. #77118 is only one of the steps required for that.

Would using an opaque type be possible, i.e. Result<_, impl Error>? Could that be replaced with <A as Allocator>::Error or AllocError later? That would decouple the allocator API question from the try_ effort.
the8472 at 2021-04-22 18:39:08
Has anyone proposed a use-case for the TryAllocError?

The discussion in this thread so far is only focused on what could be added the error, but I don't see justification why it's being done and who is it for. It feels like a solution in search of a problem to solve.

In what situations it's necessary to handle the error differently depending on the details like Layout?

Why would any application care whether it's a CapacityOverflow or actual OOM? Isn't this distinction just formalizing a quirk/inconsistency in the current Rust implementation?

I suggest using zero-sized type TryAllocError, and not giving any details at all.
- Applications that embrace fallible allocations will use them a lot, possibly in hot loops with try_push, and a complex error type would add overhead, e.g. look how much bloat &mut Vec as io::Write adds for just moving the io::Error around.
- malloc just returns NULL and AFAIK nobody ever complained about the lack of detail there. There's no errno equivalent for it.
- C++ bad_alloc exception also has no useful info ("bad allocation" string is all you get).
- When I've used the fallible_collections crate and I didn't need any details from the error. In some cases I wanted to report something more informational than "out of memory", but for this I actually needed higher-level application-domain info like "Not enough memory for a {}x{} pixels large image", and Layout is not useful for this — it's too low-level.
- In multithreaded applications (or even in just any multitasking OS) any information from the allocator is inherently racy and subject to TOCTOU errors. If you imagine some use-case like checking the error details to decide whether to retry with a smaller allocation - that won't work. By the time you check the error, the data may already be out of date. If you want to know whether another allocation would have succeeded, then the only way is to actually perform the allocation.
So I think for simplicity and performance zero-sized TryAllocError is the best.
Kornel at 2021-04-22 19:49:09
Could we do a ZST impl trait? That would be even more opaque.
John Ericson at 2021-04-22 20:53:03
Why would any application care whether it's a CapacityOverflow or actual OOM? Isn't this distinction just formalizing a quirk/inconsistency in the current Rust implementation?

I disagree, capasity overflow is more a limitation of Rust than a limitation of the hardware and so it's important difference with pure OOM. The problem with most of your following point is that your didn't take in account that C and C++ don't have this limitation and so don't need to make a difference and so your comparative like:

malloc just returns NULL and AFAIK nobody ever complained about the lack of detail there. There's no errno equivalent for it.

are pointless.
Stargateur at 2021-04-22 22:28:17
C and C++ don't have this limitation

If you're referring to allocations having to fit in isize rather than usize, C and C++ do have that limitation. While the C standard doesn't explicitly ban allocations greater than half the address space, it defines pointer subtraction as returning the signed type ptrdiff_t. Since pointer subtraction is only defined for pointers within the same allocation, this leads to a lot of problems, including miscompilations due to compiler assumptions, if and only if a single allocation can be larger than PTRDIFF_MAX bytes. For this reason, some implementations of libc have long capped malloc allocations at that size, and glibc added such a cap in 2019.

But I believe CapacityOverflow would also cover the case where multiplying the capacity by the element size simply overflows usize entirely. That could be seen as more of a hardware limitation.

C++ actually does distinguish capacity overflow (of either kind) from other errors, by throwing a different exception. C++ standard containers have a max_size() method, returning the maximum possible size that can be allocated. If you call reserve with a value higher than max_size(), you get std::length_error, as opposed to std::bad_alloc for other allocation failures.

On the other hand, C doesn't distinguish them. While C doesn't have standard containers, it does have the calloc function, which accepts a count and an element size, and multiplies them together to determine how many bytes to allocate. This multiplication is overflow-checked, but overflow results in calloc returning NULL with errno set to ENOMEM, same as other allocation failures. Edit: (So does exceeding the aforementioned cap of PTRDIFF_MAX bytes.)
comex at 2021-04-23 05:27:29
but what would an application do with that info?
```
match vec.try_reserve(size) {
   Err(CapacityOverflow) => {…} // A
   Err(AllocErr(_)) => {…} // B
   _ => {},
}
```
what app needs A case to be different from B?

I'm struggling to come up with anything, because in both cases the end result is the same — you just can't allocate the size you wanted. And in both cases you can't assume that you will be able to allocate some other size until you try to.

Even if an application wanted to allocate as close to half of the address space as it can get, then it doesn't need the error, it just needs to compute the correct size ahead of time from size_of and isize::MAX.

Even in case of a hypothetical container with non-trivial maximum capacity, this error is also unnecessary. Such collection should expose something like Collection::MAX_LEN to allow callers to choose an acceptable capacity before getting the error, instead of probing it through failures.
Kornel at 2021-04-23 14:53:29
While the C standard doesn't explicitly ban allocations greater than half the address space

I was talking about C standard, I don't care of C implementation specificity. My point remain. I agree with the rest of your message.

but what would an application do with that info?

Do you really handle type of error in your application, in almost 99% of application the way to handle error is to return it in cascade. The real value is to known what is the error for the real user and maybe fix the code. Or change application parameter to avoid allocate too much on this specific example. If a user see OOM, the obvious fix is too add RAM but adding RAM when you have hit a software limitation will not help at all. I agree this will not happen anytime soon for 64 bit. But that would be nice to have it for 32 bit system and other thing like embedded or boot code.

I'm struggling to come up with anything, because in both cases the end result is the same — you just can't allocate the size you wanted. And in both cases you can't assume that you will be able to allocate some other size until you try to.

With this logic why bother report any error anyway what the application will do ? let's just make all API return "An error have happen".

Even in case of a hypothetical container with non-trivial maximum capacity, this error is also unnecessary. Such collection should expose something like Collection::MAX_LEN to allow callers to choose an acceptable capacity before getting the error, instead of probing it through failures.

Like std ? I fail to remember such feature exist for vector. So I must call an function myself to verify I do not send wrong value before I call the real function I want to use ? This is not user friendly.
Stargateur at 2021-04-23 23:17:58
but what would an application do with that info?

Yeah, I personally agree that this information is not very useful.
comex at 2021-04-24 01:25:50
I've got an idea for making fallible vec available on stable sooner — hide it in the io::Write implementation! vec.write_all(s) is functionally equivalent to vec.try_extend_from_slice(s).

https://github.com/rust-lang/rust/pull/84612
Kornel at 2021-04-29 17:16:10
In addition to @kornelski's ZST suggestion, I personally think we shouldn't even have separate TryReserveError and AllocError.
Gary Guo at 2021-08-06 21:53:32
Now TryReserveError is an opaque type. Besides the error type, were there any other blockers to stabilization?
Kornel at 2021-08-12 11:07:36
As far as I know, that was the only blocker.
Tim Diekmann at 2021-08-12 11:22:31
Request for stabilization

I'd like to propose to stabilize this in 1.56.

Brief summary

try_reserve() and try_reserve_exact() methods on Vec, VecDeque, HashMap, HashSet, and String. These methods enable best-effort graceful handling of out of memory errors, especially large allocations on operating systems without virtual memory overcommit, or when using custom allocators that self-impose memory usage limits (like cap).
```
let mut vec = Vec::new();
vec.try_reserve(lots)?;
/// now pushing lots of bytes won't abort
```
This feature addresses "Gecko" and "Server" use-cases from the RFC. It doesn't solve allocation problems of no-std/kernel use-cases, and conversely global switches and alternative types for kernels don't address the app/server use-cases.

Testing

Unit tests are in liballoc. Fallible allocation in general has been prototyped via fallible_collections crate, e.g. shipped in the mp4 parser used in Firefox.

Documentation

The feature is documented in liballoc. The book and rust-by-example don't discuss regular .reserve(), so try_reserve() is out of scope.

Questions intentionally left unresolved

The TryReserveError type is left as an opaque struct in order to leave options open for possible future extensions to custom allocators, or it may be later reduced to a unit struct if it turns out that there's no user demand for exposing further details.
Kornel at 2021-08-12 23:30:29
Do we have a way to reserve the possibility of merging AllocError and TryReserveError into the same type?
Gary Guo at 2021-08-13 00:01:44
It could wrap it and expose via method or even deref. A bold move would be to define pub type TryReserveError = AllocError;, but that's technically breaking if someone wrote trait impls for both.
Kornel at 2021-08-13 00:06:51
If we ended up wanting TryReserveError to not distinguish between CapacityOverflow and AllocError, then having TryReserveError and AllocError as separate type would be unergonomic IMO. On the other hand a type alias would break trait impls.
Gary Guo at 2021-08-13 00:11:36
AllocError is unstable, so we could merge them before it is stabilized.
the8472 at 2021-08-13 06:43:37
Request for stabilization

In a libs team meeting there was the discussion of having a separate Vec type that contains all the fallible methods and cheap conversion methods between the fallible and infallible Vec. If that were implemented then try_reserve on what would then be the infallible Vec would be redundant.

So the question would be if that kind of overlap is acceptable to get it stabilized now or whether the separate Vec type should be pursued instead.
the8472 at 2021-08-13 06:46:54
@the8472 I've tried to use the TryVec type in a large server project, and I found it completely unworkable. That approach was a dead-end, and I ended up deleting that branch and staring over with only regular types and try_reserve.

It was both too restrictive and too lax. Too restrictive: incompatibility with Vec made small type changes cascade throughout the entire codebase, and clash with dependencies. It was annoying and needless churn, because not every use of Vec needs to grow it. Storing/returning non-growable buffers as a Vec is important, because conversion to a boxed slice often re-allocates.

And despite highly disruptive viral nature of the type, it wasn't enough: It doesn't stop code from using aborting Vec for temporaries in functions and in private fields. It doesn't stop use of vec!, Vec::new or collect() if such use doesn't affect other code through public fields and return types. I was really deeply disappointed that even after I've upturned my whole codebase to change a fundamental type, it was still aborting like before (edit: but a global switch wouldn't work either, because it would break most crates.io dependencies).

So I think having only a separate type without an easy-to-adopt try_reserve would be highly problematic for usability. A different fallible-only type could be pursued regardless, but such approach is not a replacement for what try_reserve does, so I urge you to stabilize try_reserve.
Kornel at 2021-08-13 09:52:04
The current idea in the allocator WG is to use the no_global_oom_handling feature: https://github.com/rust-lang/rust/pull/84266. When this feature is set, all fallible allocations are disabled and only the try_ variants will be available. This doesn't introduce a new type and is fully compatible.
Tim Diekmann at 2021-08-13 10:03:40
no_global_oom_handling is basically only for the Linux kernel and embedded/no-std code. It doesn't satisfy server and application use-cases from the RFC, because it's incompatible with all std-dependent crates on crates.io. It's not an alternative to try_reserve.
Kornel at 2021-08-13 10:05:36
No, it's not an alternative, but it plays together very well. I expect the usual use case to use try_reserve in critical section if needed but also use reserve (or any method calling it) in the same program. As you pointed out, splitting Vec into two types will make this very hard. Splitting Vec was also discussed in the allocator WG at Zulip (Can't find the link currently) and ended up with different conversion methods. It was discussed because some libraries (i.e. the Linux kernel) must not use reserve and TryVec would prohibit the usage. For such crates, no_global_oom_handling is great without adding extra types to std.

For me, it totally makes sense to add try_reserve as an option.

EDIT: Regarding your linked added just before I submitted the comment: Yes, exactly this. 🙂
Tim Diekmann at 2021-08-13 10:14:55
@kornelski I agree with "I don't really care about detail information" but I strongly think I want to be able to differentiate a OOM error from a too big for isize error. People will maybe thank me in 30 years for this
Stargateur at 2021-08-13 14:45:02
@TimDiekmann I think this should be added as a form of a trait to avoid too many try_ method or maybe a intermediate type that only have failure handling method. I still hate the original choice to not have return an error for OOM see my 2017 SO question https://stackoverflow.com/q/44923306/7076153.
```
impl Vec {
    fn find_a_name(&mut self) -> TryVec;
}

impl TryVec {
    fn push(&mut self) -> Result;
    // basically all vector method
```
Stargateur at 2021-08-13 14:50:58
@kornelski

@the8472 I've tried to use the TryVec type in a large server project, and I found it completely unworkable. That approach was a dead-end, and I ended up deleting that branch and staring over with only regular types and try_reserve.

It was both too restrictive and too lax. Too restrictive: incompatibility with Vec made small type changes cascade throughout the entire codebase, and clash with dependencies. It was annoying and needless churn, because not every use of Vec needs to grow it. Storing/returning non-growable buffers as a Vec is important, because conversion to a boxed slice often re-allocates.

I think that is mostly a limitation of TryVec because it doesn't live in the standard library. If they are located in the same crate then you can define interconversion-methods that turn a &mut Vec into a &mut FallibleVec and vice versa which you can pass to any API expecting those. And of course the owned types can also be turned into each other. And that of course means they can also be turned into Box<[T]> if needed.

And despite highly disruptive viral nature of the type, it wasn't enough: It doesn't stop code from using aborting Vec for temporaries in functions and in private fields. It doesn't stop use of vec!, Vec::new or collect() if such use doesn't affect other code through public fields and return types. I was really deeply disappointed that even after I've upturned my whole codebase to change a fundamental type, it was still aborting like before.

That's what no_global_oom_handling would achieve, it would basically forbid using the infallible version of Vec.

So I think having only a separate type without an easy-to-adopt try_reserve would be highly problematic for usability. A different fallible-only type could be pursued regardless, but such approach is not a replacement for what try_reserve does, so I urge you to stabilize try_reserve.

I think this isn't true if you have free conversion between the types.

@TimDiekmann

The current idea in the allocator WG is to use the no_global_oom_handling feature: #84266. When this feature is set, all fallible allocations are disabled and only the try_ variants will be available. This doesn't introduce a new type and is fully compatible.

Major issues with that design are a) you have to slap the feature gate on everything. It's annoying for maintenance. Having separate types would make this much easier since you could gate entire modules.

Additionally it makes naming easier because you wouldn't have to prefix everything with try_, you could just have two versions of reserve, push etc. on the different types, one that returns a Result and the one that doesn't. I.e. it simplifies naming.

Anyway, this is just my what I understood from the libs team meeting. Maybe those were just weakly held ideas. :man_shrugging:
the8472 at 2021-08-13 17:50:56
I think that is mostly a limitation of TryVec because it doesn't live in the standard library. If they are located in the same crate then you can define interconversion-methods that turn a &mut Vec into a &mut FallibleVec and vice versa which you can pass to any API expecting those. And of course the owned types can also be turned into each other. And that of course means they can also be turned into Box<[T]> if needed.

But what is gained by splitting the types? In particular, if there are safe, cheap interconversion methods, then I assume the types will not guarantee their allocations have been / will be fallible or infallible. Thus the split would be mostly used as a way to namespace methods, right? And, if those names end up being the same for both as you propose, as a fun way to perform method overloading.

Major issues with that design are a) you have to slap the feature gate on everything. It's annoying for maintenance. Having separate types would make this much easier since you could gate entire modules.

While that is important to consider, in the end it is an implementation detail.

Additionally it makes naming easier because you wouldn't have to prefix everything with try_, you could just have two versions of reserve, push etc. on the different types, one that returns a Result and the one that doesn't. I.e. it simplifies naming.

Yes, names would be simpler if a project only uses a single kind of vector everywhere, but for any project that happens to mix them, it could end up being more confusing -- similar to supporting overloading.
Miguel Ojeda at 2021-08-13 19:28:23
But what is gained by splitting the types? In particular, if there are safe, cheap interconversion methods, then I assume the types will not guarantee their allocations have been / will be fallible or infallible.

You mean enforcing that other libraries don't do infallible allocation? Yeah, that's not the goal here. The goal is to split the humongous API surface of Vec. Having a separate type and conversion methods makes it easy to isolate the infallible parts locally. Make sure that all your oom-panic-free modules don't contain Vec<> or to_infallible() and that should be it, the fallible convenience feature could use them in a separate module that's exempt from that.

On top of that no_global_oom_handling would of course still disable the infallible versions.
the8472 at 2021-08-13 19:52:36
If that's not a goal I wouldn't bother with a split at all. Having all allocating methods also have a try_ variant doesn't meaningfully increase the API surface at all. You just explain once what try_ means and the user is all set.

Compare with integers: Adding a method to all signed integer types is technically affecting 6 different types, but users actually still just think of it as "one" new operation you can do.
Lokathor at 2021-08-13 19:59:45
@the8472 I can't imagine using a global kill-switch no_global_oom_handling. My server/application projects have (transitively) hundreds of dependencies. I will not be able convince maintainers of every single crate to switch to fallible allocations everywhere, and I wouldn't be able to get my job done if I had to limit myself to a subset of the crates ecosystem like the no_std. Even if a single dependency, even in one place, uses some infallible feature it would stop my whole application from using no_global_oom_handling. And for a dependency-of-dependency-of-dependency it may make perfect sense to e.g. use format!(), and they wouldn't care that I can't have them use format!() because my project elsewhere does something else that needs to handle OOM. Besides, I don't even want fallible allocations everywhere in my projects. For example, my servers have substantial startup code that doesn't need to handle OOM.

The fallible allocation RFC specifically lists different use-cases, which have different needs. It's already been established that one feature won't satisfy them all.

Even if we eventually figure out an approach that works for everything from a microwave oven firmware, through Linux kernel modules, all the way up to Firefox and web services, the worst case scenario is that try_reserve() will be unnecessary, or become a convenience method for calling a better thing.
Kornel at 2021-08-13 20:00:13
@Stargateur @the8472 The traits and methods you're discussing have already been considered two years ago when the fallible allocation RFC has been accepted. The RFC chose not to add a separate fallible Vec type, nor add try_ methods for everything, because try_reserve is the minimal building block that can be used to create all of these in user code.
Kornel at 2021-08-13 20:41:19
@kornelski and still there seem to be not a consensus "In a libs team meeting there was the discussion of having a separate Vec type that contains all the fallible methods", so I think it's still open in fact, or let just stop for now to wait and just do what we agree on in the RFC ? why are we waiting so ? Why are we talking ?

unless I'm stupid you clearly do the same thing as me here. But you don't consider that your point apply to you ?

But I agree that having try_reserve allow everything, thus we will end up doing the wrapping tool outside std that just call try_reserve everytime, but I hope this call will be optimized away, cause vec will always check capacity internally, this mean if not optimized out we would check size twice for using this feature.
Stargateur at 2021-08-13 21:27:26
I asked on zulip, the response was:

Several people in that discussion very much didn't want to see separate types, because those types tend to "infect" everything in an API, and it's a pain to support both.

the8472 at 2021-08-13 21:36:02
What is the motivation for impl From<LayoutError> for TryReserveError? Could someone share an example of what a realistic use case would look like with and without this impl to show what difference it makes?

It doesn't seem straightforward that a LayoutError due to e.g. non-power-of-2 alignment would turn into a "capacity overflow" after the conversion.
David Tolnay at 2021-08-21 21:33:50
Is there a use-case for creating instances of TryReserveError?

One I can think of is someone creating a custom data structure, and wanting to mimic std API. But that's not a very strong case. 3rd party crates can use io::ErrorKind::OutOfMemory or just their own error type.

If there's no need to be able to create TryReserveError instances in user code, then it would be fine to remove the From impl.
Kornel at 2021-08-21 22:37:28
I believe this From impl is used by the standard library’s implementation of try_reserve. Yes it could be removed and replaced by a manual conversion.
Simon Sapin at 2021-08-22 05:00:00
After the merge of the stabilization in #87993, this issue can probably be closed?
h-vetinari at 2021-10-18 02:03:30
TryReserveError::kind and TryReserveErrorKind are still unstable with attributes that point here for their tracking issue. Either the description of this issue should be updated to reflect this, or a new tracking issue should be filed and the attributes updated. Since this thread is rather long already the latter may be better.
Simon Sapin at 2021-10-20 06:53:30
It's super minor but TryReserveErrorKind is missing periods at the end of sentences in the documentation.
Orson Peters at 2021-12-27 16:43:07
It's super minor but TryReserveErrorKind is missing periods at the end of sentences in the documentation.

The only such sentence I see there is "The memory allocator returned an error"; the others are just sentence fragments, which may not deserve periods <small>(for example, where sentence fragments may be used in Wikipedia, their style guide (probably the result of compromise between many strong opinions) says these "should in most cases not end with a period")</small>.
8573 at 2021-12-27 19:01:21
the others are just sentence fragments, which may not deserve periods

https://doc.rust-lang.org/1.0.0/style/style/comments.html#sentence-structure

All doc comments, including the summary line, should begin with a capital letter and end with a period, question mark, or exclamation point. Prefer full sentences to fragments.

Sentence fragments should end with a period in Rust, even if full sentences are preferred.
Orson Peters at 2021-12-27 19:18:41
Important news! HashMap got method try_insert ( https://github.com/rust-lang/rust/issues/82766 ), where try_ means something completely different to Box::try_new. Please, prevent somehow this try_insert from becoming stable
Askar Safin at 2022-01-21 13:37:32
Please file an issue dedicated to the naming of try_insert since this thread is already covering other topics.
Simon Sapin at 2022-01-24 17:58:01

The comment of example code is seems wrong. If size of data is about 60% of system memory, try_reserve might success because OS doesn't allocate physical memory immediately. Writing to allocated memory causes page fault and OS allocates memory. Sum of size of old data and new data is about 120% of system memory. So, OOM might occur.

<details> <summary> List of the comments </summary>

$ rg -C3 "can't OOM"
library/std/src/ffi/os_str.rs
311-    ///     // Pre-reserve the memory, exiting if we can't
312-    ///     s.try_reserve(OsStr::new(data).len())?;
313-    ///
314:    ///     // Now we know this can't OOM in the middle of our complex work
315-    ///     s.push(data);
316-    ///
317-    ///     Ok(s)
--
382-    ///     // Pre-reserve the memory, exiting if we can't
383-    ///     s.try_reserve_exact(OsStr::new(data).len())?;
384-    ///
385:    ///     // Now we know this can't OOM in the middle of our complex work
386-    ///     s.push(data);
387-    ///
388-    ///     Ok(s)

library/alloc/src/string.rs
1098-    ///     // Pre-reserve the memory, exiting if we can't
1099-    ///     output.try_reserve(data.len())?;
1100-    ///
1101:    ///     // Now we know this can't OOM in the middle of our complex work
1102-    ///     output.push_str(data);
1103-    ///
1104-    ///     Ok(output)
--
1139-    ///     // Pre-reserve the memory, exiting if we can't
1140-    ///     output.try_reserve_exact(data.len())?;
1141-    ///
1142:    ///     // Now we know this can't OOM in the middle of our complex work
1143-    ///     output.push_str(data);
1144-    ///
1145-    ///     Ok(output)

library/alloc/src/collections/binary_heap.rs
994-    ///     // Pre-reserve the memory, exiting if we can't
995-    ///     heap.try_reserve_exact(data.len())?;
996-    ///
997:    ///     // Now we know this can't OOM in the middle of our complex work
998-    ///     heap.extend(data.iter());
999-    ///
1000-    ///     Ok(heap.pop())
--
1029-    ///     // Pre-reserve the memory, exiting if we can't
1030-    ///     heap.try_reserve(data.len())?;
1031-    ///
1032:    ///     // Now we know this can't OOM in the middle of our complex work
1033-    ///     heap.extend(data.iter());
1034-    ///
1035-    ///     Ok(heap.pop())

library/alloc/src/collections/vec_deque/mod.rs
776-    ///     // Pre-reserve the memory, exiting if we can't
777-    ///     output.try_reserve_exact(data.len())?;
778-    ///
779:    ///     // Now we know this can't OOM(Out-Of-Memory) in the middle of our complex work
780-    ///     output.extend(data.iter().map(|&val| {
781-    ///         val * 2 + 5 // very complicated
782-    ///     }));
--
813-    ///     // Pre-reserve the memory, exiting if we can't
814-    ///     output.try_reserve(data.len())?;
815-    ///
816:    ///     // Now we know this can't OOM in the middle of our complex work
817-    ///     output.extend(data.iter().map(|&val| {
818-    ///         val * 2 + 5 // very complicated
819-    ///     }));

library/alloc/src/vec/mod.rs
893-    ///     // Pre-reserve the memory, exiting if we can't
894-    ///     output.try_reserve(data.len())?;
895-    ///
896:    ///     // Now we know this can't OOM in the middle of our complex work
897-    ///     output.extend(data.iter().map(|&val| {
898-    ///         val * 2 + 5 // very complicated
899-    ///     }));
--
936-    ///     // Pre-reserve the memory, exiting if we can't
937-    ///     output.try_reserve_exact(data.len())?;
938-    ///
939:    ///     // Now we know this can't OOM in the middle of our complex work
940-    ///     output.extend(data.iter().map(|&val| {
941-    ///         val * 2 + 5 // very complicated
942-    ///     }));

</details>

Toru3 at 2022-08-16 08:05:12

@Toru3 Overcommit is a known limitation, and it's been considered in the RFC. You can mitigate the problem on the OS level either by disabling overcommit, or running code in containers with limited amount of memory. Within Rust programs you can reduce chance of hitting the problem by using the cap allocator with a limit below the amount of physical memory available.
Kornel at 2022-08-25 14:03:42
Would it be possible/wise to put TryReserveError into core ? I may need to have access to this struct when my crate compile without alloc.
Stargateur at 2024-03-13 14:21:46

Request for stabilization

Brief summary

Testing

Documentation

Questions intentionally left unresolved