Handle out of memory errors in io:Read::read_to_end()

[kornelski]

#116570 got stuck due to a procedural confusion. Retrying so that it can get FCP with the proper team now. cc @joshtriplett @BurntSushi

---

I'd like to propose handling of out-of-memory errors in the default implementation of io::Read::read_to_end() and fs::read(). These methods create/grow a Vec with a size that is external to the program, and could be arbitrarily large.

Due to being I/O methods, they can already fail in a variety of ways, in theory even including ENOMEM from the OS too, so another failure case should not surprise anyone.

While this may not help much Linux with overcommit, it's useful for other platforms like WASM. Internals thread.

I've added documentation that makes it explicit that the OOM handling is a nice-to-have, and not a guarantee of the trait.

I haven't changed the implementation of impl Read for &[u8] and VecDeque out of caution, because in these cases users could assume read can't fail.

This code uses try_reserve() + extend_from_slice() which is optimized since #117503.

[rustbot]

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

[bors]

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

[kornelski]

@rustbot r?

[rustbot]

Error: Parsing assign command in comment failed: ...'' | error: specify user to assign to at >| ''...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

[fintelia]

Perhaps io::Repeat should also return a memory error? Right now it unconditionally OOMs...

[kornelski]

@fintelia I've made a separate PR, because I think it may be bikesheddable which error kind it should return.

[Amanieu]

We discussed this in the libs-api meeting and we are happy with this change. It doesn't need an FCP since it isn't an API breaking change.

Some concerns on the implementation:

• [u8]::read_to_end is missing a try_reserve.
• Same with VecDeque:read_to_end.
  These are both located in library/std/src/io/impls.rs.

[kornelski]

@Amanieu I've added the impls for [u8] and VecDeque, and rebased.

[Amanieu]

@bors r+

[bors]

📌 Commit 60f4628 has been approved by Amanieu

It is now in the queue for this repository.

[bors]

⌛ Testing commit 60f4628 with merge 5c9c3c7...

[bors]

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 5c9c3c7 to master...

[rust-timer]

Finished benchmarking commit (5c9c3c7): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	6.2%	[6.2%, 6.2%]	1
Improvements ✅ (primary)	-4.3%	[-6.4%, -2.2%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-4.3%	[-6.4%, -2.2%]	2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.2%	[0.0%, 0.3%]	13
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	16
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[-0.4%, 0.3%]	14

Bootstrap: 661.382s -> 662.576s (0.18%)
Artifact size: 308.09 MiB -> 308.05 MiB (-0.01%)

Wait, was it intentional to merge this with some read_to_end implementations that know their exact size using try_reserve and others using try_reserve_exact?

Prior to this PR they all used reserve.

I've ended up making the change to _exact in response to the discussion — #117925 (comment)

The default behavior remains to use try_reserve that may overshoot, because of patterns like take(1).read_to_end(append) in a loop that could end up resizing too much.

The _exact allocation is used in std::fs::read(_to_string) functions that return a new Vec. I think in these cases it's reasonable to assume that users are interested in reading and consuming the whole file, and users would rather expect these functions to be memory-efficient, than optimized for immediately appending data. File edits will likely go through parsing and serialization, and simple appends can be done more efficiently with OpenOptions::append.

exact_ is also in File::read_to_end specifically, when it's not wrapped in Take or something else. In that basic case I assume it's used similarly to std::fs::read, and if not, then the caller has an option of reserving more space themselves.

The _exact allocation is used in std::fs::read(_to_string) functions that return a new Vec. I think in these cases it's reasonable to assume that users are interested in reading and consuming the whole file, and users would rather expect these functions to be memory-efficient, than optimized for immediately appending data. File edits will likely go through parsing and serialization, and simple appends can be done more efficiently with OpenOptions::append.

This case is irrelevant because _reserve and _reserve_exact behave identically if the Vec starts out with capacity=0. (Except for an edge case of requesting fewer than 8-bytes and using a weird allocator that supports sub 8-byte allocations.)

exact_ is also in File::read_to_end specifically, when it's not wrapped in Take or something else. In that basic case I assume it's used similarly to std::fs::read, and if not, then the caller has an option of reserving more space themselves.

This adds a quirk to one specific implementation that I'm not sure is justified. As mentioned, _reserve doesn't overshoot if the Vec starts out empty. But _reserve also doesn't overshoot if the total capacity required is more than double the current capacity.

In order words, the only case that changed from this PR is when the read size requires an allocation between cap and cap*2. The code for File used to round up to cap*2 but now allocates the exact hinted size. Hitting this case at all is somewhat of an edge case, but if you do, there's a reasonable chance you may hit it many times and actually prefer the previous behavior.

@fintelia To me it makes sense that File's implementation would be specialized for its own type, with EOF being inherently a special case. But that's a detail orthogonal to OOM handling, so I've made a PR to restore previous behavior: #120538