Make sessionManager.start() idempotent (#4599)

So the debugger can just call `start()` to ensure the session has
started. Also make a bunch of public methods private.

Author: andyleejordan

src/features/DebugSession.ts

@@ -30,7 +30,7 @@ import { LanguageClientConsumer } from "../languageClientConsumer";
 import { ILogger } from "../logging";
 import { OperatingSystem, getPlatformDetails } from "../platform";
 import { PowerShellProcess } from "../process";
-import { IEditorServicesSessionDetails, SessionManager, SessionStatus } from "../session";
+import { IEditorServicesSessionDetails, SessionManager } from "../session";
 import { getSettings } from "../settings";
 import path from "path";
 import { checkIfFileExists } from "../utils";
@@ -281,9 +281,7 @@ export class DebugSessionFeature extends LanguageClientConsumer
         _executable: DebugAdapterExecutable | undefined): Promise<DebugAdapterDescriptor | undefined> {
         // NOTE: A Promise meets the shape of a ProviderResult, which allows us to make this method async.
 
-        if (this.sessionManager.getSessionStatus() !== SessionStatus.Running) {
-            await this.sessionManager.start();
-        }
+        await this.sessionManager.start();
 
         const sessionDetails = session.configuration.createTemporaryIntegratedConsole
             ? await this.createTemporaryIntegratedConsole(session)

src/session.ts

@@ -26,7 +26,7 @@ import {
 import { LanguageClientConsumer } from "./languageClientConsumer";
 import { SemVer, satisfies } from "semver";
 
-export enum SessionStatus {
+enum SessionStatus {
     NotStarted = "Not Started",
     Starting = "Starting",
     Running = "Running",
@@ -146,11 +146,34 @@ export class SessionManager implements Middleware {
     }
 
     // The `exeNameOverride` is used by `restartSession` to override ANY other setting.
+    // We've made this function idempotent, so it can used to ensure the session has started.
     public async start(exeNameOverride?: string): Promise<void> {
-        // A simple lock because this function isn't re-entrant.
-        if (this.sessionStatus === SessionStatus.Starting) {
+        switch (this.sessionStatus) {
+        case SessionStatus.NotStarted:
+            // Go ahead and start.
+            break;
+        case SessionStatus.Starting:
+            // A simple lock because this function isn't re-entrant.
             this.logger.writeWarning("Re-entered 'start' so waiting...");
-            return await this.waitUntilStarted();
+            return await this.waitWhileStarting();
+        case SessionStatus.Running:
+            // We're started, just return.
+            this.logger.writeVerbose("Already started.");
+            return;
+        case SessionStatus.Busy:
+            // We're started but busy so notify and return.
+            // TODO: Make a proper notification for this and when IntelliSense is blocked.
+            this.logger.write("The Extension Terminal is currently busy, please wait for your task to finish!");
+            return;
+        case SessionStatus.Stopping:
+            // Wait until done stopping, then start.
+            this.logger.writeVerbose("Still stopping.");
+            await this.waitWhileStopping();
+            break;
+        case SessionStatus.Failed:
+            // Try to start again.
+            this.logger.writeVerbose("Previously failed, starting again.");
+            break;
         }
 
         this.setSessionStatus("Starting...", SessionStatus.Starting);
@@ -220,7 +243,7 @@ export class SessionManager implements Middleware {
         }
     }
 
-    public async stop(): Promise<void> {
+    private async stop(): Promise<void> {
         this.setSessionStatus("Stopping...", SessionStatus.Stopping);
         // Cancel start-up if we're still waiting.
         this.startCancellationTokenSource?.cancel();
@@ -255,7 +278,7 @@ export class SessionManager implements Middleware {
         this.setSessionStatus("Not Started", SessionStatus.NotStarted);
     }
 
-    public async restartSession(exeNameOverride?: string): Promise<void> {
+    private async restartSession(exeNameOverride?: string): Promise<void> {
         this.logger.write("Restarting session...");
         await this.stop();
 
@@ -267,22 +290,18 @@ export class SessionManager implements Middleware {
     }
 
     public getSessionDetails(): IEditorServicesSessionDetails | undefined {
-        // TODO: This is used solely by the debugger and should actually just wait (with a timeout).
+        // This is used by the debugger which should have already called `start`.
         if (this.sessionDetails === undefined) {
             void this.logger.writeAndShowError("PowerShell session unavailable for debugging!");
         }
         return this.sessionDetails;
     }
 
-    public getSessionStatus(): SessionStatus {
-        return this.sessionStatus;
-    }
-
     public getPowerShellVersionDetails(): IPowerShellVersionDetails | undefined {
         return this.versionDetails;
     }
 
-    public getNewSessionFilePath(): vscode.Uri {
+    private getNewSessionFilePath(): vscode.Uri {
         const uniqueId: number = Math.floor(100000 + Math.random() * 900000);
         return vscode.Uri.joinPath(this.sessionsFolder, `PSES-VSCode-${process.env.VSCODE_PID}-${uniqueId}.json`);
     }
@@ -334,14 +353,12 @@ export class SessionManager implements Middleware {
     }
 
     public async waitUntilStarted(): Promise<void> {
-        while (this.sessionStatus === SessionStatus.Starting) {
-            if (this.startCancellationTokenSource?.token.isCancellationRequested) {
-                return;
-            }
+        while (this.sessionStatus !== SessionStatus.Running) {
             await utils.sleep(300);
         }
     }
 
+    // TODO: Is this used by the magic of "Middleware" in the client library?
     public resolveCodeLens(
         codeLens: vscode.CodeLens,
         token: vscode.CancellationToken,
@@ -803,6 +820,21 @@ Type 'help' to get help.
         return languageStatusItem;
     }
 
+    private async waitWhileStarting(): Promise<void> {
+        while (this.sessionStatus === SessionStatus.Starting) {
+            if (this.startCancellationTokenSource?.token.isCancellationRequested) {
+                return;
+            }
+            await utils.sleep(300);
+        }
+    }
+
+    private async waitWhileStopping(): Promise<void> {
+        while (this.sessionStatus === SessionStatus.Stopping) {
+            await utils.sleep(300);
+        }
+    }
+
     private setSessionStatus(detail: string, status: SessionStatus): void {
         this.logger.writeVerbose(`Session status changing from '${this.sessionStatus}' to '${status}'.`);
         this.sessionStatus = status;
@@ -842,7 +874,6 @@ Type 'help' to get help.
             this.languageStatusItem.severity = vscode.LanguageStatusSeverity.Error;
             break;
         }
-
     }
 
     private setSessionRunningStatus(): void {
@@ -910,7 +941,7 @@ Type 'help' to get help.
     }
 
     // Always shows the session terminal.
-    public showSessionTerminal(isExecute?: boolean): void {
+    private showSessionTerminal(isExecute?: boolean): void {
         this.languageServerProcess?.showTerminal(isExecute && !this.sessionSettings.integratedConsole.focusConsoleOnExecute);
     }
 

test/features/DebugSession.test.ts

@@ -10,7 +10,7 @@ import { DebugConfig, DebugSessionFeature, DebugConfigurations } from "../../src
 import { IPowerShellExtensionClient } from "../../src/features/ExternalApi";
 import * as platform from "../../src/platform";
 import { IPlatformDetails } from "../../src/platform";
-import { IEditorServicesSessionDetails, IPowerShellVersionDetails, SessionManager, SessionStatus } from "../../src/session";
+import { IEditorServicesSessionDetails, IPowerShellVersionDetails, SessionManager } from "../../src/session";
 import * as utils from "../../src/utils";
 import { BuildBinaryModuleMock, WaitEvent, ensureEditorServicesIsConnected, stubInterface, testLogger } from "../utils";
 
@@ -408,7 +408,6 @@ describe("DebugSessionFeature", () => {
         it("Creates a named pipe server for the debug adapter", async () => {
             const debugSessionFeature = createDebugSessionFeatureStub({
                 sessionManager: Sinon.createStubInstance(SessionManager, {
-                    getSessionStatus: SessionStatus.Running,
                     getSessionDetails: stubInterface<IEditorServicesSessionDetails>({
                         debugServicePipeName: "testPipeName"
                     })