From 8b01745e991559e30c71cc38a0c99e0ea7d38724 Mon Sep 17 00:00:00 2001 From: Cai Xuye Date: Wed, 18 Dec 2019 18:57:54 +0800 Subject: [PATCH 1/2] fix #6767: don't reuse integrated terminal that has child processes Signed-off-by: Cai Xuye --- packages/debug/src/browser/debug-session.tsx | 9 +++- .../src/browser/base/terminal-widget.ts | 5 +++ .../src/browser/terminal-widget-impl.ts | 4 ++ .../src/common/shell-terminal-protocol.ts | 1 + .../src/node/shell-terminal-server.ts | 44 +++++++++++++++++++ 5 files changed, 62 insertions(+), 1 deletion(-) diff --git a/packages/debug/src/browser/debug-session.tsx b/packages/debug/src/browser/debug-session.tsx index 223f4f5b00ca1..670dc04a5d7ee 100644 --- a/packages/debug/src/browser/debug-session.tsx +++ b/packages/debug/src/browser/debug-session.tsx @@ -374,7 +374,14 @@ export class DebugSession implements CompositeTreeElement { } protected async doCreateTerminal(options: TerminalWidgetOptions): Promise { - 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(); diff --git a/packages/terminal/src/browser/base/terminal-widget.ts b/packages/terminal/src/browser/base/terminal-widget.ts index e63c91b851f02..8f04abf5aa0b0 100644 --- a/packages/terminal/src/browser/base/terminal-widget.ts +++ b/packages/terminal/src/browser/base/terminal-widget.ts @@ -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; } /** diff --git a/packages/terminal/src/browser/terminal-widget-impl.ts b/packages/terminal/src/browser/terminal-widget-impl.ts index f04d0ac74ba4b..1fa778db17014 100644 --- a/packages/terminal/src/browser/terminal-widget-impl.ts +++ b/packages/terminal/src/browser/terminal-widget-impl.ts @@ -263,6 +263,10 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget this.term.clear(); } + async hasChildProcesses(): Promise { + return this.shellTerminalServer.hasChildProcesses(await this.processId); + } + storeState(): object { this.closeOnDispose = false; return { terminalId: this.terminalId, titleLabel: this.title.label }; diff --git a/packages/terminal/src/common/shell-terminal-protocol.ts b/packages/terminal/src/common/shell-terminal-protocol.ts index 8cd3b83f9547a..88b6ed128d8c3 100644 --- a/packages/terminal/src/common/shell-terminal-protocol.ts +++ b/packages/terminal/src/common/shell-terminal-protocol.ts @@ -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; } export const shellTerminalPath = '/services/shell-terminal'; diff --git a/packages/terminal/src/node/shell-terminal-server.ts b/packages/terminal/src/node/shell-terminal-server.ts index 0c6bf8ce6af09..f954f6828e18e 100644 --- a/packages/terminal/src/node/shell-terminal-server.ts +++ b/packages/terminal/src/node/shell-terminal-server.ts @@ -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 { @@ -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 { + 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 { + 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); + } } From 44d87e7e62a909b7ecf714f226b5b72c73c48726 Mon Sep 17 00:00:00 2001 From: Cai Xuye Date: Wed, 18 Dec 2019 19:29:41 +0800 Subject: [PATCH 2/2] fix #6767: avoid using server title for integrated terminal Signed-off-by: Cai Xuye --- packages/debug/src/browser/debug-session.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/debug/src/browser/debug-session.tsx b/packages/debug/src/browser/debug-session.tsx index 670dc04a5d7ee..0def58c9ecae1 100644 --- a/packages/debug/src/browser/debug-session.tsx +++ b/packages/debug/src/browser/debug-session.tsx @@ -368,7 +368,7 @@ export class DebugSession implements CompositeTreeElement { } protected async runInTerminal({ arguments: { title, cwd, args, env } }: DebugProtocol.RunInTerminalRequest): Promise { - const terminal = await this.doCreateTerminal({ title, cwd, env }); + const terminal = await this.doCreateTerminal({ title, cwd, env, useServerTitle: false }); terminal.sendText(args.join(' ') + '\n'); return { processId: await terminal.processId }; }