clean up interactive window creation

microsoft · Oct 6, 2022 · 77afeda · 77afeda
1 parent 48fa7d5
commit 77afeda
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 17 deletions.
diff --git a/src/interactive-window/editor-integration/codewatcher.ts b/src/interactive-window/editor-integration/codewatcher.ts
@@ -20,7 +20,6 @@ import {
 
 import { IDocumentManager } from '../../platform/common/application/types';
 import { ICellRange, IConfigurationService, IDisposable, Resource } from '../../platform/common/types';
-import { chainable } from '../../platform/common/utils/decorators';
 import { isUri, noop } from '../../platform/common/utils/misc';
 import { capturePerfTelemetry, captureUsageTelemetry } from '../../telemetry';
 import { ICodeExecutionHelper } from '../../platform/terminals/types';
@@ -984,9 +983,7 @@ export class CodeWatcher implements ICodeWatcher {
         }
     }
 
-    @chainable()
     private getActiveInteractiveWindow() {
-        // This should be chained so that a queue forms when getting the interactive window
         return this.interactiveWindowProvider.getOrCreate(this.document?.uri);
     }
     private async addCode(

diff --git a/src/interactive-window/interactiveWindow.ts b/src/interactive-window/interactiveWindow.ts
@@ -100,6 +100,7 @@ export class InteractiveWindow implements IInteractiveWindowLoadable {
     public get kernelConnectionMetadata(): KernelConnectionMetadata | undefined {
         return this.currentKernelInfo.metadata;
     }
+    private initialized = false;
     private _onDidChangeViewState = new EventEmitter<void>();
     private closedEvent = new EventEmitter<void>();
     private _submitters: Uri[] = [];
@@ -180,7 +181,11 @@ export class InteractiveWindow implements IInteractiveWindowLoadable {
         }
     }
 
-    public start() {
+    public initialize() {
+        if (this.initialized) {
+            return;
+        }
+        this.initialized = true;
         window.onDidChangeActiveNotebookEditor((e) => {
             if (e === this.notebookEditor) {
                 this._onDidChangeViewState.fire();
@@ -228,7 +233,7 @@ export class InteractiveWindow implements IInteractiveWindowLoadable {
             }
         }
 
-        this.start();
+        this.initialize();
     }
 
     /**

diff --git a/src/interactive-window/interactiveWindowProvider.ts b/src/interactive-window/interactiveWindowProvider.ts
@@ -30,7 +30,6 @@ import {
     Resource,
     WORKSPACE_MEMENTO
 } from '../platform/common/types';
-import { chainable } from '../platform/common/utils/decorators';
 import * as localize from '../platform/common/utils/localize';
 import { noop } from '../platform/common/utils/misc';
 import { IServiceContainer } from '../platform/ioc/types';
@@ -139,7 +138,6 @@ export class InteractiveWindowProvider
         this._updateWindowCache();
     }
 
-    @chainable()
     public async getOrCreate(resource: Resource, connection?: KernelConnectionMetadata): Promise<IInteractiveWindow> {
         if (!this.workspaceService.isTrusted) {
             // This should not happen, but if it does, then just throw an error.
@@ -160,8 +158,8 @@ export class InteractiveWindowProvider
         if (!result) {
             // No match. Create a new item.
             result = await this.create(resource, mode, connection);
-            // start the kernel
-            result.start();
+            // ensure events are wired up and kernel is started.
+            result.initialize();
         } else {
             const preferredController = connection
                 ? this.controllerRegistration.get(connection, InteractiveWindowView)
@@ -186,17 +184,11 @@ export class InteractiveWindowProvider
         return noop();
     }
 
-    // Note to future devs: this function must be synchronous. Do not await on anything before calling
-    // the interactive window ctor and adding the interactive window to the provider's list of known windows.
-    // Otherwise we risk a race condition where e.g. multiple run cell requests come in quick and we end up
-    // instantiating multiples.
     private async create(resource: Resource, mode: InteractiveWindowMode, connection?: KernelConnectionMetadata) {
+        // track when a creation is pending, so consumers can wait before checking for an existing one.
         const creationInProgress = createDeferred<void>();
-        // Ensure we don't end up calling this method multiple times when creating an IW for the same resource.
         this.pendingCreations.push(creationInProgress.promise);
         try {
-            // Set it as soon as we create it. The .ctor for the interactive window
-            // may cause a subclass to talk to the IInteractiveWindowProvider to get the active interactive window.
             // Find our preferred controller
             const preferredController = connection
                 ? this.controllerRegistration.get(connection, InteractiveWindowView)

diff --git a/src/interactive-window/types.ts b/src/interactive-window/types.ts
@@ -67,7 +67,7 @@ export interface IInteractiveWindow extends IInteractiveBase {
     readonly inputUri?: Uri;
     readonly notebookDocument?: NotebookDocument;
     closed: Event<void>;
-    start(): void;
+    initialize(): void;
     restore(preferredController: IVSCodeNotebookController | undefined): Promise<void>;
     addCode(code: string, file: Uri, line: number): Promise<boolean>;
     addErrorMessage(message: string, cell: NotebookCell): Promise<void>;