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

pull reduce grace time into ptyService method #120195

Merged
merged 9 commits into from Apr 1, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/vs/platform/terminal/common/terminal.ts
Expand Up @@ -142,10 +142,8 @@ export interface IPtyService {

/**
* Lists all orphaned processes, ie. those without a connected frontend.
* @param reduceGraceTime Whether to reduce the reconnection grace time for all orphaned
* terminals.
*/
listProcesses(reduceGraceTime: boolean): Promise<IProcessDetails[]>;
listProcesses(): Promise<IProcessDetails[]>;

start(id: number): Promise<ITerminalLaunchError | undefined>;
shutdown(id: number, immediate: boolean): Promise<void>;
Expand All @@ -160,6 +158,7 @@ export interface IPtyService {

setTerminalLayoutInfo(args: ISetTerminalLayoutInfoArgs): Promise<void>;
getTerminalLayoutInfo(args: IGetTerminalLayoutInfoArgs): Promise<ITerminalsLayoutInfo | undefined>;
reduceConnectionGraceTime(): void;
}

export enum HeartbeatConstants {
Expand Down
8 changes: 5 additions & 3 deletions src/vs/platform/terminal/node/ptyHostService.ts
Expand Up @@ -153,10 +153,12 @@ export class PtyHostService extends Disposable implements IPtyService {
detachFromProcess(id: number): Promise<void> {
return this._proxy.detachFromProcess(id);
}
listProcesses(reduceGraceTime: boolean): Promise<IProcessDetails[]> {
return this._proxy.listProcesses(reduceGraceTime);
listProcesses(): Promise<IProcessDetails[]> {
return this._proxy.listProcesses();
}
reduceConnectionGraceTime(): void {
return this._proxy.reduceConnectionGraceTime();
}

start(id: number): Promise<ITerminalLaunchError | undefined> {
return this._proxy.start(id);
}
Expand Down
10 changes: 5 additions & 5 deletions src/vs/platform/terminal/node/ptyService.ts
Expand Up @@ -114,13 +114,13 @@ export class PtyService extends Disposable implements IPtyService {
this._throwIfNoPty(id).detach();
}

async listProcesses(reduceGraceTime: boolean): Promise<IProcessDetails[]> {
if (reduceGraceTime) {
for (const pty of this._ptys.values()) {
pty.reduceGraceTime();
}
reduceConnectionGraceTime(): void {
for (const pty of this._ptys.values()) {
pty.reduceGraceTime();
}
}

async listProcesses(): Promise<IProcessDetails[]> {
const persistentProcesses = Array.from(this._ptys.entries()).filter(([_, pty]) => pty.shouldPersistTerminal);

this._logService.info(`Listing ${persistentProcesses.length} persistent terminals, ${this._ptys.size} total terminals`);
Expand Down
Expand Up @@ -168,8 +168,8 @@ export class RemoteTerminalService extends Disposable implements IRemoteTerminal
return undefined;
}

public async listProcesses(reduceGraceTime: boolean = false): Promise<IRemoteTerminalAttachTarget[]> {
const terms = this._remoteTerminalChannel ? await this._remoteTerminalChannel.listProcesses(reduceGraceTime) : [];
public async listProcesses(): Promise<IRemoteTerminalAttachTarget[]> {
const terms = this._remoteTerminalChannel ? await this._remoteTerminalChannel.listProcesses() : [];
return terms.map(termDto => {
return <IRemoteTerminalAttachTarget>{
id: termDto.id,
Expand All @@ -190,8 +190,14 @@ export class RemoteTerminalService extends Disposable implements IRemoteTerminal
return this._remoteTerminalChannel.setTerminalLayoutInfo(layout);
}

public reduceConnectionGraceTime(): void {
if (!this._remoteTerminalChannel) {
throw new Error('Cannot reduce grace time when there is no remote');
}
this._remoteTerminalChannel.reduceConnectionGraceTime();
}

public async getTerminalLayoutInfo(): Promise<ITerminalsLayoutInfo | undefined> {
await this._remoteTerminalChannel?.listProcesses(true);
if (!this._remoteTerminalChannel) {
throw new Error(`Cannot call getActiveInstanceId when there is no remote`);
}
Expand Down
1 change: 1 addition & 0 deletions src/vs/workbench/contrib/terminal/browser/terminal.ts
Expand Up @@ -189,6 +189,7 @@ export interface ITerminalService {

export interface IRemoteTerminalService extends IOffProcessTerminalService {
createProcess(shellLaunchConfig: IShellLaunchConfig, activeWorkspaceRootUri: URI | undefined, cols: number, rows: number, shouldPersist: boolean, configHelper: ITerminalConfigHelper): Promise<ITerminalChildProcess>;
reduceConnectionGraceTime(): void;
}

/**
Expand Down
12 changes: 9 additions & 3 deletions src/vs/workbench/contrib/terminal/browser/terminalActions.ts
Expand Up @@ -670,9 +670,15 @@ export function registerTerminalActions() {
const labelService = accessor.get(ILabelService);
const remoteAgentService = accessor.get(IRemoteAgentService);
const notificationService = accessor.get(INotificationService);
const offProcTerminalService = remoteAgentService.getConnection() ? accessor.get(IRemoteTerminalService) : accessor.get(ILocalTerminalService);
const remoteTerms = await offProcTerminalService.listProcesses();
const unattachedTerms = remoteTerms.filter(term => !terminalService.isAttachedToTerminal(term));
let offProcTerminalService = remoteAgentService.getConnection() ? accessor.get(IRemoteTerminalService) : accessor.get(ILocalTerminalService);

const terms = await offProcTerminalService.listProcesses();

if ('reduceConnectionGraceTime' in offProcTerminalService) {
offProcTerminalService.reduceConnectionGraceTime();
}
Copy link
Member

Choose a reason for hiding this comment

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

Thinking about this more, let's do this in local too so they work the same. It shouldn't hurt to reduce here since when attach is run


const unattachedTerms = terms.filter(term => !terminalService.isAttachedToTerminal(term));
const items = unattachedTerms.map(term => {
const cwdLabel = labelService.getUriLabel(URI.file(term.cwd));
return {
Expand Down
Expand Up @@ -189,6 +189,7 @@ export class TerminalService implements ITerminalService {
private async _reconnectToRemoteTerminals(): Promise<void> {
// Reattach to all remote terminals
const layoutInfo = await this._remoteTerminalService.getTerminalLayoutInfo();
this._remoteTerminalService.reduceConnectionGraceTime();
const reconnectCounter = this._recreateTerminalTabs(layoutInfo);
/* __GDPR__
"terminalReconnection" : {
Expand Down
Expand Up @@ -232,11 +232,12 @@ export class RemoteTerminalChannelClient {
public attachToProcess(id: number): Promise<void> {
return this._channel.call('$attachToProcess', [id]);
}

public listProcesses(reduceGraceTime: boolean): Promise<IProcessDetails[]> {
return this._channel.call('$listProcesses', [reduceGraceTime]);
public listProcesses(): Promise<IProcessDetails[]> {
return this._channel.call('$listProcesses');
}
public reduceConnectionGraceTime(): Promise<void> {
return this._channel.call('$reduceConnectionGraceTime');
}

public start(id: number): Promise<ITerminalLaunchError | void> {
return this._channel.call('$start', [id]);
}
Expand All @@ -261,7 +262,6 @@ export class RemoteTerminalChannelClient {
public orphanQuestionReply(id: number): Promise<void> {
return this._channel.call('$orphanQuestionReply', [id]);
}

public sendCommandResult(reqId: number, isError: boolean, payload: any): Promise<void> {
return this._channel.call<void>('$sendCommandResult', [reqId, isError, payload]);
}
Expand Down
Expand Up @@ -116,8 +116,8 @@ export class LocalTerminalService extends Disposable implements ILocalTerminalSe
return undefined;
}

public async listProcesses(reduceGraceTime: boolean): Promise<IProcessDetails[]> {
return this._localPtyService.listProcesses(reduceGraceTime);
public async listProcesses(): Promise<IProcessDetails[]> {
return this._localPtyService.listProcesses();
}

public async setTerminalLayoutInfo(layoutInfo?: ITerminalsLayoutInfoById): Promise<void> {
Expand Down