Skip to content

Commit

Permalink
Refactor start/stop/restart logic to make it robust
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@andyleejordan
andyleejordan committed May 23, 2023
1 parent 3c7c103 commit c1a3621
Show file tree
Hide file tree
Showing 3 changed files with 336 additions and 298 deletions.
5 changes: 4 additions & 1 deletion src/features/DebugSession.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,10 @@ export class DebugSessionFeature extends LanguageClientConsumer
private async createTemporaryIntegratedConsole(session: DebugSession): Promise<IEditorServicesSessionDetails | undefined> {
const settings = getSettings();
this.tempDebugProcess = await this.sessionManager.createDebugSessionProcess(settings);
this.tempSessionDetails = await this.tempDebugProcess.start(`DebugSession-${this.sessionCount++}`);
// TODO: Maybe set a timeout on the cancellation token?
const cancellationTokenSource = new CancellationTokenSource();
this.tempSessionDetails = await this.tempDebugProcess.start(
`DebugSession-${this.sessionCount++}`, cancellationTokenSource.token);

// 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.
if (session.configuration.attachDotnetDebugger) {
Expand Down
80 changes: 34 additions & 46 deletions src/process.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@ import { promisify } from "util";

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

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

private consoleTerminal?: vscode.Terminal;
private consoleCloseSubscription?: vscode.Disposable;

private pid?: number;

constructor(
public exePath: string,
private bundledModulesPath: string,
Expand All @@ -33,7 +34,7 @@ export class PowerShellProcess {
this.onExited = this.onExitedEmitter.event;
}

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

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

// Make sure no old session file exists
await PowerShellProcess.deleteSessionFile(this.sessionFilePath);
await this.deleteSessionFile(this.sessionFilePath);

// Launch PowerShell in the integrated terminal
const terminalOptions: vscode.TerminalOptions = {
Expand All @@ -113,23 +114,17 @@ export class PowerShellProcess {
// subscription should happen before we create the terminal so if it
// fails immediately, the event fires.
this.consoleCloseSubscription = vscode.window.onDidCloseTerminal((terminal) => this.onTerminalClose(terminal));

this.consoleTerminal = vscode.window.createTerminal(terminalOptions);

const pwshName = path.basename(this.exePath);
this.logger.write(`${pwshName} started.`);

// Log that the PowerShell terminal process has been started
const pid = await this.getPid();
this.logTerminalPid(pid ?? 0, pwshName);
this.pid = await this.getPid();
this.logger.write(`PowerShell process started with PID: ${this.pid}`);

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

return await this.waitForSessionFile();
return await this.waitForSessionFile(cancellationToken);
}

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

// Returns the process Id of the consoleTerminal
public async getPid(): Promise<number | undefined> {
if (!this.consoleTerminal) { return undefined; }
return await this.consoleTerminal.processId;
return await this.consoleTerminal?.processId;
}

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

public async dispose(): Promise<void> {
// Clean up the session file
this.logger.write("Disposing PowerShell Extension Terminal...");
public dispose(): void {
this.logger.writeVerbose(`Disposing PowerShell process with PID: ${this.pid}`);

void this.deleteSessionFile(this.sessionFilePath);

this.consoleTerminal?.dispose();
this.consoleTerminal = undefined;

this.consoleCloseSubscription?.dispose();
this.consoleCloseSubscription = undefined;

await PowerShellProcess.deleteSessionFile(this.sessionFilePath);
}

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

private logTerminalPid(pid: number, exeName: string): void {
this.logger.write(`${exeName} PID: ${pid}`);
}

private isLoginShell(pwshPath: string): boolean {
try {
// We can't know what version of PowerShell we have without running it
Expand All @@ -184,64 +173,63 @@ export class PowerShellProcess {
} catch {
return false;
}

return true;
}

private static async readSessionFile(sessionFilePath: vscode.Uri): Promise<IEditorServicesSessionDetails> {
private async readSessionFile(sessionFilePath: vscode.Uri): Promise<IEditorServicesSessionDetails> {
const fileContents = await vscode.workspace.fs.readFile(sessionFilePath);
return JSON.parse(fileContents.toString());
}

private static async deleteSessionFile(sessionFilePath: vscode.Uri): Promise<void> {
private async deleteSessionFile(sessionFilePath: vscode.Uri): Promise<void> {
try {
await vscode.workspace.fs.delete(sessionFilePath);
} catch {
// We don't care about any reasons for it to fail.
}
}

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

// Check every 2 seconds
this.logger.write("Waiting for session file...");
// Check every second.
this.logger.writeVerbose(`Waiting for session file: ${this.sessionFilePath}`);
for (let i = numOfTries; i > 0; i--) {
if (cancellationToken.isCancellationRequested) {
this.logger.writeWarning("Canceled while waiting for session file.");
return undefined;
}

if (this.consoleTerminal === undefined) {
const err = "PowerShell Extension Terminal didn't start!";
this.logger.write(err);
throw new Error(err);
this.logger.writeError("Extension Terminal is undefined.");
return undefined;
}

if (await utils.checkIfFileExists(this.sessionFilePath)) {
this.logger.write("Session file found!");
const sessionDetails = await PowerShellProcess.readSessionFile(this.sessionFilePath);
await PowerShellProcess.deleteSessionFile(this.sessionFilePath);
return sessionDetails;
this.logger.writeVerbose("Session file found.");
return await this.readSessionFile(this.sessionFilePath);
}

if (warnAt === i) {
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.");
}

// Wait a bit and try again
await utils.sleep(2000);
// Wait a bit and try again.
await utils.sleep(1000);
}

const err = "Timed out waiting for session file to appear!";
this.logger.write(err);
throw new Error(err);
this.logger.writeError("Timed out waiting for session file!");
return undefined;
}

private onTerminalClose(terminal: vscode.Terminal): void {
if (terminal !== this.consoleTerminal) {
return;
}

this.logger.write("PowerShell process terminated or Extension Terminal was closed!");
this.logger.writeWarning(`PowerShell process terminated or Extension Terminal was closed, PID: ${this.pid}`);
this.onExitedEmitter.fire();
void this.dispose();
this.dispose();
}
}
Loading

0 comments on commit c1a3621

Please sign in to comment.