-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
std: Prevent print panics when using TLS #29491
Conversation
Currently if a print happens while a thread is being torn down it may cause a panic if the LOCAL_STDOUT TLS slot has been destroyed by that point. This adds a guard to check and prints to the process stdout if that's the case (as we do for if the slot is already borrowed). Closes rust-lang#29488
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
// | ||
// If, however, the actual I/O causes an error, we do indeed panic. | ||
let result = match LOCAL_STDOUT.state() { | ||
LocalKeyState::Uninitialized | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOCAL_STDOUT.with
would initialize data, so I think this branch should go together with the next arm.
EDIT: this is also fine, because if stdout ends up actually changed from the global default, it would also get initialised.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this branch could actually go either way, but I figured that we may as well avoid initializing TLS if we're not gonna use it anyway (e.g. as you also mentioned it'd just go to the same place regardless)
LGTM. Would r+. |
Do we still use LOCAL_STDOUT? Can we kill it instead? |
@sfackler currently it's used to still capture the stdout of tests, but that's the only use that I know of. |
ping r? @brson |
@bors r+ |
📌 Commit 8588654 has been approved by |
Currently if a print happens while a thread is being torn down it may cause a panic if the LOCAL_STDOUT TLS slot has been destroyed by that point. This adds a guard to check and prints to the process stdout if that's the case (as we do for if the slot is already borrowed). Closes #29488
Currently if a print happens while a thread is being torn down it may cause a
panic if the LOCAL_STDOUT TLS slot has been destroyed by that point. This adds a
guard to check and prints to the process stdout if that's the case (as we do for
if the slot is already borrowed).
Closes #29488