From 099445be72f9430392be9beaaaf944ecb5356e0b Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sat, 10 Sep 2022 07:52:24 +0200 Subject: [PATCH] debt - avoid `statSync` when computing workspace identifier --- src/vs/code/electron-main/main.ts | 5 +- src/vs/code/node/cli.ts | 4 +- .../electron-main/diagnosticsMainService.ts | 56 +++++++++---------- src/vs/platform/environment/node/wait.ts | 2 +- src/vs/platform/workspaces/node/workspaces.ts | 18 +++--- .../test/electron-main/workspaces.test.ts | 2 +- src/vs/server/node/server.cli.ts | 4 +- 7 files changed, 46 insertions(+), 45 deletions(-) diff --git a/src/vs/code/electron-main/main.ts b/src/vs/code/electron-main/main.ts index d2e59149d7d..e358efbb545 100644 --- a/src/vs/code/electron-main/main.ts +++ b/src/vs/code/electron-main/main.ts @@ -34,7 +34,7 @@ import { DiagnosticsService } from 'vs/platform/diagnostics/node/diagnosticsServ import { NativeParsedArgs } from 'vs/platform/environment/common/argv'; import { EnvironmentMainService, IEnvironmentMainService } from 'vs/platform/environment/electron-main/environmentMainService'; import { addArg, parseMainProcessArgv } from 'vs/platform/environment/node/argvHelper'; -import { createWaitMarkerFile } from 'vs/platform/environment/node/wait'; +import { createWaitMarkerFileSync } from 'vs/platform/environment/node/wait'; import { IFileService } from 'vs/platform/files/common/files'; import { FileService } from 'vs/platform/files/common/fileService'; import { DiskFileSystemProvider } from 'vs/platform/files/node/diskFileSystemProvider'; @@ -469,8 +469,9 @@ class CodeMain { // // Note: we are not doing this if the wait marker has been already // added as argument. This can happen if Code was started from CLI. + if (args.wait && !args.waitMarkerFilePath) { - const waitMarkerFilePath = createWaitMarkerFile(args.verbose); + const waitMarkerFilePath = createWaitMarkerFileSync(args.verbose); if (waitMarkerFilePath) { addArg(process.argv, '--waitMarkerFilePath', waitMarkerFilePath); args.waitMarkerFilePath = waitMarkerFilePath; diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 190d64038e9..daae8a10f16 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -19,7 +19,7 @@ import { NativeParsedArgs } from 'vs/platform/environment/common/argv'; import { buildHelpMessage, buildVersionMessage, OPTIONS } from 'vs/platform/environment/node/argv'; import { addArg, parseCLIProcessArgv } from 'vs/platform/environment/node/argvHelper'; import { getStdinFilePath, hasStdinWithoutTty, readFromStdin, stdinDataListener } from 'vs/platform/environment/node/stdin'; -import { createWaitMarkerFile } from 'vs/platform/environment/node/wait'; +import { createWaitMarkerFileSync } from 'vs/platform/environment/node/wait'; import product from 'vs/platform/product/common/product'; import { CancellationTokenSource } from 'vs/base/common/cancellation'; import { randomPath } from 'vs/base/common/extpath'; @@ -220,7 +220,7 @@ export async function main(argv: string[]): Promise { // is closed and then exit the waiting process. let waitMarkerFilePath: string | undefined; if (args.wait) { - waitMarkerFilePath = createWaitMarkerFile(verbose); + waitMarkerFilePath = createWaitMarkerFileSync(verbose); if (waitMarkerFilePath) { addArg(argv, '--waitMarkerFilePath', waitMarkerFilePath); } diff --git a/src/vs/platform/diagnostics/electron-main/diagnosticsMainService.ts b/src/vs/platform/diagnostics/electron-main/diagnosticsMainService.ts index 4d618da59dc..3d10b4b8a15 100644 --- a/src/vs/platform/diagnostics/electron-main/diagnosticsMainService.ts +++ b/src/vs/platform/diagnostics/electron-main/diagnosticsMainService.ts @@ -42,33 +42,33 @@ export class DiagnosticsMainService implements IDiagnosticsMainService { async getRemoteDiagnostics(options: IRemoteDiagnosticOptions): Promise<(IRemoteDiagnosticInfo | IRemoteDiagnosticError)[]> { const windows = this.windowsMainService.getWindows(); - const diagnostics: Array = await Promise.all(windows.map(window => { - return new Promise((resolve) => { - const remoteAuthority = window.remoteAuthority; - if (remoteAuthority) { - const replyChannel = `vscode:getDiagnosticInfoResponse${window.id}`; - const args: IDiagnosticInfoOptions = { - includeProcesses: options.includeProcesses, - folders: options.includeWorkspaceMetadata ? this.getFolderURIs(window) : undefined - }; + const diagnostics: Array = await Promise.all(windows.map(async window => { + const remoteAuthority = window.remoteAuthority; + if (!remoteAuthority) { + return undefined; + } - window.sendWhenReady('vscode:getDiagnosticInfo', CancellationToken.None, { replyChannel, args }); + const replyChannel = `vscode:getDiagnosticInfoResponse${window.id}`; + const args: IDiagnosticInfoOptions = { + includeProcesses: options.includeProcesses, + folders: options.includeWorkspaceMetadata ? await this.getFolderURIs(window) : undefined + }; - validatedIpcMain.once(replyChannel, (_: IpcEvent, data: IRemoteDiagnosticInfo) => { - // No data is returned if getting the connection fails. - if (!data) { - resolve({ hostName: remoteAuthority, errorMessage: `Unable to resolve connection to '${remoteAuthority}'.` }); - } + return new Promise(resolve => { + window.sendWhenReady('vscode:getDiagnosticInfo', CancellationToken.None, { replyChannel, args }); - resolve(data); - }); + validatedIpcMain.once(replyChannel, (_: IpcEvent, data: IRemoteDiagnosticInfo) => { + // No data is returned if getting the connection fails. + if (!data) { + resolve({ hostName: remoteAuthority, errorMessage: `Unable to resolve connection to '${remoteAuthority}'.` }); + } - setTimeout(() => { - resolve({ hostName: remoteAuthority, errorMessage: `Connection to '${remoteAuthority}' could not be established` }); - }, 5000); - } else { - resolve(undefined); - } + resolve(data); + }); + + setTimeout(() => { + resolve({ hostName: remoteAuthority, errorMessage: `Connection to '${remoteAuthority}' could not be established` }); + }, 5000); }); })); @@ -82,7 +82,7 @@ export class DiagnosticsMainService implements IDiagnosticsMainService { for (const window of BrowserWindow.getAllWindows()) { const codeWindow = this.windowsMainService.getWindowById(window.id); if (codeWindow) { - windows.push(this.codeWindowToInfo(codeWindow)); + windows.push(await this.codeWindowToInfo(codeWindow)); } else { windows.push(this.browserWindowToInfo(window)); } @@ -97,8 +97,8 @@ export class DiagnosticsMainService implements IDiagnosticsMainService { }; } - private codeWindowToInfo(window: ICodeWindow): IWindowDiagnostics { - const folderURIs = this.getFolderURIs(window); + private async codeWindowToInfo(window: ICodeWindow): Promise { + const folderURIs = await this.getFolderURIs(window); const win = assertIsDefined(window.win); return this.browserWindowToInfo(win, folderURIs, window.remoteAuthority); @@ -113,14 +113,14 @@ export class DiagnosticsMainService implements IDiagnosticsMainService { }; } - private getFolderURIs(window: ICodeWindow): URI[] { + private async getFolderURIs(window: ICodeWindow): Promise { const folderURIs: URI[] = []; const workspace = window.openedWorkspace; if (isSingleFolderWorkspaceIdentifier(workspace)) { folderURIs.push(workspace.uri); } else if (isWorkspaceIdentifier(workspace)) { - const resolvedWorkspace = this.workspacesManagementMainService.resolveLocalWorkspaceSync(workspace.configPath); // workspace folders can only be shown for local (resolved) workspaces + const resolvedWorkspace = await this.workspacesManagementMainService.resolveLocalWorkspace(workspace.configPath); // workspace folders can only be shown for local (resolved) workspaces if (resolvedWorkspace) { const rootFolders = resolvedWorkspace.folders; rootFolders.forEach(root => { diff --git a/src/vs/platform/environment/node/wait.ts b/src/vs/platform/environment/node/wait.ts index 0be07ad2266..793d8e0467e 100644 --- a/src/vs/platform/environment/node/wait.ts +++ b/src/vs/platform/environment/node/wait.ts @@ -7,7 +7,7 @@ import { writeFileSync } from 'fs'; import { tmpdir } from 'os'; import { randomPath } from 'vs/base/common/extpath'; -export function createWaitMarkerFile(verbose?: boolean): string | undefined { +export function createWaitMarkerFileSync(verbose?: boolean): string | undefined { const randomWaitMarkerPath = randomPath(tmpdir()); try { diff --git a/src/vs/platform/workspaces/node/workspaces.ts b/src/vs/platform/workspaces/node/workspaces.ts index 9ca6e95faa7..beffbbe2776 100644 --- a/src/vs/platform/workspaces/node/workspaces.ts +++ b/src/vs/platform/workspaces/node/workspaces.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { createHash } from 'crypto'; -import { Stats, statSync } from 'fs'; +import { Stats } from 'fs'; import { Schemas } from 'vs/base/common/network'; import { isLinux, isMacintosh, isWindows } from 'vs/base/common/platform'; import { originalFSPath } from 'vs/base/common/resources'; @@ -53,13 +53,15 @@ export function getSingleFolderWorkspaceIdentifier(folderUri: URI, folderStat?: return createHash('md5').update(folderUri.toString()).digest('hex'); } - // Local: produce a hash from the path and include creation time as salt + // Local: we use the ctime as extra salt to the + // identifier so that folders getting recreated + // result in a different identifier. However, if + // the stat is not provided we return `undefined` + // to ensure identifiers are stable for the given + // URI. + if (!folderStat) { - try { - folderStat = statSync(folderUri.fsPath); - } catch (error) { - return undefined; // folder does not exist - } + return undefined; } let ctime: number | undefined; @@ -75,8 +77,6 @@ export function getSingleFolderWorkspaceIdentifier(folderUri: URI, folderStat?: } } - // we use the ctime as extra salt to the ID so that we catch the case of a folder getting - // deleted and recreated. in that case we do not want to carry over previous state return createHash('md5').update(folderUri.fsPath).update(ctime ? String(ctime) : '').digest('hex'); } diff --git a/src/vs/platform/workspaces/test/electron-main/workspaces.test.ts b/src/vs/platform/workspaces/test/electron-main/workspaces.test.ts index d0a8bc804a7..42427b5ea6d 100644 --- a/src/vs/platform/workspaces/test/electron-main/workspaces.test.ts +++ b/src/vs/platform/workspaces/test/electron-main/workspaces.test.ts @@ -41,7 +41,7 @@ flakySuite('Workspaces', () => { fs.mkdirSync(path.join(testDir, 'f1')); const localExistingUri = URI.file(path.join(testDir, 'f1')); - const localExistingUriId = getSingleFolderWorkspaceIdentifier(localExistingUri); + const localExistingUriId = getSingleFolderWorkspaceIdentifier(localExistingUri, fs.statSync(localExistingUri.fsPath)); assert.ok(localExistingUriId?.id); }); diff --git a/src/vs/server/node/server.cli.ts b/src/vs/server/node/server.cli.ts index db42c9ffe3b..d7b1c9b307e 100644 --- a/src/vs/server/node/server.cli.ts +++ b/src/vs/server/node/server.cli.ts @@ -12,7 +12,7 @@ import { cwd } from 'vs/base/common/process'; import { dirname, extname, resolve, join } from 'vs/base/common/path'; import { parseArgs, buildHelpMessage, buildVersionMessage, OPTIONS, OptionDescriptions, ErrorReporter } from 'vs/platform/environment/node/argv'; import { NativeParsedArgs } from 'vs/platform/environment/common/argv'; -import { createWaitMarkerFile } from 'vs/platform/environment/node/wait'; +import { createWaitMarkerFileSync } from 'vs/platform/environment/node/wait'; import { PipeCommand } from 'vs/workbench/api/node/extHostCLIServer'; import { hasStdinWithoutTty, getStdinFilePath, readFromStdin } from 'vs/platform/environment/node/stdin'; @@ -306,7 +306,7 @@ export async function main(desc: ProductDescription, args: string[]): Promise