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

Use workspace ID in revive pty id map #186947

Merged
merged 1 commit into from
Jul 3, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/vs/platform/terminal/common/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ export interface IPtyService extends IPtyHostController {
getProfiles?(workspaceId: string, profiles: unknown, defaultProfile: unknown, includeDetectedProfiles?: boolean): Promise<ITerminalProfile[]>;
getEnvironment(): Promise<IProcessEnvironment>;
getWslPath(original: string, direction: 'unix-to-win' | 'win-to-unix'): Promise<string>;
getRevivedPtyNewId(id: number): Promise<number | undefined>;
getRevivedPtyNewId(workspaceId: string, id: number): Promise<number | undefined>;
setTerminalLayoutInfo(args: ISetTerminalLayoutInfoArgs): Promise<void>;
getTerminalLayoutInfo(args: IGetTerminalLayoutInfoArgs): Promise<ITerminalsLayoutInfo | undefined>;
reduceConnectionGraceTime(): Promise<void>;
Expand All @@ -353,7 +353,7 @@ export interface IPtyService extends IPtyHostController {
* Revives a workspaces terminal processes, these can then be reconnected to using the normal
* flow for restoring terminals after reloading.
*/
reviveTerminalProcesses(state: ISerializedTerminalState[], dateTimeFormatLocate: string): Promise<void>;
reviveTerminalProcesses(workspaceId: string, state: ISerializedTerminalState[], dateTimeFormatLocate: string): Promise<void>;
refreshProperty<T extends ProcessPropertyType>(id: number, property: T): Promise<IProcessPropertyMap[T]>;
updateProperty<T extends ProcessPropertyType>(id: number, property: T, value: IProcessPropertyMap[T]): Promise<void>;

Expand Down
8 changes: 4 additions & 4 deletions src/vs/platform/terminal/node/ptyHostService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,8 @@ export class PtyHostService extends Disposable implements IPtyService {
return this._proxy.getWslPath(original, direction);
}

getRevivedPtyNewId(id: number): Promise<number | undefined> {
return this._proxy.getRevivedPtyNewId(id);
getRevivedPtyNewId(workspaceId: string, id: number): Promise<number | undefined> {
return this._proxy.getRevivedPtyNewId(workspaceId, id);
}

setTerminalLayoutInfo(args: ISetTerminalLayoutInfoArgs): Promise<void> {
Expand Down Expand Up @@ -328,8 +328,8 @@ export class PtyHostService extends Disposable implements IPtyService {
return this._proxy.serializeTerminalState(ids);
}

async reviveTerminalProcesses(state: ISerializedTerminalState[], dateTimeFormatLocate: string) {
return this._proxy.reviveTerminalProcesses(state, dateTimeFormatLocate);
async reviveTerminalProcesses(workspaceId: string, state: ISerializedTerminalState[], dateTimeFormatLocate: string) {
return this._proxy.reviveTerminalProcesses(workspaceId, state, dateTimeFormatLocate);
}

async refreshProperty<T extends ProcessPropertyType>(id: number, property: T): Promise<IProcessPropertyMap[T]> {
Expand Down
38 changes: 22 additions & 16 deletions src/vs/platform/terminal/node/ptyService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class PtyService extends Disposable implements IPtyService {
private readonly _ptys: Map<number, PersistentTerminalProcess> = new Map();
private readonly _workspaceLayoutInfos = new Map<WorkspaceId, ISetTerminalLayoutInfoArgs>();
private readonly _detachInstanceRequestStore: RequestStore<IProcessDetails | undefined, { workspaceId: string; instanceId: number }>;
private readonly _revivedPtyIdMap: Map<number, { newId: number; state: ISerializedTerminalState }> = new Map();
private readonly _revivedPtyIdMap: Map<string, { newId: number; state: ISerializedTerminalState }> = new Map();
private readonly _autoReplies: Map<string, string> = new Map();

private _lastPtyId: number = 0;
Expand Down Expand Up @@ -210,15 +210,15 @@ export class PtyService extends Disposable implements IPtyService {
}

@traceRpc
async reviveTerminalProcesses(state: ISerializedTerminalState[], dateTimeFormatLocale: string) {
async reviveTerminalProcesses(workspaceId: string, state: ISerializedTerminalState[], dateTimeFormatLocale: string) {
const promises: Promise<void>[] = [];
for (const terminal of state) {
promises.push(this._reviveTerminalProcess(terminal));
promises.push(this._reviveTerminalProcess(workspaceId, terminal));
}
await Promise.all(promises);
}

private async _reviveTerminalProcess(terminal: ISerializedTerminalState): Promise<void> {
private async _reviveTerminalProcess(workspaceId: string, terminal: ISerializedTerminalState): Promise<void> {
const restoreMessage = localize('terminal-history-restored', "History restored");
// TODO: We may at some point want to show date information in a hover via a custom sequence:
// new Date(terminal.timestamp).toLocaleDateString(dateTimeFormatLocale)
Expand Down Expand Up @@ -246,8 +246,9 @@ export class PtyService extends Disposable implements IPtyService {
terminal.replayEvent.events[0].data
);
// Don't start the process here as there's no terminal to answer CPR
this._revivedPtyIdMap.set(terminal.id, { newId, state: terminal });
this._logService.info(`Revived process, old id ${terminal.id} -> new id ${newId}`);
const oldId = this._getRevivingProcessId(workspaceId, terminal.id);
this._revivedPtyIdMap.set(oldId, { newId, state: terminal });
this._logService.info(`Revived process, old id ${oldId} -> new id ${newId}`);
}

@traceRpc
Expand Down Expand Up @@ -499,11 +500,11 @@ export class PtyService extends Disposable implements IPtyService {
}

@traceRpc
async getRevivedPtyNewId(id: number): Promise<number | undefined> {
async getRevivedPtyNewId(workspaceId: string, id: number): Promise<number | undefined> {
try {
return this._revivedPtyIdMap.get(id)?.newId;
return this._revivedPtyIdMap.get(this._getRevivingProcessId(workspaceId, id))?.newId;
} catch (e) {
this._logService.warn(`Couldn't find terminal ID ${id}`, e.message);
this._logService.warn(`Couldn't find terminal ID ${workspaceId}-${id}`, e.message);
}
return undefined;
}
Expand All @@ -518,7 +519,7 @@ export class PtyService extends Disposable implements IPtyService {
performance.mark('code/willGetTerminalLayoutInfo');
const layout = this._workspaceLayoutInfos.get(args.workspaceId);
if (layout) {
const expandedTabs = await Promise.all(layout.tabs.map(async tab => this._expandTerminalTab(tab)));
const expandedTabs = await Promise.all(layout.tabs.map(async tab => this._expandTerminalTab(args.workspaceId, tab)));
const tabs = expandedTabs.filter(t => t.terminals.length > 0);
performance.mark('code/didGetTerminalLayoutInfo');
return { tabs };
Expand All @@ -527,8 +528,8 @@ export class PtyService extends Disposable implements IPtyService {
return undefined;
}

private async _expandTerminalTab(tab: ITerminalTabLayoutInfoById): Promise<ITerminalTabLayoutInfoDto> {
const expandedTerminals = (await Promise.all(tab.terminals.map(t => this._expandTerminalInstance(t))));
private async _expandTerminalTab(workspaceId: string, tab: ITerminalTabLayoutInfoById): Promise<ITerminalTabLayoutInfoDto> {
const expandedTerminals = (await Promise.all(tab.terminals.map(t => this._expandTerminalInstance(workspaceId, t))));
const filtered = expandedTerminals.filter(term => term.terminal !== null) as IRawTerminalInstanceLayoutInfo<IProcessDetails>[];
return {
isActive: tab.isActive,
Expand All @@ -537,11 +538,12 @@ export class PtyService extends Disposable implements IPtyService {
};
}

private async _expandTerminalInstance(t: ITerminalInstanceLayoutInfoById): Promise<IRawTerminalInstanceLayoutInfo<IProcessDetails | null>> {
private async _expandTerminalInstance(workspaceId: string, t: ITerminalInstanceLayoutInfoById): Promise<IRawTerminalInstanceLayoutInfo<IProcessDetails | null>> {
try {
const revivedPtyId = this._revivedPtyIdMap.get(t.terminal)?.newId;
this._logService.info(`Expanding terminal instance, old id ${t.terminal} -> new id ${revivedPtyId}`);
this._revivedPtyIdMap.delete(t.terminal);
const oldId = this._getRevivingProcessId(workspaceId, t.terminal);
const revivedPtyId = this._revivedPtyIdMap.get(oldId)?.newId;
this._logService.info(`Expanding terminal instance, old id ${oldId} -> new id ${revivedPtyId}`);
this._revivedPtyIdMap.delete(oldId);
const persistentProcessId = revivedPtyId ?? t.terminal;
const persistentProcess = this._throwIfNoPty(persistentProcessId);
const processDetails = persistentProcess && await this._buildProcessDetails(t.terminal, persistentProcess, revivedPtyId !== undefined);
Expand All @@ -561,6 +563,10 @@ export class PtyService extends Disposable implements IPtyService {
}
}

private _getRevivingProcessId(workspaceId: string, ptyId: number): string {
return `${workspaceId}-${ptyId}`;
}

private async _buildProcessDetails(id: number, persistentProcess: PersistentTerminalProcess, wasRevived: boolean = false): Promise<IProcessDetails> {
performance.mark(`code/willBuildProcessDetails/${id}`);
// If the process was just revived, don't do the orphan check as it will
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke
async attachToRevivedProcess(id: number): Promise<ITerminalChildProcess | undefined> {
await this._connectToDirectProxy();
try {
const newId = await this._proxy.getRevivedPtyNewId(id) ?? id;
const newId = await this._proxy.getRevivedPtyNewId(this._getWorkspaceId(), id) ?? id;
return await this.attachToProcess(newId);
} catch (e) {
this._logService.warn(`Couldn't attach to process ${e.message}`);
Expand Down Expand Up @@ -288,9 +288,8 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke
}

async getTerminalLayoutInfo(): Promise<ITerminalsLayoutInfo | undefined> {
const layoutArgs: IGetTerminalLayoutInfoArgs = {
workspaceId: this._getWorkspaceId()
};
const workspaceId = this._getWorkspaceId();
const layoutArgs: IGetTerminalLayoutInfoArgs = { workspaceId };

// Revive processes if needed
const serializedState = this._storageService.get(TerminalStorageKeys.TerminalBufferState, StorageScope.WORKSPACE);
Expand All @@ -317,7 +316,7 @@ class LocalTerminalBackend extends BaseTerminalBackend implements ITerminalBacke
mark('code/terminal/didGetReviveEnvironments');

mark('code/terminal/willReviveTerminalProcesses');
await this._proxy.reviveTerminalProcesses(parsed, Intl.DateTimeFormat().resolvedOptions().locale);
await this._proxy.reviveTerminalProcesses(workspaceId, parsed, Intl.DateTimeFormat().resolvedOptions().locale);
mark('code/terminal/didReviveTerminalProcesses');
this._storageService.remove(TerminalStorageKeys.TerminalBufferState, StorageScope.WORKSPACE);
// If reviving processes, send the terminal layout info back to the pty host as it
Expand Down