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

Silence kill_kernel when no process is present #576

Merged
merged 1 commit into from
Aug 25, 2020

Conversation

kevin-bates
Copy link
Member

During a kernel's shutdown, a race condition can occur between the kernel manager and the kernel restarter such that the restarter sees the kernel process has terminated, yet it's view of the kernel manager still thinks its managing an instance. In such cases, the restarter initiates its restart sequence and, when attempting to kill the kernel, finds that there's now no kernel process being managed. In such conditions, the kernel manager was raising an exception indicating there's no process to kill. Instead, it should perform the same action as when there is a process to kill, only to have the signaling fail because there is no longer any process - which is to ignore that condition. This change essentially ignores the case when a final kill request (i.e., SIGKILL vs. SIGTERM) finds that a process is no longer being managed (i.e., self.has_kernel is None).

Fixes #575

@MSeal
Copy link
Contributor

MSeal commented Aug 25, 2020

Thanks. Do we need a patch release real quick for this change to get the improvement out?

@MSeal MSeal merged commit 60daf9e into jupyter:master Aug 25, 2020
@kevin-bates
Copy link
Member Author

Thank you David and Matt.

Do we need a patch release real quick for this change to get the improvement out?

Thanks for asking. Unless there are other changes already in the queue, I don't consider this a "release-triggering" PR. Looking at the set of unreleased PRs, I think #568 might be the only other "fix", but it strikes me as a bit more urgent than this PR.

Or perhaps @ahmadmustafaanis views the priority differently. Ahmad, do you feel this warrants an immediate patch release?

@MSeal
Copy link
Contributor

MSeal commented Aug 25, 2020

That's a good point on on #568. I should release for that change given it's been sitting fixed for a bit -- I can kick off a patch release this afternoon with both.

@kevin-bates kevin-bates deleted the silent-kill branch August 25, 2020 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exception in callback Error when closing the kernel
3 participants