Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #6767: don't reuse integrated terminal that has child processes #6769

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions packages/debug/src/browser/debug-session.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -368,13 +368,20 @@ export class DebugSession implements CompositeTreeElement {
}

protected async runInTerminal({ arguments: { title, cwd, args, env } }: DebugProtocol.RunInTerminalRequest): Promise<DebugProtocol.RunInTerminalResponse['body']> {
const terminal = await this.doCreateTerminal({ title, cwd, env });
const terminal = await this.doCreateTerminal({ title, cwd, env, useServerTitle: false });
paul-marechal marked this conversation as resolved.
Show resolved Hide resolved
terminal.sendText(args.join(' ') + '\n');
return { processId: await terminal.processId };
}

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;
vince-fugnitto marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -263,6 +263,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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a CQ for the following copied code:
https://dev.eclipse.org/ipzilla/show_bug.cgi?id=21340

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);
}
}