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

Kernel interrupt indicates kernel crashed when it didn't #2318

Closed
jmew opened this issue Feb 11, 2020 · 8 comments
Closed

Kernel interrupt indicates kernel crashed when it didn't #2318

jmew opened this issue Feb 11, 2020 · 8 comments
Assignees

Comments

@jmew
Copy link
Contributor

jmew commented Feb 11, 2020

interrupting the kernel will cause the kernel to crash, which then loses the entire kernel context of previous execution

Repro steps in screenshot below.

image

@rchiodo
Copy link
Contributor

rchiodo commented Feb 12, 2020

This is by design. A kernel interrupt is throwing an exception. If the python code running doesn't handle it, it can cause the entire process to die.

Jupyter behaves this way too.

@rchiodo rchiodo closed this as completed Feb 12, 2020
@jmew
Copy link
Contributor Author

jmew commented Feb 12, 2020

I apologize if im missing something,

But in Jupyter, the kernel doesnt get restarted if it is interrupted. The context remains. E.g. in the example below, the kernel maintains reference to the x=1 execution.
image

But in the interactive window, the context is lost (e.g. the kernel loses context that x=1 was run previously)
image

@jmew jmew reopened this Feb 12, 2020
@rchiodo
Copy link
Contributor

rchiodo commented Feb 12, 2020

That's just luck. We do nothing different than jupyter does.

@jmew jmew closed this as completed Feb 12, 2020
@rchiodo
Copy link
Contributor

rchiodo commented Feb 12, 2020

Actually I think we're showing the message even if there isn't a restart. The kernel didn't actually restart.

@rchiodo rchiodo reopened this Feb 12, 2020
@rchiodo
Copy link
Contributor

rchiodo commented Feb 12, 2020

So the bug here is the message showing up, not the kernel restarting :)

@rchiodo rchiodo changed the title Kernel interrupt causes kernel to crash Kernel interrupt indicates kernel crashed when it didn't Feb 12, 2020
@rchiodo
Copy link
Contributor

rchiodo commented Feb 12, 2020

Looking at the code, looks like there's a number of problems:

  1. Restart handler in interruptKernel doesn't use the status from the restart handler (Should look like this)
            const restartHandler = (e: ServerStatus) => {
                if (e === ServerStatus.Restarting) {
  1. When the interrupt finishes, the 'then' clause is indicating a restart. It should do nothing.
            // Start our interrupt. If it fails, indicate a restart
            this.session
                .interrupt(timeoutMs)
                .then(() => restarted.resolve([]))
                .catch(exc => {
                    traceWarning(`Error during interrupt: ${exc}`);
                    restarted.resolve([]);
                });

@greazer
Copy link
Member

greazer commented Feb 12, 2020

Not fixing this will cause customers to not trust the tool.

@rchiodo rchiodo self-assigned this Feb 19, 2020
@IanMatthewHuff IanMatthewHuff self-assigned this Feb 25, 2020
@IanMatthewHuff
Copy link
Member

Validated. No more restart message when the kernel was interrupted.
image.png

@lock lock bot locked as resolved and limited conversation to collaborators Mar 10, 2020
@microsoft microsoft unlocked this conversation Nov 13, 2020
@DonJayamanne DonJayamanne transferred this issue from microsoft/vscode-python Nov 13, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants