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

Don't initialize stdout during io cleanup #119394

Closed
wants to merge 1 commit into from

Conversation

DaniPopes
Copy link
Contributor

Slightly simplify the std::io::cleanup code by using OnceCell::get instead of checking initialization manually with get_or_init.

This also has the consequence of not initializing stdout with an empty buffer if it hasn't been already, but I believe this is inconsequential given that this code only runs at process exit.

Slightly simplify the `std::io::cleanup` code by using `OnceCell::get` instead
of checking initialization manually with `get_or_init`.

This also has the consequence of not initializing stdout with an empty buffer
if it hasn't been already, but I believe this is inconsequential given that
this code only runs at process exit.
@rustbot
Copy link
Collaborator

rustbot commented Dec 28, 2023

r? @joshtriplett

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 28, 2023
@bjorn3
Copy link
Member

bjorn3 commented Dec 30, 2023

This doesn't disable buffering unlike the previous code, which may matter if sys::cleanup() wants to write to stdout. None of the current implementations do, and I don't think it is all that likely to happen, but if it does, it will likely lead to confusion why nothing gets printed.

@joshtriplett
Copy link
Member

r? libs-api

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Feb 11, 2024
@rustbot rustbot assigned dtolnay and unassigned joshtriplett Feb 11, 2024
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cleanup is intentionally done this way since #101404 in order to fix #101375.

Thanks anyway for the PR!

@dtolnay dtolnay closed this Feb 11, 2024
@DaniPopes DaniPopes deleted the simpler-stdio-cleanup branch February 11, 2024 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants