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

FCP poll for ErrorKind::OutOfMemory #84916

Closed
joshtriplett opened this issue May 4, 2021 · 7 comments
Closed

FCP poll for ErrorKind::OutOfMemory #84916

joshtriplett opened this issue May 4, 2021 · 7 comments
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@joshtriplett
Copy link
Member

joshtriplett commented May 4, 2021

I unintentionally merged #84744 without first starting an FCP. We'd previously discussed adding more ErrorKind values, but nonetheless this specific change should have gone through FCP. I've opened this issue to check consensus on this change; if there's any issue with it, I'll post a revert.

@joshtriplett joshtriplett added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label May 4, 2021
@joshtriplett
Copy link
Member Author

@rfcbot merge

@rfcbot
Copy link

rfcbot commented May 4, 2021

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

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.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 4, 2021
@BurntSushi
Copy link
Member

One question I had is, other than #84744, where might we use this?

@joshtriplett
Copy link
Member Author

@BurntSushi Anywhere that calls last_os_error() when errno is ENOMEM (or the equivalent on Windows). While on Linux it's common to invoke the OOM killer rather than return ENOMEM, it's possible to disable that (and some systems do), and other systems do return ENOMEM. And even on Linux with overcommit and OOM, there are ways to get ENOMEM; for instance, you could get this from fork() of a process using a substantial amount of the system's memory, so it could bubble up from Command.

@BurntSushi
Copy link
Member

@joshtriplett Understood. Thanks for explaining that! This SGTM then.

@rfcbot
Copy link

rfcbot commented May 4, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels May 4, 2021
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 14, 2021
@rfcbot
Copy link

rfcbot commented May 14, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label May 14, 2021
@dtolnay dtolnay closed this as completed May 14, 2021
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants