🗑 feat/dispose cycles (#614)

* only disposed if not already disposed
* enable faux disconnect
* enable shutdown with local state
* restart any existing session
* core may be undefined on disconnect request
* 🧹tidy
* 🤖 simplify passive hide widgets arg

packages/core/src/manager.ts

@@ -77,7 +77,6 @@ export class ThebeManager extends KernelWidgetManager {
   }
 
   async display_view(msg: any, view: any, options: any): Promise<Widget> {
-    console.log('widget display_view', { view, options });
     if (options.el) {
       LuminoWidget.Widget.attach(view.luminoWidget, options.el);
     }

packages/core/src/passive.ts

@@ -44,24 +44,17 @@ function stripWidgets(outputs: nbformat.IOutput[], hideWidgets?: boolean) {
 class PassiveCellRenderer implements IPassiveCell {
   readonly id: string;
   readonly rendermime: IRenderMimeRegistry;
-  readonly hideWidgets: boolean;
   protected model: OutputAreaModel;
   protected area: OutputArea;
 
-  constructor(
-    id: string,
-    rendermime?: IRenderMimeRegistry,
-    mathjax?: MathjaxOptions,
-    hideWidgets = false,
-  ) {
+  constructor(id: string, rendermime?: IRenderMimeRegistry, mathjax?: MathjaxOptions) {
     this.id = id;
     this.rendermime = rendermime ?? getRenderMimeRegistry(mathjax ?? makeMathjaxOptions());
     this.model = new OutputAreaModel({ trusted: true });
     this.area = new OutputArea({
       model: this.model,
       rendermime: this.rendermime,
     });
-    this.hideWidgets = hideWidgets;
   }
 
   /**
@@ -151,8 +144,8 @@ class PassiveCellRenderer implements IPassiveCell {
    * @param outputs - serialised jupyter outputs
    * @returns
    */
-  render(outputs: nbformat.IOutput[]) {
-    this.model.fromJSON(stripWidgets(outputs, this.hideWidgets));
+  render(outputs: nbformat.IOutput[], hideWidgets?: boolean) {
+    this.model.fromJSON(stripWidgets(outputs, hideWidgets));
   }
 }
 

packages/core/src/server.ts

@@ -71,8 +71,8 @@ class ThebeServer implements ServerRuntime, ServerRestAPI {
 
   dispose() {
     if (this._isDisposed) return;
-    this.serviceManager?.dispose();
-    this.sessionManager?.dispose();
+    if (!this.serviceManager?.isDisposed) this.serviceManager?.dispose();
+    if (!this.sessionManager?.isDisposed) this.sessionManager?.dispose();
 
     // Implementing the flag at this level as
     this._isDisposed = true;

packages/core/src/session.ts

@@ -59,16 +59,17 @@ class ThebeSession {
   }
 
   async shutdown() {
+    if (this.connection.isDisposed) return;
     await this.connection.shutdown();
     this.events.triggerStatus({
       status: SessionStatusEvent.shutdown,
       message: `session ${this.name}`,
     });
-    return this.connection.dispose();
+    this.dispose();
   }
 
   dispose() {
-    this.connection.dispose();
+    if (!this.connection.isDisposed) this.connection.dispose();
   }
 }
 

packages/core/src/types.ts

@@ -81,7 +81,7 @@ export interface IPassiveCell {
   setOutputText(text: string): void;
   clear(): void;
   clearOnError(error?: any): void;
-  render(outputs: IOutput[]): void;
+  render(outputs: IOutput[], hideWidgets?: boolean): void;
 }
 
 export interface IThebeCell extends IPassiveCell {

packages/react/src/ThebeServerProvider.tsx

@@ -19,6 +19,7 @@ export const ThebeServerContext = React.createContext<
       connecting: boolean;
       ready: boolean;
       connect: () => void;
+      disconnect: () => Promise<void>;
     }
   | undefined
 >(undefined);
@@ -42,6 +43,7 @@ export function ThebeServerProvider({
   const { core } = useThebeCore();
   const [doConnect, setDoConnect] = useState(connect);
   const [connecting, setConnecting] = useState<boolean>(false);
+  const [server, setServer] = useState<ThebeServer | undefined>();
   const [ready, setReady] = useState<boolean>(false);
 
   // create a valid configuration, either using the one supplied
@@ -52,11 +54,9 @@ export function ThebeServerProvider({
     [core, options],
   );
 
-  // because core is async, server is still maybe undefined, but can be created before
-  // the connection function is kicked off
-  const server = useMemo(() => {
+  useEffect(() => {
     if (!core || !thebeConfig) return;
-    return new core.ThebeServer(thebeConfig);
+    setServer(new core.ThebeServer(thebeConfig));
   }, [core, thebeConfig]);
 
   // Once the core is loaded, connect to a server
@@ -82,6 +82,14 @@ export function ThebeServerProvider({
         connecting,
         ready: (server?.isReady ?? false) && ready, // TODO server status may change, affecting readiness
         connect: () => setDoConnect(true),
+        disconnect: async () => {
+          if (core && thebeConfig && server) {
+            server.dispose();
+            setServer(new core.ThebeServer(thebeConfig));
+          }
+          setReady(false);
+          setDoConnect(false);
+        },
       }}
     >
       {children}
@@ -124,7 +132,7 @@ export function useThebeServer() {
   if (serverContext === undefined) {
     throw new Error('useThebeServer must be used inside a ThebeServerProvider');
   }
-  const { config, events, server, connecting, ready, connect } = serverContext;
+  const { config, events, server, connecting, ready, connect, disconnect } = serverContext;
 
   const { core } = useThebeCore();
   const [error, setError] = useState<string | undefined>(); // TODO how to handle errors better via the provider
@@ -168,6 +176,7 @@ export function useThebeServer() {
     ready,
     error,
     connect,
+    disconnect,
     subscribe,
     unsubAll,
   };

packages/react/src/ThebeSessionProvider.tsx

@@ -8,7 +8,8 @@ interface ThebeSessionContextData {
   starting: boolean;
   ready: boolean;
   error?: string;
-  start: () => void;
+  start: () => Promise<void>;
+  shutdown: () => Promise<void>;
 }
 
 export const ThebeSessionContext = React.createContext<ThebeSessionContextData | undefined>(
@@ -21,44 +22,53 @@ export function ThebeSessionProvider({
   shutdownOnUnmount = false,
   children,
 }: React.PropsWithChildren<{
-  start: boolean;
+  start?: boolean;
   name?: string;
   shutdownOnUnmount?: boolean;
 }>) {
   const { config, server, ready: serverReady } = useThebeServer();
 
-  const [doStart, setDoStart] = useState(start);
   const [starting, setStarting] = useState(false);
-  const [sessionName, setSessionName] = useState(name);
   const [session, setSession] = useState<ThebeSession | undefined>();
+  const [ready, setReady] = useState(false);
   const [error, setError] = useState<string | undefined>();
 
-  /// Once server connection is open, start a session
-  useEffect(() => {
-    if (!doStart || !server || !serverReady) return;
+  const startSession = () => {
     setStarting(true);
     server
-      .startNewSession({ ...config?.kernels, name: sessionName, path: sessionName })
-      .then((sesh) => {
+      ?.startNewSession({ ...config?.kernels, name, path: name })
+      .then((sesh: ThebeSession | null) => {
         setStarting(false);
-        if (sesh != null) return setSession(sesh);
-        server.getKernelSpecs().then((specs) => {
-          setError(
-            `Could not start a session - available kernels: ${Object.keys(specs.kernelspecs)}`,
-          );
-        });
+        if (sesh == null) {
+          server?.getKernelSpecs().then((specs) => {
+            setError(
+              `Could not start a session - available kernels: ${Object.keys(specs.kernelspecs)}`,
+            );
+          });
+          return;
+        }
+        setSession(sesh);
+        setReady(true); // not this could use the thebe event mechanism
       });
-  }, [doStart, server, serverReady]);
+  };
+
+  /// Once server connection is open, auto start a session if start prop is true
+  useEffect(() => {
+    if (!server || !serverReady || !starting || !start) return;
+    startSession();
+  }, [start, starting, server, serverReady]);
 
   // shutdown session on navigate away
   useEffect(() => {
     return () => {
-      if (shutdownOnUnmount) session?.shutdown();
+      if (shutdownOnUnmount) {
+        session?.shutdown().then(() => {
+          setReady(false);
+        });
+      }
     };
   }, [session]);
 
-  const ready = !!session && session.kernel != null;
-
   return (
     <ThebeSessionContext.Provider
       value={{
@@ -67,9 +77,18 @@ export function ThebeSessionProvider({
         ready,
         session,
         // eslint-disable-next-line @typescript-eslint/no-shadow
-        start: (name?: string) => {
-          if (name) setSessionName(name);
-          setDoStart(true);
+        start: async () => {
+          if (!!session && ready) {
+            await session.restart();
+          } else {
+            startSession();
+          }
+        },
+        shutdown: async () => {
+          if (session) {
+            await session.shutdown();
+            setReady(false);
+          }
         },
         error,
       }}