From b7cefbd9b33433181476c89ac41226236b572163 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 5 May 2022 15:03:15 -0700 Subject: [PATCH] Fix disposal of temporary console and event handler Fixes a bug where the first patch would register a new event handler every time, leading to multiple 'p's being sent. We currently only support one process at a time, and so this patch consolidates the process's and the event handler's disposal. --- src/features/DebugSession.ts | 13 +++---------- src/session.ts | 18 +++++++++++++----- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/features/DebugSession.ts b/src/features/DebugSession.ts index 9ebfbc243d..af23e26f9e 100644 --- a/src/features/DebugSession.ts +++ b/src/features/DebugSession.ts @@ -25,6 +25,7 @@ export class DebugSessionFeature extends LanguageClientConsumer private sessionCount: number = 1; private tempDebugProcess: PowerShellProcess; + private tempDebugEventHandler: vscode.Disposable; private tempSessionDetails: utils.IEditorServicesSessionDetails; constructor(context: ExtensionContext, private sessionManager: SessionManager, private logger: Logger) { @@ -313,18 +314,10 @@ export class DebugSessionFeature extends LanguageClientConsumer const sessionFilePath = utils.getDebugSessionFilePath(); if (config.createTemporaryIntegratedConsole) { - if (this.tempDebugProcess) { - this.tempDebugProcess.dispose(); - } - - this.tempDebugProcess = - this.sessionManager.createDebugSessionProcess( - sessionFilePath, - settings); - + // TODO: This should be cleaned up to support multiple temporary consoles. + this.tempDebugProcess = this.sessionManager.createDebugSessionProcess(sessionFilePath, settings); this.tempSessionDetails = await this.tempDebugProcess.start(`DebugSession-${this.sessionCount++}`); utils.writeSessionFile(sessionFilePath, this.tempSessionDetails); - } else { utils.writeSessionFile(sessionFilePath, this.sessionManager.getSessionDetails()); } diff --git a/src/session.ts b/src/session.ts index 93dc28c173..57bb385715 100644 --- a/src/session.ts +++ b/src/session.ts @@ -50,6 +50,7 @@ export class SessionManager implements Middleware { private statusBarItem: vscode.StatusBarItem; private languageServerProcess: PowerShellProcess; private debugSessionProcess: PowerShellProcess; + private debugEventHandler: vscode.Disposable; private versionDetails: IPowerShellVersionDetails; private registeredCommands: vscode.Disposable[] = []; private languageServerClient: LanguageClient = undefined; @@ -228,9 +229,10 @@ export class SessionManager implements Middleware { this.languageServerClient = undefined; } - // Kill the PowerShell proceses we spawned + // Kill the PowerShell process we spawned if (this.debugSessionProcess) { this.debugSessionProcess.dispose(); + this.debugEventHandler.dispose(); } if (this.languageServerProcess) { this.languageServerProcess.dispose(); @@ -260,6 +262,15 @@ export class SessionManager implements Middleware { sessionPath: string, sessionSettings: Settings.ISettings): PowerShellProcess { + // NOTE: We only support one temporary integrated console at a time. To + // support more, we need to track each separately, and tie the session + // for the event handler to the right process (and dispose of the event + // handler when the process is disposed). + if (this.debugSessionProcess) { + this.debugSessionProcess.dispose() + this.debugEventHandler.dispose(); + } + this.debugSessionProcess = new PowerShellProcess( this.PowerShellExeDetails.exePath, @@ -273,10 +284,7 @@ export class SessionManager implements Middleware { // Similar to the regular integrated console, we need to send a key // press to the process spawned for temporary integrated consoles when // the server requests a cancellation os Console.ReadKey. - // - // TODO: There may be multiple sessions running in parallel, so we need - // to track a process per session, but that already isn't being done. - vscode.debug.onDidReceiveDebugSessionCustomEvent( + this.debugEventHandler = vscode.debug.onDidReceiveDebugSessionCustomEvent( e => { if (e.event === "powerShell/sendKeyPress") { this.debugSessionProcess.sendKeyPress();