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

Exit the process a short time after entering our ctrl-c handler #125523

Merged
merged 1 commit into from
May 26, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions compiler/rustc_driver_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ use std::process::{self, Command, Stdio};
use std::str;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, OnceLock};
use std::time::{Instant, SystemTime};
use std::time::{Duration, Instant, SystemTime};
use time::OffsetDateTime;
use tracing::trace;

Expand Down Expand Up @@ -1502,14 +1502,13 @@ pub fn init_logger(early_dcx: &EarlyDiagCtxt, cfg: rustc_log::LoggerConfig) {
pub fn install_ctrlc_handler() {
#[cfg(not(target_family = "wasm"))]
ctrlc::set_handler(move || {
// Indicate that we have been signaled to stop. If we were already signaled, exit
// immediately. In our interpreter loop we try to consult this value often, but if for
// whatever reason we don't get to that check or the cleanup we do upon finding that
// this bool has become true takes a long time, the exit here will promptly exit the
// process on the second Ctrl-C.
if CTRL_C_RECEIVED.swap(true, Ordering::Relaxed) {
std::process::exit(1);
}
// Indicate that we have been signaled to stop, then give the rest of the compiler a bit of
// time to check CTRL_C_RECEIVED and run its own shutdown logic, but after a short amount
// of time exit the process. This sleep+exit ensures that even if nobody is checking
// CTRL_C_RECEIVED, the compiler exits reasonably promptly.
CTRL_C_RECEIVED.store(true, Ordering::Relaxed);
std::thread::sleep(Duration::from_millis(100));
Copy link
Member

Choose a reason for hiding this comment

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

this has a change in behavior which I think may not be intentional: if the user hits Ctrl C twice, it will still wait 100 ms instead of exiting immediately

Copy link
Member Author

@saethlin saethlin May 25, 2024

Choose a reason for hiding this comment

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

It's intentional. You could definitely argue that it is bad, if you want.

I added the behavior of immediately exiting on the second ctrl-c because I thought it would be the obvious thing to try if the compiler didn't respond to the first. At least the most recent issue #125428 indicates this is either not obvious or prevented in some scenarios.

100 ms is a long time for a program, but significantly shorter than even an athlete's reaction time. Unless the OS scheduler truly hates us, the process should exit before the user even has time to realize the process didn't immediately exit and send a second ctrl-c.

std::process::exit(1);
})
.expect("Unable to install ctrlc handler");
}
Expand Down
Loading