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
Move to having the Scheduler directly handle SIGINT to cancel Sessions #11399
Move to having the Scheduler directly handle SIGINT to cancel Sessions #11399
Conversation
…evant Sessions. # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
pub fn new(executor: &Executor) -> Result<Sessions, String> { | ||
let sessions: Arc<Mutex<Vec<Weak<InnerSession>>>> = Arc::default(); | ||
let signal_task_abort_handle = { | ||
let mut signal_stream = signal(SignalKind::interrupt()) |
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.
LGTM. My only worry in the small was what happens when new gets called more than once in the same process (I don't think that can happen though from an in the large view of the code). Even if this occurred, the tokio docs says its ok and events just get delivered to all signal streams and thats fine if it happens since AsyncLatch will only fire once and not error after.
…evant Sessions. (pantsbuild#11399) ### Problem pantsbuild#11223 moved to using `Session::cancel` to interrupt 1) `InteractiveProcess`, 2) work happening in `Scheduler::execute`. In the context of `pantsd`, `session.cancel()` is called when the read-half of the `pantsd` connection is closed. When `pantsd` is not used, `ExceptionSink` was expected to run `sessions_cancel_all` to cancel all Sessions. But as demonstrated in pantsbuild#11359 (and as we previously knew from @gshuflin's work), Python signal handlers do not run until you interact with certain types of Python code... and none of this code was present directly inside of the `Scheduler::execute` loop (although `InteractiveProcess` instances were appropriately interrupted). ### Solution Move SIGINT handling (for this usecase) out of `ExceptionSink` and into the rust `Scheduler` code in a new `Sessions` struct that cancels all active `Sessions` each time a SIGINT arrives. ### Result Fixes pantsbuild#11359. [ci skip-build-wheels]
…evant Sessions. (cherrypick of #11399) (#11400) ### Problem #11223 moved to using `Session::cancel` to interrupt 1) `InteractiveProcess`, 2) work happening in `Scheduler::execute`. In the context of `pantsd`, `session.cancel()` is called when the read-half of the `pantsd` connection is closed. When `pantsd` is not used, `ExceptionSink` was expected to run `sessions_cancel_all` to cancel all Sessions. But as demonstrated in #11359 (and as we previously knew from @gshuflin's work), Python signal handlers do not run until you interact with certain types of Python code... and none of this code was present directly inside of the `Scheduler::execute` loop (although `InteractiveProcess` instances were appropriately interrupted). ### Solution Move SIGINT handling (for this usecase) out of `ExceptionSink` and into the rust `Scheduler` code in a new `Sessions` struct that cancels all active `Sessions` each time a SIGINT arrives. ### Result Fixes #11359. [ci skip-build-wheels]
Problem
#11223 moved to using
Session::cancel
to interrupt 1)InteractiveProcess
, 2) work happening inScheduler::execute
.In the context of
pantsd
,session.cancel()
is called when the read-half of thepantsd
connection is closed. Whenpantsd
is not used,ExceptionSink
was expected to runsessions_cancel_all
to cancel all Sessions.But as demonstrated in #11359 (and as we previously knew from @gshuflin's work), Python signal handlers do not run until you interact with certain types of Python code... and none of this code was present directly inside of the
Scheduler::execute
loop (althoughInteractiveProcess
instances were appropriately interrupted).Solution
Move SIGINT handling (for this usecase) out of
ExceptionSink
and into the rustScheduler
code in a newSessions
struct that cancels all activeSessions
each time a SIGINT arrives.Result
Fixes #11359.
[ci skip-build-wheels]