Allow double (or more) interrupts (#12278)

* Allow double (or more) interrupts

* Fix other tests that were checking previous allowed state

Author: rchiodo

news/2 Fixes/10587.md

@@ -0,0 +1 @@
+Allow interrupt to happen more than once in a notebook or the interactive window.

src/client/datascience/interactive-common/interactiveBase.ts

@@ -430,8 +430,6 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
     @captureTelemetry(Telemetry.Interrupt)
     public async interruptKernel(): Promise<void> {
         if (this._notebook && !this.restartingKernel) {
-            this.restartingKernel = true;
-
             const status = this.statusProvider.set(
                 localize.DataScience.interruptKernelStatus(),
                 true,
@@ -448,7 +446,7 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
                 status.dispose();
 
                 // We timed out, ask the user if they want to restart instead.
-                if (result === InterruptResult.TimedOut) {
+                if (result === InterruptResult.TimedOut && !this.restartingKernel) {
                     const message = localize.DataScience.restartKernelAfterInterruptMessage();
                     const yes = localize.DataScience.restartKernelMessageYes();
                     const no = localize.DataScience.restartKernelMessageNo();
@@ -464,8 +462,6 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
                 status.dispose();
                 traceError(err);
                 this.applicationShell.showErrorMessage(err);
-            } finally {
-                this.restartingKernel = false;
             }
         }
     }

src/client/datascience/jupyter/jupyterNotebook.ts

@@ -167,7 +167,12 @@ export class JupyterNotebookBase implements INotebook {
     public get onKernelRestarted(): Event<void> {
         return this.kernelRestarted.event;
     }
+
+    public get onKernelInterrupted(): Event<void> {
+        return this.kernelInterrupted.event;
+    }
     private readonly kernelRestarted = new EventEmitter<void>();
+    private readonly kernelInterrupted = new EventEmitter<void>();
     private disposed = new EventEmitter<void>();
     private sessionStatusChanged: Disposable | undefined;
     private initializedMatplotlib = false;
@@ -522,6 +527,9 @@ export class JupyterNotebookBase implements INotebook {
                 // Cancel all other pending cells as we interrupted.
                 this.finishUncompletedCells();
 
+                // Fire event that we interrupted.
+                this.kernelInterrupted.fire();
+
                 // Indicate the interrupt worked.
                 return InterruptResult.Success;
             } catch (exc) {

src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts

@@ -72,6 +72,7 @@ export class GuestJupyterNotebook
         IJupyterKernelSpec | LiveKernelModel
     >().event;
     public onKernelRestarted = new EventEmitter<void>().event;
+    public onKernelInterrupted = new EventEmitter<void>().event;
     public onDisposed = new EventEmitter<void>().event;
     private _jupyterLab?: typeof import('@jupyterlab/services');
     private responseQueue: ResponseQueue = new ResponseQueue();

src/client/datascience/types.ts

@@ -182,6 +182,7 @@ export interface INotebook extends IAsyncDisposable {
     onDisposed: Event<void>;
     onKernelChanged: Event<IJupyterKernelSpec | LiveKernelModel>;
     onKernelRestarted: Event<void>;
+    onKernelInterrupted: Event<void>;
     clear(id: string): void;
     executeObservable(code: string, file: string, line: number, id: string, silent: boolean): Observable<ICell[]>;
     execute(

src/datascience-ui/history-react/interactivePanel.tsx

@@ -136,7 +136,6 @@ ${buildSettingsCss(this.props.settings)}`}</style>
                         <ImageButton
                             baseTheme={this.props.baseTheme}
                             onClick={this.props.restartKernel}
-                            disabled={this.props.busy}
                             tooltip={getLocString('DataScience.restartServer', 'Restart IPython kernel')}
                         >
                             <Image
@@ -148,7 +147,6 @@ ${buildSettingsCss(this.props.settings)}`}</style>
                         <ImageButton
                             baseTheme={this.props.baseTheme}
                             onClick={this.props.interruptKernel}
-                            disabled={this.props.busy}
                             tooltip={getLocString('DataScience.interruptKernel', 'Interrupt IPython kernel')}
                         >
                             <Image

src/datascience-ui/native-editor/toolbar.tsx

@@ -160,7 +160,7 @@ export class Toolbar extends React.PureComponent<INativeEditorToolbarProps> {
                         <ImageButton
                             baseTheme={this.props.baseTheme}
                             onClick={this.props.restartKernel}
-                            disabled={this.props.busy || !canRestartAndInterruptKernel}
+                            disabled={!canRestartAndInterruptKernel}
                             className="native-button"
                             tooltip={getLocString('DataScience.restartServer', 'Restart IPython kernel')}
                         >
@@ -173,7 +173,7 @@ export class Toolbar extends React.PureComponent<INativeEditorToolbarProps> {
                         <ImageButton
                             baseTheme={this.props.baseTheme}
                             onClick={this.props.interruptKernel}
-                            disabled={this.props.busy || !canRestartAndInterruptKernel}
+                            disabled={!canRestartAndInterruptKernel}
                             className="native-button"
                             tooltip={getLocString('DataScience.interruptKernel', 'Interrupt IPython kernel')}
                         >

src/test/datascience/interactiveWindow.functional.test.tsx

@@ -12,7 +12,7 @@ import { Disposable, Selection, TextDocument, TextEditor, Uri } from 'vscode';
 import { ReactWrapper } from 'enzyme';
 import { IApplicationShell, IDocumentManager } from '../../client/common/application/types';
 import { IDataScienceSettings } from '../../client/common/types';
-import { createDeferred, waitForPromise } from '../../client/common/utils/async';
+import { createDeferred, sleep, waitForPromise } from '../../client/common/utils/async';
 import { noop } from '../../client/common/utils/misc';
 import { EXTENSION_ROOT_DIR } from '../../client/constants';
 import { generateCellsFromDocument } from '../../client/datascience/cellFactory';
@@ -555,6 +555,52 @@ Type:      builtin_function_or_method`,
         }
     );
 
+    const interruptCode = `
+import time
+for i in range(0, 100):
+    try:
+        time.sleep(0.5)
+    except KeyboardInterrupt:
+        time.sleep(0.5)`;
+
+    runMountedTest(
+        'Interrupt double',
+        async (wrapper) => {
+            let interruptedKernel = false;
+            const interactiveWindow = await getOrCreateInteractiveWindow(ioc);
+            await interactiveWindow.show();
+            interactiveWindow.notebook?.onKernelInterrupted(() => (interruptedKernel = true));
+
+            let timerCount = 0;
+            addContinuousMockData(ioc, interruptCode, async (_c) => {
+                timerCount += 1;
+                await sleep(0.5);
+                return Promise.resolve({ result: '', haveMore: timerCount < 100 });
+            });
+
+            addMockData(ioc, interruptCode, undefined, 'text/plain');
+
+            // Run the interrupt code and then interrupt it twice to make sure we can interrupt twice
+            const waitForAdd = addCode(ioc, wrapper, interruptCode);
+
+            // 'Click' the button in the react control. We need to verify we can
+            // click it more than once.
+            const interrupt = findButton(wrapper, InteractivePanel, 4);
+            interrupt?.simulate('click');
+            await sleep(0.1);
+            interrupt?.simulate('click');
+
+            // We should get out of the wait for add
+            await waitForAdd;
+
+            // We should have also fired an interrupt
+            assert.ok(interruptedKernel, 'Kernel was not interrupted');
+        },
+        () => {
+            return ioc;
+        }
+    );
+
     runMountedTest(
         'Export',
         async (wrapper) => {

src/test/datascience/mockJupyterNotebook.ts

@@ -51,6 +51,10 @@ export class MockJupyterNotebook implements INotebook {
     >().event;
     public onDisposed = new EventEmitter<void>().event;
     public onKernelRestarted = new EventEmitter<void>().event;
+    public get onKernelInterrupted(): Event<void> {
+        return this.kernelInterrupted.event;
+    }
+    private kernelInterrupted = new EventEmitter<void>();
     private onStatusChangedEvent: EventEmitter<ServerStatus> | undefined;
 
     constructor(private providerConnection: INotebookProviderConnection | undefined) {

src/test/datascience/nativeEditor.toolbar.functional.test.tsx

@@ -176,13 +176,6 @@ suite('DataScience Native Toolbar', () => {
     suite('Restart & Interrupt Kernel', () => {
         getNamesAndValues<ServerStatus>(ServerStatus).forEach((status) => {
             // Should always be disabled if busy.
-            test(`If Kernel status is ${status.name} and busy, both are disabled`, () => {
-                props.kernel.jupyterServerStatus = status.name as any;
-                props.busy = true;
-                mountToolbar();
-                assertDisabled(Button.RestartKernel);
-                assertDisabled(Button.InterruptKernel);
-            });
             if (status.name === ServerStatus.NotStarted) {
                 // Should be disabled if not busy and status === 'Not Started'.
                 test(`If Kernel status is ${ServerStatus.NotStarted} and not busy, both are disabled`, () => {
@@ -193,10 +186,10 @@ suite('DataScience Native Toolbar', () => {
                     assertDisabled(Button.InterruptKernel);
                 });
             } else {
-                // Should be enabled if not busy and status != 'Not Started'.
-                test(`If Kernel status is ${status.name} and not busy, both are enabled`, () => {
+                // Should be enabled if busy and status != 'Not Started'.
+                test(`If Kernel status is ${status.name}, both are enabled`, () => {
                     props.kernel.jupyterServerStatus = status.name as any;
-                    props.busy = false;
+                    props.busy = true;
                     mountToolbar();
                     assertEnabled(Button.RestartKernel);
                     assertEnabled(Button.InterruptKernel);