Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Deny OOM, embrace try_reserve #448

Open
wants to merge 13 commits into
base: trunk
Choose a base branch
from

Conversation

VictorKoenders
Copy link
Contributor

@VictorKoenders VictorKoenders commented Dec 11, 2021

With Rust 1.57, several try_reserve methods have been stabilized. This PR makes bincode take advantage of this.

A nightly flag can be set to disable all fallible allocations. This is currently blocked on the following features:

And uses the following features, although this could be implemented manually:

Other useful issues:

  • Vec::push_within_capacity to reduce the amount of unsafe code to replace Vec::push tracking issue

@codecov-commenter
Copy link

codecov-commenter commented Dec 11, 2021

Codecov Report

Attention: Patch coverage is 51.36986% with 71 lines in your changes are missing coverage. Please review.

Project coverage is 57.05%. Comparing base (e7dff9d) to head (c4e11a1).
Report is 2 commits behind head on trunk.

Files Patch % Lines
src/features/impl_alloc.rs 53.43% 61 Missing ⚠️
src/error.rs 0.00% 5 Missing ⚠️
src/features/impl_std.rs 50.00% 2 Missing ⚠️
src/features/serde/mod.rs 0.00% 2 Missing ⚠️
src/features/serde/de_borrowed.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk     #448      +/-   ##
==========================================
- Coverage   57.21%   57.05%   -0.16%     
==========================================
  Files          51       51              
  Lines        4410     4427      +17     
==========================================
+ Hits         2523     2526       +3     
- Misses       1887     1901      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@VictorKoenders VictorKoenders force-pushed the feature/fallible_allocations branch 4 times, most recently from b806672 to f2731c0 Compare December 11, 2021 10:03
@stale
Copy link

stale bot commented Apr 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 2, 2022
@ZoeyR ZoeyR added the not-stale label Apr 4, 2022
@stale stale bot removed the stale label Apr 4, 2022
@VictorKoenders VictorKoenders force-pushed the feature/fallible_allocations branch 9 times, most recently from cef0e15 to fb27373 Compare August 17, 2022 10:51
@VictorKoenders VictorKoenders marked this pull request as ready for review August 18, 2022 18:34
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
src/features/impl_alloc.rs Outdated Show resolved Hide resolved
src/features/impl_alloc.rs Show resolved Hide resolved
@gimbling-away
Copy link
Contributor

image
Forgot a closing paren there in the PR description?

@VictorKoenders VictorKoenders force-pushed the feature/fallible_allocations branch 2 times, most recently from eeeefa5 to 9d58ec1 Compare March 30, 2023 13:10
Added try_reserve to VecWriter, Vec, VecDeque and HashMap
Made the OOM test succeed on the latest nightly
Made the OOM check work on nightly
Disabled stable checks for now
Removed feature that is stable in current nightly
Fixed issue where Box<[T]> would not do the size limit check
Added a drop guard to `impl<T> Decode for Vec<T>` to make sure memory is not leaked when T::Decode panics
Made the implementation of `VecWriter` use less lines of unsfe
Fixed errors while merging
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants