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

Fixes race conditions of writing and reading settings from multiple windows #126299

Merged
merged 1 commit into from
Jun 15, 2021
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
4 changes: 4 additions & 0 deletions src/vs/code/electron-main/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ import { getRemoteAuthority } from 'vs/platform/remote/common/remoteHosts';
import { ISignService } from 'vs/platform/sign/common/sign';
import { IExternalTerminalMainService } from 'vs/platform/externalTerminal/common/externalTerminal';
import { LinuxExternalTerminalService, MacExternalTerminalService, WindowsExternalTerminalService } from 'vs/platform/externalTerminal/node/externalTerminalService';
import { UserConfigurationFileServiceId, UserConfigurationFileService } from 'vs/platform/configuration/common/userConfigurationFileService';

/**
* The main VS Code application. There will only ever be one instance,
Expand Down Expand Up @@ -588,6 +589,9 @@ export class CodeApplication extends Disposable {
const launchChannel = ProxyChannel.fromService(accessor.get(ILaunchMainService), { disableMarshalling: true });
this.mainProcessNodeIpcServer.registerChannel('launch', launchChannel);

// Configuration
mainProcessElectronServer.registerChannel(UserConfigurationFileServiceId, ProxyChannel.fromService(new UserConfigurationFileService(this.environmentMainService, this.fileService, this.logService)));

// Update
const updateChannel = new UpdateChannel(accessor.get(IUpdateService));
mainProcessElectronServer.registerChannel('update', updateChannel);
Expand Down
22 changes: 20 additions & 2 deletions src/vs/workbench/services/configuration/browser/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { URI } from 'vs/base/common/uri';
import { Event, Emitter } from 'vs/base/common/event';
import * as errors from 'vs/base/common/errors';
import { Disposable, IDisposable, dispose, toDisposable, MutableDisposable, combinedDisposable, DisposableStore } from 'vs/base/common/lifecycle';
import { RunOnceScheduler } from 'vs/base/common/async';
import { RunOnceScheduler, timeout } from 'vs/base/common/async';
import { FileChangeType, FileChangesEvent, IFileService, whenProviderRegistered, FileOperationError, FileOperationResult } from 'vs/platform/files/common/files';
import { ConfigurationModel, ConfigurationModelParser, ConfigurationParseOptions, UserSettings } from 'vs/platform/configuration/common/configurationModels';
import { WorkspaceConfigurationModelParser, StandaloneConfigurationModelParser } from 'vs/workbench/services/configuration/common/configurationModels';
Expand All @@ -23,6 +23,7 @@ import { hash } from 'vs/base/common/hash';
import { IUriIdentityService } from 'vs/workbench/services/uriIdentity/common/uriIdentity';
import { ILogService } from 'vs/platform/log/common/log';
import { IStringDictionary } from 'vs/base/common/collections';
import { ResourceMap } from 'vs/base/common/map';

export class UserConfiguration extends Disposable {

Expand Down Expand Up @@ -93,6 +94,10 @@ class FileServiceBasedConfiguration extends Disposable {
private readonly _onDidChange: Emitter<void> = this._register(new Emitter<void>());
readonly onDidChange: Event<void> = this._onDidChange.event;

private readonly resourcesContentMap = new ResourceMap<boolean>(uri => this.uriIdentityService.extUri.getComparisonKey(uri));

private disposed: boolean = false;

constructor(
name: string,
private readonly settingsResource: URI,
Expand All @@ -116,19 +121,32 @@ class FileServiceBasedConfiguration extends Disposable {
this._cache = new ConfigurationModel();

this._register(Event.debounce(Event.filter(this.fileService.onDidFilesChange, e => this.handleFileEvents(e)), () => undefined, 100)(() => this._onDidChange.fire()));
this._register(toDisposable(() => this.disposed = true));
}

async resolveContents(): Promise<[string | undefined, [string, string | undefined][]]> {

const resolveContents = async (resources: URI[]): Promise<(string | undefined)[]> => {
return Promise.all(resources.map(async resource => {
try {
const content = (await this.fileService.readFile(resource, { atomic: true })).value.toString();
let content = (await this.fileService.readFile(resource, { atomic: true })).value.toString();

// If file is empty and had content before then file would have been truncated by node because of parallel writes from other windows
// To prevent such case, retry reading the file in 20ms intervals until file has content or max 5 trials or disposed.
// https://github.com/microsoft/vscode/issues/115740 https://github.com/microsoft/vscode/issues/125970
for (let trial = 1; !content && this.resourcesContentMap.get(resource) && !this.disposed && trial <= 5; trial++) {
await timeout(20);
this.logService.debug(`Retry (${trial}): Reading the configuration file`, resource.toString());
content = (await this.fileService.readFile(resource)).value.toString();
}

this.resourcesContentMap.set(resource, !!content);
if (!content) {
this.logService.debug(`Configuration file '${resource.toString()}' is empty`);
}
return content;
} catch (error) {
this.resourcesContentMap.delete(resource);
this.logService.trace(`Error while resolving configuration file '${resource.toString()}': ${errors.getErrorMessage(error)}`);
if ((<FileOperationError>error).fileOperationResult !== FileOperationResult.FILE_NOT_FOUND
&& (<FileOperationError>error).fileOperationResult !== FileOperationResult.FILE_NOT_DIRECTORY) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { registerMainProcessRemoteService } from 'vs/platform/ipc/electron-sandbox/services';
import { IUserConfigurationFileService, UserConfigurationFileServiceId } from 'vs/platform/configuration/common/userConfigurationFileService';

registerMainProcessRemoteService(IUserConfigurationFileService, UserConfigurationFileServiceId);
2 changes: 0 additions & 2 deletions src/vs/workbench/workbench.common.main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,7 @@ import { OpenerService } from 'vs/editor/browser/services/openerService';
import { IOpenerService } from 'vs/platform/opener/common/opener';
import { IgnoredExtensionsManagementService, IIgnoredExtensionsManagementService } from 'vs/platform/userDataSync/common/ignoredExtensions';
import { ExtensionsStorageSyncService, IExtensionsStorageSyncService } from 'vs/platform/userDataSync/common/extensionsStorageSync';
import { IUserConfigurationFileService, UserConfigurationFileService } from 'vs/platform/configuration/common/userConfigurationFileService';

registerSingleton(IUserConfigurationFileService, UserConfigurationFileService);
registerSingleton(IIgnoredExtensionsManagementService, IgnoredExtensionsManagementService);
registerSingleton(IGlobalExtensionEnablementService, GlobalExtensionEnablementService);
registerSingleton(IExtensionsStorageSyncService, ExtensionsStorageSyncService);
Expand Down
1 change: 1 addition & 0 deletions src/vs/workbench/workbench.sandbox.main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import 'vs/platform/diagnostics/electron-sandbox/diagnosticsService';
import 'vs/platform/checksum/electron-sandbox/checksumService';
import 'vs/platform/telemetry/electron-sandbox/customEndpointTelemetryService';
import 'vs/workbench/services/files/electron-sandbox/elevatedFileService';
import 'vs/workbench/services/configuration/electron-sandbox/userConfigurationFileService';

import { registerSingleton } from 'vs/platform/instantiation/common/extensions';
import { IUserDataInitializationService, UserDataInitializationService } from 'vs/workbench/services/userData/browser/userDataInit';
Expand Down
2 changes: 2 additions & 0 deletions src/vs/workbench/workbench.web.main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ import { TitlebarPart } from 'vs/workbench/browser/parts/titlebar/titlebarPart';
import { ITimerService, TimerService } from 'vs/workbench/services/timer/browser/timerService';
import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver';
import { ConfigurationResolverService } from 'vs/workbench/services/configurationResolver/browser/configurationResolverService';
import { IUserConfigurationFileService, UserConfigurationFileService } from 'vs/platform/configuration/common/userConfigurationFileService';

registerSingleton(IUserConfigurationFileService, UserConfigurationFileService);
registerSingleton(IWorkbenchExtensionManagementService, ExtensionManagementService);
registerSingleton(IAccessibilityService, AccessibilityService, true);
registerSingleton(IContextMenuService, ContextMenuService);
Expand Down