Merge pull request #7674 from jasongrout/terminalrework

Rework services terminal code

examples/terminal/src/index.ts

@@ -12,7 +12,7 @@ import '../index.css';
 
 import { DockPanel, Widget } from '@lumino/widgets';
 
-import { TerminalSession } from '@jupyterlab/services';
+import { TerminalManager } from '@jupyterlab/services';
 
 import { Terminal } from '@jupyterlab/terminal';
 
@@ -28,12 +28,13 @@ async function main(): Promise<void> {
     dock.fit();
   });
 
-  const s1 = await TerminalSession.startNew();
+  const manager = new TerminalManager();
+  const s1 = await manager.startNew();
   const term1 = new Terminal(s1, { theme: 'light' });
   term1.title.closable = true;
   dock.addWidget(term1);
 
-  const s2 = await TerminalSession.startNew();
+  const s2 = await manager.startNew();
   const term2 = new Terminal(s2, { theme: 'dark' });
   term2.title.closable = true;
   dock.addWidget(term2, { mode: 'tab-before' });

packages/docregistry/src/context.ts

@@ -548,7 +548,9 @@ export class Context<T extends DocumentRegistry.IModel>
           throw err;
         }
       )
-      .catch();
+      .catch(() => {
+        /* no-op */
+      });
   }
 
   /**

packages/services/examples/browser/src/terminal.ts

@@ -1,17 +1,18 @@
 // Copyright (c) Jupyter Development Team.
 // Distributed under the terms of the Modified BSD License.
 
-import { TerminalSession } from '@jupyterlab/services';
+import { Terminal, TerminalManager } from '@jupyterlab/services';
 
 import { log } from './log';
 
 export async function main() {
   log('Terminal');
 
   // See if terminals are available
-  if (TerminalSession.isAvailable()) {
+  if (Terminal.isAvailable()) {
+    let manager = new TerminalManager();
     // Create a named terminal session and send some data.
-    let session = await TerminalSession.startNew();
+    let session = await manager.startNew();
     session.send({ type: 'stdin', content: ['foo'] });
   }
 }

packages/services/src/kernel/default.ts

@@ -52,22 +52,6 @@ export class KernelConnection implements Kernel.IKernelConnection {
     this._username = options.username ?? '';
     this.handleComms = options.handleComms ?? true;
 
-    this.connectionStatusChanged.connect((sender, connectionStatus) => {
-      // Send pending messages and update status to be consistent.
-      if (connectionStatus === 'connected' && this.status !== 'dead') {
-        // Make sure we send at least one message to get kernel status back.
-        if (this._pendingMessages.length > 0) {
-          this._sendPending();
-        } else {
-          void this.requestKernelInfo();
-        }
-      } else {
-        // If the connection is down, then we don't know what is happening with
-        // the kernel, so set the status to unknown.
-        this._updateStatus('unknown');
-      }
-    });
-
     this._createSocket();
 
     // Immediately queue up a request for initial kernel info.
@@ -264,7 +248,6 @@ export class KernelConnection implements Kernel.IKernelConnection {
     this._updateConnectionStatus('disconnected');
     this._clearKernelState();
     this._clearSocket();
-    clearTimeout(this._reconnectTimeout);
 
     // Clear Lumino signals
     Signal.clearData(this);
@@ -398,10 +381,8 @@ export class KernelConnection implements Kernel.IKernelConnection {
     // Send if the ws allows it, otherwise buffer the message.
     if (this.connectionStatus === 'connected') {
       this._ws!.send(serialize.serialize(msg));
-      // console.log(`SENT WS message to ${this.id}`, msg);
     } else if (queue) {
       this._pendingMessages.push(msg);
-      // console.log(`PENDING WS message to ${this.id}`, msg);
     } else {
       throw new Error('Could not send message');
     }
@@ -1034,8 +1015,6 @@ export class KernelConnection implements Kernel.IKernelConnection {
    * Handle status iopub messages from the kernel.
    */
   private _updateStatus(status: KernelMessage.Status): void {
-    // "unknown" | "starting" | "idle" | "busy" | "restarting" | "autorestarting" | "dead"
-
     if (this._status === status || this._status === 'dead') {
       return;
     }
@@ -1194,14 +1173,16 @@ export class KernelConnection implements Kernel.IKernelConnection {
 
   /**
    * Create the kernel websocket connection and add socket status handlers.
-   *
-   * #### Notes
-   * You are responsible for clearing the old socket so that the handlers
-   * from it are gone. For example, calling ._clearSocket(), etc.
    */
   private _createSocket = () => {
     this._errorIfDisposed();
 
+    // Make sure the socket is clear
+    this._clearSocket();
+
+    // Update the connection status to reflect opening a new connection.
+    this._updateConnectionStatus('connecting');
+
     let settings = this.serverSettings;
     let partialUrl = URLExt.join(
       settings.wsUrl,
@@ -1246,6 +1227,28 @@ export class KernelConnection implements Kernel.IKernelConnection {
 
     this._connectionStatus = connectionStatus;
 
+    // If we are not 'connecting', reset any reconnection attempts.
+    if (connectionStatus !== 'connecting') {
+      this._reconnectAttempt = 0;
+      clearTimeout(this._reconnectTimeout);
+    }
+
+    if (this.status !== 'dead') {
+      if (connectionStatus === 'connected') {
+        // Send pending messages, and make sure we send at least one message
+        // to get kernel status back.
+        if (this._pendingMessages.length > 0) {
+          this._sendPending();
+        } else {
+          void this.requestKernelInfo();
+        }
+      } else {
+        // If the connection is down, then we do not know what is happening
+        // with the kernel, so set the status to unknown.
+        this._updateStatus('unknown');
+      }
+    }
+
     // Notify others that the connection status changed.
     this._connectionStatusChanged.emit(connectionStatus);
   }
@@ -1386,7 +1389,6 @@ export class KernelConnection implements Kernel.IKernelConnection {
    * Handle a websocket open event.
    */
   private _onWSOpen = (evt: Event) => {
-    this._reconnectAttempt = 0;
     this._updateConnectionStatus('connected');
   };
 

packages/services/src/kernel/manager.ts

@@ -87,6 +87,9 @@ export class KernelManager extends BaseManager implements Kernel.IManager {
    * Dispose of the resources used by the manager.
    */
   dispose(): void {
+    if (this.isDisposed) {
+      return;
+    }
     this._models.clear();
     this._kernelConnections.forEach(x => x.dispose());
     this._pollModels.dispose();
@@ -292,6 +295,14 @@ export class KernelManager extends BaseManager implements Kernel.IManager {
 
   private _onDisposed(kernelConnection: KernelConnection) {
     this._kernelConnections.delete(kernelConnection);
+    // A dispose emission could mean the server session is deleted, or that
+    // the kernel JS object is disposed and the kernel still exists on the
+    // server, so we refresh from the server to make sure we reflect the
+    // server state.
+
+    void this.refreshRunning().catch(() => {
+      /* no-op */
+    });
   }
 
   private _onStatusChanged(

packages/services/src/manager.ts

@@ -19,7 +19,7 @@ import { Session, SessionManager } from './session';
 
 import { Setting, SettingManager } from './setting';
 
-import { TerminalSession, TerminalManager } from './terminal';
+import { Terminal, TerminalManager } from './terminal';
 
 import { ServerConnection } from './serverconnection';
 
@@ -219,7 +219,7 @@ export namespace ServiceManager {
     /**
      * The terminals manager for the manager.
      */
-    readonly terminals: TerminalSession.IManager;
+    readonly terminals: Terminal.IManager;
 
     /**
      * The workspace manager for the manager.

packages/services/src/session/manager.ts

@@ -87,6 +87,9 @@ export class SessionManager extends BaseManager implements Session.IManager {
    * Dispose of the resources used by the manager.
    */
   dispose(): void {
+    if (this.isDisposed) {
+      return;
+    }
     this._models.clear();
     this._sessionConnections.forEach(x => x.dispose());
     this._pollModels.dispose();