Skip to content

Commit

Permalink
Kill processes immediately on shutdown, use SIGTERM
Browse files Browse the repository at this point in the history
Fixes #56217
  • Loading branch information
Tyriar committed Aug 27, 2018
1 parent 112f31a commit 95c0e07
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape
this._proxy.$createProcess(request.proxy.terminalId, shellLaunchConfigDto, request.cols, request.rows);
request.proxy.onInput(data => this._proxy.$acceptProcessInput(request.proxy.terminalId, data));
request.proxy.onResize(dimensions => this._proxy.$acceptProcessResize(request.proxy.terminalId, dimensions.cols, dimensions.rows));
request.proxy.onShutdown(() => this._proxy.$acceptProcessShutdown(request.proxy.terminalId));
request.proxy.onShutdown(immediate => this._proxy.$acceptProcessShutdown(request.proxy.terminalId, immediate));
}

public $sendProcessTitle(terminalId: number, title: string): void {
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/api/node/extHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -878,7 +878,7 @@ export interface ExtHostTerminalServiceShape {
$createProcess(id: number, shellLaunchConfig: ShellLaunchConfigDto, cols: number, rows: number): void;
$acceptProcessInput(id: number, data: string): void;
$acceptProcessResize(id: number, cols: number, rows: number): void;
$acceptProcessShutdown(id: number): void;
$acceptProcessShutdown(id: number, immediate: boolean): void;
}

export interface ExtHostSCMShape {
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/api/node/extHostTerminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,8 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
}
}

public $acceptProcessShutdown(id: number): void {
this._terminalProcesses[id].shutdown();
public $acceptProcessShutdown(id: number, immediate: boolean): void {
this._terminalProcesses[id].shutdown(immediate);
}

private _onProcessExit(id: number, exitCode: number): void {
Expand Down
8 changes: 6 additions & 2 deletions src/vs/workbench/parts/terminal/common/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,11 @@ export interface ITerminalInstance {

/**
* Dispose the terminal instance, removing it from the panel/service and freeing up resources.
*
* @param isShuttingDown Whether VS Code is shutting down, if so kill any terminal processes
* immediately.
*/
dispose(): void;
dispose(isShuttingDown?: boolean): void;

/**
* Registers a link matcher, allowing custom link patterns to be matched and handled.
Expand Down Expand Up @@ -573,6 +576,7 @@ export interface ITerminalProcessManager extends IDisposable {
readonly onProcessExit: Event<number>;

addDisposable(disposable: IDisposable);
dispose(immediate?: boolean);
createProcess(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number);
write(data: string): void;
setDimensions(cols: number, rows: number): void;
Expand Down Expand Up @@ -608,7 +612,7 @@ export interface ITerminalProcessExtHostProxy extends IDisposable {

onInput: Event<string>;
onResize: Event<{ cols: number, rows: number }>;
onShutdown: Event<void>;
onShutdown: Event<boolean>;
}

export interface ITerminalProcessExtHostRequest {
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/parts/terminal/common/terminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export abstract class TerminalService implements ITerminalService {

private _onShutdown(): void {
// Dispose of all instances
this.terminalInstances.forEach(instance => instance.dispose());
this.terminalInstances.forEach(instance => instance.dispose(true));
}

public getTabLabels(): string[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ export class TerminalInstance implements ITerminalInstance {
this._terminalFocusContextKey.set(terminalFocused);
}

public dispose(): void {
public dispose(isShuttingDown?: boolean): void {
this._logService.trace(`terminalInstance#dispose (id: ${this.id})`);

this._windowsShellHelper = lifecycle.dispose(this._windowsShellHelper);
Expand All @@ -588,7 +588,7 @@ export class TerminalInstance implements ITerminalInstance {
this._xterm.dispose();
this._xterm = null;
}
this._processManager = lifecycle.dispose(this._processManager);
this._processManager.dispose(isShuttingDown);
if (!this._isDisposed) {
this._isDisposed = true;
this._onDisposed.fire(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,13 @@ export class TerminalProcessManager implements ITerminalProcessManager {
});
}

public dispose(): void {
public dispose(immediate?: boolean): void {
if (this._process) {
// If the process was still connected this dispose came from
// within VS Code, not the process, so mark the process as
// killed by the user.
this.processState = ProcessState.KILLED_BY_USER;
this._process.shutdown();
this._process.shutdown(immediate);
this._process = null;
}
this._disposables.forEach(d => d.dispose());
Expand Down
8 changes: 7 additions & 1 deletion src/vs/workbench/parts/terminal/node/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,13 @@ export interface ITerminalChildProcess {
onProcessIdReady: Event<number>;
onProcessTitleChanged: Event<string>;

shutdown(): void;
/**
* Shutdown the terminal process.
*
* @param immediate When true the process will be killed immediately, otherwise the process will
* be given some time to make sure no additional data comes through.
*/
shutdown(immediate: boolean): void;
input(data: string): void;
resize(cols: number, rows: number): void;
}
Expand Down
34 changes: 23 additions & 11 deletions src/vs/workbench/parts/terminal/node/terminalProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,17 +97,25 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable {
if (this._closeTimeout) {
clearTimeout(this._closeTimeout);
}
this._closeTimeout = setTimeout(() => {
// Attempt to kill the pty, it may have already been killed at this
// point but we want to make sure
try {
this._closeTimeout = setTimeout(() => this._kill(), 250);
}

private _kill(): void {
// Attempt to kill the pty, it may have already been killed at this
// point but we want to make sure
try {
if (!platform.isWindows) {
// Send SIGTERM, SIGHUP does not seem to work when the parent process dies
// immediately after.
process.kill(this._ptyProcess.pid, 'SIGTERM');
} else {
this._ptyProcess.kill();
} catch (ex) {
// Swallow, the pty has already been killed
}
this._onProcessExit.fire(this._exitCode);
this.dispose();
}, 250);
} catch (ex) {
// Swallow, the pty has already been killed
}
this._onProcessExit.fire(this._exitCode);
this.dispose();
}

private _sendProcessId() {
Expand All @@ -119,8 +127,12 @@ export class TerminalProcess implements ITerminalChildProcess, IDisposable {
this._onProcessTitleChanged.fire(this._currentTitle);
}

public shutdown(): void {
this._queueProcessExit();
public shutdown(immediate: boolean): void {
if (immediate) {
this._kill();
} else {
this._queueProcessExit();
}
}

public input(data: string): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ export class TerminalProcessExtHostProxy implements ITerminalChildProcess, ITerm
public get onInput(): Event<string> { return this._onInput.event; }
private readonly _onResize: Emitter<{ cols: number, rows: number }> = new Emitter<{ cols: number, rows: number }>();
public get onResize(): Event<{ cols: number, rows: number }> { return this._onResize.event; }
private readonly _onShutdown: Emitter<void> = new Emitter<void>();
public get onShutdown(): Event<void> { return this._onShutdown.event; }
private readonly _onShutdown: Emitter<boolean> = new Emitter<boolean>();
public get onShutdown(): Event<boolean> { return this._onShutdown.event; }

constructor(
public terminalId: number,
Expand Down Expand Up @@ -65,8 +65,9 @@ export class TerminalProcessExtHostProxy implements ITerminalChildProcess, ITerm
this._onProcessExit.fire(exitCode);
this.dispose();
}
public shutdown(): void {
this._onShutdown.fire();

public shutdown(immediate: boolean): void {
this._onShutdown.fire(immediate);
}

public input(data: string): void {
Expand Down

0 comments on commit 95c0e07

Please sign in to comment.