Skip to content

Fix disposal of temporary console and event handler #3953

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 5, 2022
Merged

Fix disposal of temporary console and event handler #3953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions src/features/DebugSession.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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());
}
Expand Down
18 changes: 13 additions & 5 deletions src/session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand All @@ -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();
Expand Down