Skip to content

Commit

Permalink
fix #6767: don't reuse integrated terminal that has child processes
Browse files Browse the repository at this point in the history
Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
  • Loading branch information
a1994846931931 committed Jan 21, 2020
1 parent fd2d384 commit 6976745
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 1 deletion.
9 changes: 8 additions & 1 deletion packages/debug/src/browser/debug-session.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,14 @@ export class DebugSession implements CompositeTreeElement {
}

protected async doCreateTerminal(options: TerminalWidgetOptions): Promise<TerminalWidget> {
let terminal = this.terminalServer.all.find(t => t.title.label === options.title || t.title.caption === options.title);
let terminal = undefined;
for (const t of this.terminalServer.all) {
if ((t.title.label === options.title || t.title.caption === options.title) && (await t.hasChildProcesses()) === false) {
terminal = t;
break;
}
}

if (!terminal) {
terminal = await this.terminalServer.newTerminal(options);
await terminal.start();
Expand Down
5 changes: 5 additions & 0 deletions packages/terminal/src/browser/base/terminal-widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ export abstract class TerminalWidget extends BaseWidget {
* Cleat terminal output.
*/
abstract clearOutput(): void;

/**
* Whether the terminal process has child processes.
*/
abstract hasChildProcesses(): Promise<boolean>;
}

/**
Expand Down
4 changes: 4 additions & 0 deletions packages/terminal/src/browser/terminal-widget-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,10 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget
this.term.clear();
}

async hasChildProcesses(): Promise<boolean> {
return this.shellTerminalServer.hasChildProcesses(await this.processId);
}

storeState(): object {
this.closeOnDispose = false;
return { terminalId: this.terminalId, titleLabel: this.title.label };
Expand Down
1 change: 1 addition & 0 deletions packages/terminal/src/common/shell-terminal-protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { IBaseTerminalServer, IBaseTerminalServerOptions } from './base-terminal
export const IShellTerminalServer = Symbol('IShellTerminalServer');

export interface IShellTerminalServer extends IBaseTerminalServer {
hasChildProcesses(processId: number | undefined): Promise<boolean>;
}

export const shellTerminalPath = '/services/shell-terminal';
Expand Down
44 changes: 44 additions & 0 deletions packages/terminal/src/node/shell-terminal-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { IShellTerminalServerOptions } from '../common/shell-terminal-protocol';
import { BaseTerminalServer } from '../node/base-terminal-server';
import { ShellProcessFactory } from '../node/shell-process';
import { ProcessManager } from '@theia/process/lib/node';
import { isWindows } from '@theia/core/lib/common/os';
import * as cp from 'child_process';

@injectable()
export class ShellTerminalServer extends BaseTerminalServer {
Expand All @@ -41,4 +43,46 @@ export class ShellTerminalServer extends BaseTerminalServer {
return Promise.resolve(-1);
}
}

// copied and modified from https://github.com/microsoft/vscode/blob/4636be2b71c87bfb0bfe3c94278b447a5efcc1f1/src/vs/workbench/contrib/debug/node/terminals.ts#L32-L75
private spawnAsPromised(command: string, args: string[]): Promise<string> {
return new Promise((resolve, reject) => {
let stdout = '';
const child = cp.spawn(command, args);
if (child.pid) {
child.stdout.on('data', (data: Buffer) => {
stdout += data.toString();
});
}
child.on('error', err => {
reject(err);
});
child.on('close', code => {
resolve(stdout);
});
});
}

public hasChildProcesses(processId: number | undefined): Promise<boolean> {
if (processId) {
// if shell has at least one child process, assume that shell is busy
if (isWindows) {
return this.spawnAsPromised('wmic', ['process', 'get', 'ParentProcessId']).then(stdout => {
const pids = stdout.split('\r\n');
return pids.some(p => parseInt(p) === processId);
}, error => true);
} else {
return this.spawnAsPromised('/usr/bin/pgrep', ['-lP', String(processId)]).then(stdout => {
const r = stdout.trim();
if (r.length === 0 || r.indexOf(' tmux') >= 0) { // ignore 'tmux';
return false;
} else {
return true;
}
}, error => true);
}
}
// fall back to safe side
return Promise.resolve(true);
}
}

0 comments on commit 6976745

Please sign in to comment.