Skip to content

Commit 5304fa3

Browse files
committed
Refactor start/stop/restart logic to make it robust
Includes some reorganization, a new cancellation token, and a bunch of bug fixes. We can now consistently restart the language server without losing track of the Extension Terminal, even if it's still starting up. Our `SessionStatus` is now actually useful, and the logical flow actually makes sense. Plus the giant methods have been broken up and error handling is consistent.
1 parent afdc407 commit 5304fa3

File tree

3 files changed

+336
-298
lines changed

3 files changed

+336
-298
lines changed

src/features/DebugSession.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,10 @@ export class DebugSessionFeature extends LanguageClientConsumer
359359
private async createTemporaryIntegratedConsole(session: DebugSession): Promise<IEditorServicesSessionDetails | undefined> {
360360
const settings = getSettings();
361361
this.tempDebugProcess = await this.sessionManager.createDebugSessionProcess(settings);
362-
this.tempSessionDetails = await this.tempDebugProcess.start(`DebugSession-${this.sessionCount++}`);
362+
// TODO: Maybe set a timeout on the cancellation token?
363+
const cancellationTokenSource = new CancellationTokenSource();
364+
this.tempSessionDetails = await this.tempDebugProcess.start(
365+
`DebugSession-${this.sessionCount++}`, cancellationTokenSource.token);
363366

364367
// NOTE: Dotnet attach debugging is only currently supported if a temporary debug terminal is used, otherwise we get lots of lock conflicts from loading the assemblies.
365368
if (session.configuration.attachDotnetDebugger) {

src/process.ts

+34-46
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,16 @@ import { promisify } from "util";
1212

1313
export class PowerShellProcess {
1414
// This is used to warn the user that the extension is taking longer than expected to startup.
15-
// After the 15th try we've hit 30 seconds and should warn.
16-
private static warnUserThreshold = 15;
15+
private static warnUserThreshold = 30;
1716

1817
public onExited: vscode.Event<void>;
1918
private onExitedEmitter = new vscode.EventEmitter<void>();
2019

2120
private consoleTerminal?: vscode.Terminal;
2221
private consoleCloseSubscription?: vscode.Disposable;
2322

23+
private pid?: number;
24+
2425
constructor(
2526
public exePath: string,
2627
private bundledModulesPath: string,
@@ -33,7 +34,7 @@ export class PowerShellProcess {
3334
this.onExited = this.onExitedEmitter.event;
3435
}
3536

36-
public async start(logFileName: string): Promise<IEditorServicesSessionDetails> {
37+
public async start(logFileName: string, cancellationToken: vscode.CancellationToken): Promise<IEditorServicesSessionDetails | undefined> {
3738
const editorServicesLogPath = this.logger.getLogFilePath(logFileName);
3839

3940
const psesModulePath =
@@ -95,7 +96,7 @@ export class PowerShellProcess {
9596
this.logger.writeVerbose(`Starting process: ${this.exePath} ${powerShellArgs.slice(0, -2).join(" ")} -Command ${startEditorServices}`);
9697

9798
// Make sure no old session file exists
98-
await PowerShellProcess.deleteSessionFile(this.sessionFilePath);
99+
await this.deleteSessionFile(this.sessionFilePath);
99100

100101
// Launch PowerShell in the integrated terminal
101102
const terminalOptions: vscode.TerminalOptions = {
@@ -113,23 +114,17 @@ export class PowerShellProcess {
113114
// subscription should happen before we create the terminal so if it
114115
// fails immediately, the event fires.
115116
this.consoleCloseSubscription = vscode.window.onDidCloseTerminal((terminal) => this.onTerminalClose(terminal));
116-
117117
this.consoleTerminal = vscode.window.createTerminal(terminalOptions);
118-
119-
const pwshName = path.basename(this.exePath);
120-
this.logger.write(`${pwshName} started.`);
121-
122-
// Log that the PowerShell terminal process has been started
123-
const pid = await this.getPid();
124-
this.logTerminalPid(pid ?? 0, pwshName);
118+
this.pid = await this.getPid();
119+
this.logger.write(`PowerShell process started with PID: ${this.pid}`);
125120

126121
if (this.sessionSettings.integratedConsole.showOnStartup
127122
&& !this.sessionSettings.integratedConsole.startInBackground) {
128123
// We still need to run this to set the active terminal to the extension terminal.
129124
this.consoleTerminal.show(true);
130125
}
131126

132-
return await this.waitForSessionFile();
127+
return await this.waitForSessionFile(cancellationToken);
133128
}
134129

135130
// This function should only be used after a failure has occurred because it is slow!
@@ -141,25 +136,23 @@ export class PowerShellProcess {
141136

142137
// Returns the process Id of the consoleTerminal
143138
public async getPid(): Promise<number | undefined> {
144-
if (!this.consoleTerminal) { return undefined; }
145-
return await this.consoleTerminal.processId;
139+
return await this.consoleTerminal?.processId;
146140
}
147141

148142
public showTerminal(preserveFocus?: boolean): void {
149143
this.consoleTerminal?.show(preserveFocus);
150144
}
151145

152-
public async dispose(): Promise<void> {
153-
// Clean up the session file
154-
this.logger.write("Disposing PowerShell Extension Terminal...");
146+
public dispose(): void {
147+
this.logger.writeVerbose(`Disposing PowerShell process with PID: ${this.pid}`);
148+
149+
void this.deleteSessionFile(this.sessionFilePath);
155150

156151
this.consoleTerminal?.dispose();
157152
this.consoleTerminal = undefined;
158153

159154
this.consoleCloseSubscription?.dispose();
160155
this.consoleCloseSubscription = undefined;
161-
162-
await PowerShellProcess.deleteSessionFile(this.sessionFilePath);
163156
}
164157

165158
public sendKeyPress(): void {
@@ -170,10 +163,6 @@ export class PowerShellProcess {
170163
this.consoleTerminal?.sendText("p", false);
171164
}
172165

173-
private logTerminalPid(pid: number, exeName: string): void {
174-
this.logger.write(`${exeName} PID: ${pid}`);
175-
}
176-
177166
private isLoginShell(pwshPath: string): boolean {
178167
try {
179168
// We can't know what version of PowerShell we have without running it
@@ -184,64 +173,63 @@ export class PowerShellProcess {
184173
} catch {
185174
return false;
186175
}
187-
188176
return true;
189177
}
190178

191-
private static async readSessionFile(sessionFilePath: vscode.Uri): Promise<IEditorServicesSessionDetails> {
179+
private async readSessionFile(sessionFilePath: vscode.Uri): Promise<IEditorServicesSessionDetails> {
192180
const fileContents = await vscode.workspace.fs.readFile(sessionFilePath);
193181
return JSON.parse(fileContents.toString());
194182
}
195183

196-
private static async deleteSessionFile(sessionFilePath: vscode.Uri): Promise<void> {
184+
private async deleteSessionFile(sessionFilePath: vscode.Uri): Promise<void> {
197185
try {
198186
await vscode.workspace.fs.delete(sessionFilePath);
199187
} catch {
200188
// We don't care about any reasons for it to fail.
201189
}
202190
}
203191

204-
private async waitForSessionFile(): Promise<IEditorServicesSessionDetails> {
205-
// Determine how many tries by dividing by 2000 thus checking every 2 seconds.
206-
const numOfTries = this.sessionSettings.developer.waitForSessionFileTimeoutSeconds / 2;
192+
private async waitForSessionFile(cancellationToken: vscode.CancellationToken): Promise<IEditorServicesSessionDetails | undefined> {
193+
const numOfTries = this.sessionSettings.developer.waitForSessionFileTimeoutSeconds;
207194
const warnAt = numOfTries - PowerShellProcess.warnUserThreshold;
208195

209-
// Check every 2 seconds
210-
this.logger.write("Waiting for session file...");
196+
// Check every second.
197+
this.logger.writeVerbose(`Waiting for session file: ${this.sessionFilePath}`);
211198
for (let i = numOfTries; i > 0; i--) {
199+
if (cancellationToken.isCancellationRequested) {
200+
this.logger.writeWarning("Canceled while waiting for session file.");
201+
return undefined;
202+
}
203+
212204
if (this.consoleTerminal === undefined) {
213-
const err = "PowerShell Extension Terminal didn't start!";
214-
this.logger.write(err);
215-
throw new Error(err);
205+
this.logger.writeError("Extension Terminal is undefined.");
206+
return undefined;
216207
}
217208

218209
if (await utils.checkIfFileExists(this.sessionFilePath)) {
219-
this.logger.write("Session file found!");
220-
const sessionDetails = await PowerShellProcess.readSessionFile(this.sessionFilePath);
221-
await PowerShellProcess.deleteSessionFile(this.sessionFilePath);
222-
return sessionDetails;
210+
this.logger.writeVerbose("Session file found.");
211+
return await this.readSessionFile(this.sessionFilePath);
223212
}
224213

225214
if (warnAt === i) {
226215
void this.logger.writeAndShowWarning("Loading the PowerShell extension is taking longer than expected. If you're using privilege enforcement software, this can affect start up performance.");
227216
}
228217

229-
// Wait a bit and try again
230-
await utils.sleep(2000);
218+
// Wait a bit and try again.
219+
await utils.sleep(1000);
231220
}
232221

233-
const err = "Timed out waiting for session file to appear!";
234-
this.logger.write(err);
235-
throw new Error(err);
222+
this.logger.writeError("Timed out waiting for session file!");
223+
return undefined;
236224
}
237225

238226
private onTerminalClose(terminal: vscode.Terminal): void {
239227
if (terminal !== this.consoleTerminal) {
240228
return;
241229
}
242230

243-
this.logger.write("PowerShell process terminated or Extension Terminal was closed!");
231+
this.logger.writeWarning(`PowerShell process terminated or Extension Terminal was closed, PID: ${this.pid}`);
244232
this.onExitedEmitter.fire();
245-
void this.dispose();
233+
this.dispose();
246234
}
247235
}

0 commit comments

Comments
 (0)