Implement a guard for pending user input to avoid deadlocks (#8713)

* Add hadPendingInput flag as guard agains multiple cell run while waiting on user input

* pendingInput guard on kernel level

* Modal Stdin

* Better typedoc for pending input

* Add pendingInput test

* remove undeeded code and add docstring

* Fixing tests

* Add translation

* Fixing tests

Co-authored-by: goanpeca <goanpeca@gmail.com>

jupyterlab/tests/echo_kernel.py

@@ -18,11 +18,19 @@ class EchoKernel(Kernel):
     banner = "Echo kernel - as useful as a parrot"
 
     def do_execute(self, code, silent, store_history=True,
-            user_expressions=None, allow_stdin=False):
+                   user_expressions=None, allow_stdin=False):
         if not silent:
             stream_content = {'name': 'stdout', 'text': code}
             self.send_response(self.iopub_socket, 'stream', stream_content)
 
+            # Send a input_request if code contains input command.
+            if allow_stdin and code and code.find('input(') != -1:
+                self._input_request('Echo Prompt',
+                    self._parent_ident,
+                    self._parent_header,
+                    password=False,
+                )
+
         return {'status': 'ok',
                 # The base class increments the execution count
                 'execution_count': self.execution_count,

packages/apputils/src/sessioncontext.tsx

@@ -106,6 +106,11 @@ export interface ISessionContext extends IObservableDisposable {
    */
   readonly connectionStatusChanged: ISignal<this, Kernel.ConnectionStatus>;
 
+  /**
+   * A flag indicating if session is has pending input, proxied from the session connection.
+   */
+  readonly pendingInput: boolean;
+
   /**
    * A signal emitted for a kernel messages, proxied from the session connection.
    */
@@ -394,6 +399,13 @@ export class SessionContext implements ISessionContext {
     return this._connectionStatusChanged;
   }
 
+  /**
+   * A flag indicating if the session has ending input, proxied from the kernel.
+   */
+  get pendingInput(): boolean {
+    return this._pendingInput;
+  }
+
   /**
    * A signal emitted for iopub kernel messages, proxied from the kernel.
    */
@@ -881,6 +893,7 @@ export class SessionContext implements ISessionContext {
         this._onConnectionStatusChanged,
         this
       );
+      session.pendingInput.connect(this._onPendingInput, this);
       session.iopubMessage.connect(this._onIopubMessage, this);
       session.unhandledMessage.connect(this._onUnhandledMessage, this);
 
@@ -1031,6 +1044,17 @@ export class SessionContext implements ISessionContext {
     this._connectionStatusChanged.emit(status);
   }
 
+  /**
+   * Handle a change to the pending input.
+   */
+  private _onPendingInput(
+    sender: Session.ISessionConnection,
+    value: boolean
+  ): void {
+    // Set the signal value
+    this._pendingInput = value;
+  }
+
   /**
    * Handle an iopub message.
    */
@@ -1082,9 +1106,9 @@ export class SessionContext implements ISessionContext {
   private _connectionStatusChanged = new Signal<this, Kernel.ConnectionStatus>(
     this
   );
-
   private translator: ITranslator;
   private _trans: TranslationBundle;
+  private _pendingInput = false;
   private _iopubMessage = new Signal<this, KernelMessage.IIOPubMessage>(this);
   private _unhandledMessage = new Signal<this, KernelMessage.IMessage>(this);
   private _propertyChanged = new Signal<this, 'path' | 'name' | 'type'>(this);

packages/notebook/src/actions.tsx

@@ -1858,6 +1858,16 @@ namespace Private {
             });
             break;
           }
+          if (sessionContext.pendingInput) {
+            void showDialog({
+              title: trans.__('Waiting on User Input'),
+              body: trans.__(
+                'Did not run selected cell because there is a cell waiting on input! Submit your input and try again.'
+              ),
+              buttons: [Dialog.okButton({ label: trans.__('Ok') })]
+            });
+            return Promise.resolve(false);
+          }
           const deletedCells = notebook.model?.deletedCells ?? [];
           return CodeCell.execute(cell as CodeCell, sessionContext, {
             deletedCells,

packages/outputarea/src/widget.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Jupyter Development Team.
 // Distributed under the terms of the Modified BSD License.
 
-import { ISessionContext } from '@jupyterlab/apputils';
+import { Dialog, ISessionContext } from '@jupyterlab/apputils';
 import * as nbformat from '@jupyterlab/nbformat';
 import { IOutputModel, IRenderMimeRegistry } from '@jupyterlab/rendermime';
 import { IRenderMime } from '@jupyterlab/rendermime-interfaces';
@@ -381,6 +381,40 @@ export class OutputArea extends Widget {
     const layout = this.layout as PanelLayout;
     layout.addWidget(panel);
 
+    const buttons = [
+      Dialog.cancelButton({ label: 'Cancel' }),
+      Dialog.okButton({ label: 'Select' })
+    ];
+
+    const modalInput = factory.createStdin({
+      prompt: stdinPrompt,
+      password,
+      future
+    });
+
+    const dialog = new Dialog<Promise<string>>({
+      title: 'Your input is requested',
+      body: modalInput,
+      buttons
+    });
+
+    void dialog.launch().then(result => {
+      if (result.button.accept) {
+        const value = result.value;
+        if (value) {
+          void value.then(v => {
+            // Use stdin as the stream so it does not get combined with stdout.
+            this.model.add({
+              output_type: 'stream',
+              name: 'stdin',
+              text: v + '\n'
+            });
+            panel.dispose();
+          });
+        }
+      }
+    });
+
     /**
      * Wait for the stdin to complete, add it to the model (so it persists)
      * and remove the stdin widget.
@@ -859,10 +893,19 @@ export class Stdin extends Widget implements IStdin {
   /**
    * The value of the widget.
    */
-  get value() {
+  get value(): Promise<string> {
     return this._promise.promise.then(() => this._value);
   }
 
+  /**
+   * Submit and get the value of the Stdin widget.
+   */
+  getValue(): Promise<string> {
+    console.log('WHAT THE FUCK!');
+    this.submit();
+    return this.value;
+  }
+
   /**
    * Handle the DOM events for the widget.
    *
@@ -874,24 +917,31 @@ export class Stdin extends Widget implements IStdin {
    * not be called directly by user code.
    */
   handleEvent(event: Event): void {
-    const input = this._input;
     if (event.type === 'keydown') {
       if ((event as KeyboardEvent).keyCode === 13) {
         // Enter
-        this._future.sendInputReply({
-          status: 'ok',
-          value: input.value
-        });
-        if (input.type === 'password') {
-          this._value += Array(input.value.length + 1).join('·');
-        } else {
-          this._value += input.value;
-        }
-        this._promise.resolve(void 0);
+        this.submit();
       }
     }
   }
 
+  /*
+   * Submit input value.
+   */
+  submit() {
+    const input = this._input;
+    this._future.sendInputReply({
+      status: 'ok',
+      value: input.value
+    });
+    if (input.type === 'password') {
+      this._value += Array(input.value.length + 1).join('·');
+    } else {
+      this._value += input.value;
+    }
+    this._promise.resolve(void 0);
+  }
+
   /**
    * Handle `after-attach` messages sent to the widget.
    */

packages/services/src/kernel/default.ts

@@ -144,6 +144,13 @@ export class KernelConnection implements Kernel.IKernelConnection {
     return this._anyMessage;
   }
 
+  /**
+   * A signal emitted when a kernel has pending inputs from the user.
+   */
+  get pendingInput(): ISignal<this, boolean> {
+    return this._pendingInput;
+  }
+
   /**
    * The id of the server-side kernel.
    */
@@ -816,6 +823,8 @@ export class KernelConnection implements Kernel.IKernelConnection {
 
     this._sendMessage(msg);
     this._anyMessage.emit({ msg, direction: 'send' });
+
+    this.hasPendingInput = false;
   }
 
   /**
@@ -1502,6 +1511,14 @@ export class KernelConnection implements Kernel.IKernelConnection {
     }
   };
 
+  get hasPendingInput(): boolean {
+    return this._hasPendingInput;
+  }
+  set hasPendingInput(value: boolean) {
+    this._hasPendingInput = value;
+    this._pendingInput.emit(value);
+  }
+
   private _id = '';
   private _name = '';
   private _status: KernelMessage.Status = 'unknown';
@@ -1542,10 +1559,12 @@ export class KernelConnection implements Kernel.IKernelConnection {
   private _disposed = new Signal<this, void>(this);
   private _iopubMessage = new Signal<this, KernelMessage.IIOPubMessage>(this);
   private _anyMessage = new Signal<this, Kernel.IAnyMessageArgs>(this);
+  private _pendingInput = new Signal<this, boolean>(this);
   private _unhandledMessage = new Signal<this, KernelMessage.IMessage>(this);
   private _displayIdToParentIds = new Map<string, string[]>();
   private _msgIdToDisplayIds = new Map<string, string[]>();
   private _msgChain: Promise<void> = Promise.resolve();
+  private _hasPendingInput = false;
   private _noOp = () => {
     /* no-op */
   };

packages/services/src/kernel/future.ts

@@ -238,6 +238,7 @@ export abstract class KernelFutureHandler<
   }
 
   private async _handleStdin(msg: KernelMessage.IStdinMessage): Promise<void> {
+    this._kernel.hasPendingInput = true;
     const stdin = this._stdin;
     if (stdin) {
       // tslint:disable-next-line:await-promise

packages/services/src/kernel/kernel.ts

@@ -110,6 +110,17 @@ export interface IKernelConnection extends IObservableDisposable {
    */
   handleComms: boolean;
 
+  /**
+   * Whether the kernel connection has pending input.
+   *
+   * #### Notes
+   * This is a guard to avoid deadlock is the user asks input
+   * as second time before submitting his first input
+   *
+   * See https://github.com/jupyterlab/jupyterlab/issues/8632
+   */
+  hasPendingInput: boolean;
+
   /**
    * Send a shell message to the kernel.
    *
@@ -486,6 +497,11 @@ export interface IKernelConnection extends IObservableDisposable {
    */
   anyMessage: ISignal<this, IAnyMessageArgs>;
 
+  /**
+   * A signal emitted when a kernel has pending inputs from the user.
+   */
+  pendingInput: ISignal<this, boolean>;
+
   /**
    * The server settings for the kernel.
    */

packages/services/src/session/default.ts

@@ -70,6 +70,13 @@ export class SessionConnection implements Session.ISessionConnection {
     return this._connectionStatusChanged;
   }
 
+  /**
+   * A signal proxied from the kernel pending input.
+   */
+  get pendingInput(): ISignal<this, boolean> {
+    return this._pendingInput;
+  }
+
   /**
    * A signal proxied from the kernel about iopub kernel messages.
    */
@@ -315,6 +322,7 @@ export class SessionConnection implements Session.ISessionConnection {
     this._kernel = kc;
     kc.statusChanged.connect(this.onKernelStatus, this);
     kc.connectionStatusChanged.connect(this.onKernelConnectionStatus, this);
+    kc.pendingInput.connect(this.onPendingInput, this);
     kc.unhandledMessage.connect(this.onUnhandledMessage, this);
     kc.iopubMessage.connect(this.onIOPubMessage, this);
     kc.anyMessage.connect(this.onAnyMessage, this);
@@ -340,6 +348,13 @@ export class SessionConnection implements Session.ISessionConnection {
     this._connectionStatusChanged.emit(state);
   }
 
+  /**
+   * Handle a change in the pendingInput.
+   */
+  protected onPendingInput(sender: Kernel.IKernelConnection, state: boolean) {
+    this._pendingInput.emit(state);
+  }
+
   /**
    * Handle iopub kernel messages.
    */
@@ -416,6 +431,7 @@ export class SessionConnection implements Session.ISessionConnection {
   private _connectionStatusChanged = new Signal<this, Kernel.ConnectionStatus>(
     this
   );
+  private _pendingInput = new Signal<this, boolean>(this);
   private _iopubMessage = new Signal<this, KernelMessage.IIOPubMessage>(this);
   private _unhandledMessage = new Signal<this, KernelMessage.IMessage>(this);
   private _anyMessage = new Signal<this, Kernel.IAnyMessageArgs>(this);

packages/services/src/session/session.ts

@@ -55,6 +55,12 @@ export interface ISessionConnection extends IObservableDisposable {
    */
   connectionStatusChanged: ISignal<this, Kernel.ConnectionStatus>;
 
+  /**
+   * The kernel pendingInput signal, proxied from the current
+   * kernel.
+   */
+  pendingInput: ISignal<this, boolean>;
+
   /**
    * The kernel iopubMessage signal, proxied from the current kernel.
    */

packages/services/test/kernel/ikernel.spec.ts

@@ -92,6 +92,27 @@ describe('Kernel.IKernel', () => {
     });
   });
 
+  describe('#pendingInput', () => {
+    it('should be a signal following input request', async () => {
+      let called = false;
+      defaultKernel.pendingInput.connect((sender, args) => {
+        if (!called) {
+          called = true;
+          defaultKernel.sendInputReply({ status: 'ok', value: 'foo' });
+        }
+      });
+      const code = `input("Input something")`;
+      await defaultKernel.requestExecute(
+        {
+          code: code,
+          allow_stdin: true
+        },
+        true
+      ).done;
+      expect(called).toBe(true);
+    });
+  });
+
   describe('#iopubMessage', () => {
     it('should be emitted for an iopub message', async () => {
       let called = false;

testutils/src/mock.ts

@@ -245,8 +245,13 @@ export const KernelMock = jest.fn<
     Kernel.IKernelConnection,
     Kernel.Status
   >(thisObject);
+  const pendingInputSignal = new Signal<Kernel.IKernelConnection, boolean>(
+    thisObject
+  );
   (thisObject as any).statusChanged = statusChangedSignal;
   (thisObject as any).iopubMessage = iopubMessageSignal;
+  (thisObject as any).pendingInput = pendingInputSignal;
+  (thisObject as any).hasPendingInput = false;
   return thisObject;
 });
 
@@ -329,6 +334,10 @@ export const SessionConnectionMock = jest.fn<
     KernelMessage.IMessage
   >(thisObject);
 
+  const pendingInputSignal = new Signal<Session.ISessionConnection, boolean>(
+    thisObject
+  );
+
   kernel!.iopubMessage.connect((_, args) => {
     iopubMessageSignal.emit(args);
   }, thisObject);
@@ -337,6 +346,10 @@ export const SessionConnectionMock = jest.fn<
     statusChangedSignal.emit(args);
   }, thisObject);
 
+  kernel!.pendingInput.connect((_, args) => {
+    pendingInputSignal.emit(args);
+  }, thisObject);
+
   (thisObject as any).disposed = disposedSignal;
   (thisObject as any).connectionStatusChanged = connectionStatusChangedSignal;
   (thisObject as any).propertyChanged = propertyChangedSignal;
@@ -344,6 +357,7 @@ export const SessionConnectionMock = jest.fn<
   (thisObject as any).kernelChanged = kernelChangedSignal;
   (thisObject as any).iopubMessage = iopubMessageSignal;
   (thisObject as any).unhandledMessage = unhandledMessageSignal;
+  (thisObject as any).pendingInput = pendingInputSignal;
   return thisObject;
 });
 
@@ -420,12 +434,17 @@ export const SessionContextMock = jest.fn<
     kernelChangedSignal.emit(args);
   });
 
+  session!.pendingInput.connect((_, args) => {
+    (thisObject as any).pendingInput = args;
+  });
+
   (thisObject as any).statusChanged = statusChangedSignal;
   (thisObject as any).kernelChanged = kernelChangedSignal;
   (thisObject as any).iopubMessage = iopubMessageSignal;
   (thisObject as any).propertyChanged = propertyChangedSignal;
   (thisObject as any).disposed = disposedSignal;
   (thisObject as any).session = session;
+  (thisObject as any).pendingInput = false;
 
   return thisObject;
 });