maxmustermann/vscode
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

web - rewrite how storage is handled on shutdown

Browse source 
cc @sandy081
This commit is contained in:
Benjamin Pasero 2021-11-08 08:53:25 +01:00
parent bdbaac08a1
commit 8d5348aba6
No known key found for this signature in database
GPG key ID: E6380CC4C8219E65
10 changed files with 92 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
src/vs/base/parts/storage/common/storage.ts
View file

@ -68,6 +68,7 @@ export interface IStorage extends IDisposable {
set(key: string, value: string | boolean | number | undefined | null): Promise<void>;
delete(key: string): Promise<void>;
flush(delay?: number): Promise<void>;
whenFlushed(): Promise<void>;
close(): Promise<void>;
@ -236,7 +237,7 @@ export class Storage extends Disposable implements IStorage {
this._onDidChangeStorage.fire(key);
// Accumulate work by scheduling after timeout
return this.flushDelayer.trigger(() => this.flushPending());
return this.doFlush();
}
async delete(key: string): Promise<void> {
@ -260,7 +261,7 @@ export class Storage extends Disposable implements IStorage {
this._onDidChangeStorage.fire(key);
// Accumulate work by scheduling after timeout
return this.flushDelayer.trigger(() => this.flushPending());
return this.doFlush();
}
async close(): Promise<void> {
@ -283,7 +284,7 @@ export class Storage extends Disposable implements IStorage {
// Recovery: we pass our cache over as recovery option in case
// the DB is not healthy.
try {
await this.flushDelayer.trigger(() => this.flushPending(), 0 /* as soon as possible */);
await this.doFlush(0 /* as soon as possible */);
} catch (error) {
// Ignore
}
@ -318,6 +319,18 @@ export class Storage extends Disposable implements IStorage {
});
}
async flush(delay?: number): Promise<void> {
if (!this.hasPending) {
return; // return early if nothing to do
}
return this.doFlush(delay);
}
private async doFlush(delay?: number): Promise<void> {
return this.flushDelayer.trigger(() => this.flushPending(), delay);
}
async whenFlushed(): Promise<void> {
if (!this.hasPending) {
return; // return early if nothing to do

19
src/vs/base/parts/storage/test/node/storage.test.ts
View file

@ -222,6 +222,25 @@ flakySuite('Storage Library', function () {
await storage.close();
});
test('explicit flush', async () => {
let storage = new Storage(new SQLiteStorageDatabase(join(testDir, 'storage.db')));
await storage.init();
storage.set('foo', 'bar');
storage.set('bar', 'foo');
let flushPromiseResolved = false;
storage.whenFlushed().then(() => flushPromiseResolved = true);
strictEqual(flushPromiseResolved, false);
await storage.flush(0);
strictEqual(flushPromiseResolved, true);
await storage.close();
});
test('conflicting updates', () => {
return runWithFakedTimers({}, async function () {
let storage = new Storage(new SQLiteStorageDatabase(join(testDir, 'storage.db')));

1
src/vs/base/test/parts/quickinput/browser/quickinput.test.ts
View file

@ -167,7 +167,6 @@ suite('QuickInput', () => {
void (await new Promise<void>(resolve => {
quickpick.onDidAccept(() => {
console.log(quickpick.selectedItems.map(i => i.label).join(', '));
quickpick.canSelectMany = true;
quickpick.items = [{ label: 'a' }, { label: 'b' }, { label: 'c' }];
resolve();

20
src/vs/platform/storage/browser/storageService.ts
View file

@ -3,6 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { isSafari } from 'vs/base/browser/browser';
import { IndexedDB } from 'vs/base/browser/indexedDB';
import { Promises } from 'vs/base/common/async';
import { toErrorMessage } from 'vs/base/common/errorMessage';
@ -105,13 +106,20 @@ export class BrowserStorageService extends AbstractStorageService {
}
close(): void {
// We explicitly do not close our DBs because writing data onBeforeUnload()
// can result in unexpected results. Namely, it seems that - even though this
// operation is async - sometimes it is being triggered on unload and
// succeeds. Often though, the DBs turn out to be empty because the write
// never had a chance to complete.
// Safari: there is an issue where the page can hang on load when
// a previous session has kept IndexedDB transactions running.
// The only fix seems to be to cancel any pending transactions
// (https://github.com/microsoft/vscode/issues/136295)
//
// Instead we trigger dispose() to ensure that no timeouts or callbacks
// On all other browsers, we keep the databases opened because
// we expect data to be written when the unload happens.
if (isSafari) {
this.globalStorageDatabase?.close();
this.workspaceStorageDatabase?.close();
}
// Always dispose to ensure that no timeouts or callbacks
// get triggered in this phase.
this.dispose();
}

29
src/vs/platform/storage/common/storage.ts
View file

@ -461,16 +461,33 @@ export abstract class AbstractStorageService extends Disposable implements IStor
return this.getBoolean(IS_NEW_KEY, scope) === true;
}
async flush(reason: WillSaveStateReason = WillSaveStateReason.NONE): Promise<void> {
async flush(reason = WillSaveStateReason.NONE): Promise<void> {
// Signal event to collect changes
this._onWillSaveState.fire({ reason });
// Await flush
await Promises.settled([
this.getStorage(StorageScope.GLOBAL)?.whenFlushed() ?? Promise.resolve(),
this.getStorage(StorageScope.WORKSPACE)?.whenFlushed() ?? Promise.resolve()
]);
const globalStorage = this.getStorage(StorageScope.GLOBAL);
const workspaceStorage = this.getStorage(StorageScope.WORKSPACE);
switch (reason) {
// Unspecific reason: just wait when data is flushed
case WillSaveStateReason.NONE:
await Promises.settled([
globalStorage?.whenFlushed() ?? Promise.resolve(),
workspaceStorage?.whenFlushed() ?? Promise.resolve()
]);
break;
// Shutdown: we want to flush as soon as possible
// and not hit any delays that might be there
case WillSaveStateReason.SHUTDOWN:
await Promises.settled([
globalStorage?.flush(0) ?? Promise.resolve(),
workspaceStorage?.flush(0) ?? Promise.resolve()
]);
break;
}
}
async logStorage(): Promise<void> {

9
src/vs/workbench/browser/web.main.ts
View file

@ -70,7 +70,7 @@ import { IndexedDB } from 'vs/base/browser/indexedDB';
class BrowserMain extends Disposable {
private readonly onWillShutdownDisposables = new DisposableStore();
private readonly onWillShutdownDisposables = this._register(new DisposableStore());
constructor(
private readonly domElement: HTMLElement,
@ -138,11 +138,6 @@ class BrowserMain extends Disposable {
private registerListeners(workbench: Workbench, storageService: BrowserStorageService, logService: ILogService): void {
// Workbench Lifecycle
this._register(workbench.onBeforeShutdown(event => {
if (storageService.hasPendingUpdate) {
event.veto(true, 'veto.pendingStorageUpdate'); // prevent data loss from pending storage update
}
}));
this._register(workbench.onWillShutdown(() => this.onWillShutdownDisposables.clear()));
this._register(workbench.onDidShutdown(() => this.dispose()));
}
@ -348,7 +343,7 @@ class BrowserMain extends Disposable {
try {
await storageService.initialize();
// Close onWillShutdown
// Register to close on shutdown
this.onWillShutdownDisposables.add(toDisposable(() => storageService.close()));
return storageService;

6
src/vs/workbench/browser/workbench.ts
View file

@ -20,7 +20,7 @@ import { IStorageService, WillSaveStateReason, StorageScope, StorageTarget } fro
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
import { LifecyclePhase, ILifecycleService, WillShutdownEvent, BeforeShutdownEvent } from 'vs/workbench/services/lifecycle/common/lifecycle';
import { LifecyclePhase, ILifecycleService, WillShutdownEvent } from 'vs/workbench/services/lifecycle/common/lifecycle';
import { INotificationService } from 'vs/platform/notification/common/notification';
import { NotificationService } from 'vs/workbench/services/notification/common/notificationService';
import { NotificationsCenter } from 'vs/workbench/browser/parts/notifications/notificationsCenter';
@ -50,9 +50,6 @@ export interface IWorkbenchOptions {
export class Workbench extends Layout {
private readonly _onBeforeShutdown = this._register(new Emitter<BeforeShutdownEvent>());
readonly onBeforeShutdown = this._onBeforeShutdown.event;
private readonly _onWillShutdown = this._register(new Emitter<WillShutdownEvent>());
readonly onWillShutdown = this._onWillShutdown.event;
@ -239,7 +236,6 @@ export class Workbench extends Layout {
}
// Lifecycle
this._register(lifecycleService.onBeforeShutdown(event => this._onBeforeShutdown.fire(event)));
this._register(lifecycleService.onWillShutdown(event => this._onWillShutdown.fire(event)));
this._register(lifecycleService.onDidShutdown(() => {
this._onDidShutdown.fire();

16
src/vs/workbench/services/host/browser/browserHostService.ts
View file

@ -32,7 +32,6 @@ import Severity from 'vs/base/common/severity';
import { IDialogService } from 'vs/platform/dialogs/common/dialogs';
import { DomEmitter } from 'vs/base/browser/event';
import { isUndefined } from 'vs/base/common/types';
import { IStorageService, WillSaveStateReason } from 'vs/platform/storage/common/storage';
/**
* A workspace to open in the workbench can either be:
@ -109,8 +108,7 @@ export class BrowserHostService extends Disposable implements IHostService {
@IInstantiationService private readonly instantiationService: IInstantiationService,
@ILifecycleService private readonly lifecycleService: BrowserLifecycleService,
@ILogService private readonly logService: ILogService,
@IDialogService private readonly dialogService: IDialogService,
@IStorageService private readonly storageService: IStorageService
@IDialogService private readonly dialogService: IDialogService
) {
super();
@ -138,13 +136,6 @@ export class BrowserHostService extends Disposable implements IHostService {
private onBeforeShutdown(e: BeforeShutdownEvent): void {
// Optimistically trigger a UI state flush
// without waiting for it. The browser does
// not guarantee that this is being executed
// but if a dialog opens, we have a chance
// to succeed.
this.storageService.flush(WillSaveStateReason.SHUTDOWN);
switch (this.shutdownReason) {
// Unknown / Keyboard shows veto depending on setting
@ -469,10 +460,7 @@ export class BrowserHostService extends Disposable implements IHostService {
this.shutdownReason = HostShutdownReason.Api;
// Signal shutdown reason to lifecycle
this.lifecycleService.withExpectedShutdown(reason);
// Ensure UI state is persisted
await this.storageService.flush(WillSaveStateReason.SHUTDOWN);
return this.lifecycleService.withExpectedShutdown(reason);
}
//#endregion

18
src/vs/workbench/services/lifecycle/browser/lifecycleService.ts
View file

@ -10,7 +10,7 @@ import { localize } from 'vs/nls';
import { registerSingleton } from 'vs/platform/instantiation/common/extensions';
import { IDisposable } from 'vs/base/common/lifecycle';
import { addDisposableListener, EventType } from 'vs/base/browser/dom';
import { IStorageService } from 'vs/platform/storage/common/storage';
import { IStorageService, WillSaveStateReason } from 'vs/platform/storage/common/storage';
export class BrowserLifecycleService extends AbstractLifecycleService {
@ -99,9 +99,9 @@ export class BrowserLifecycleService extends AbstractLifecycleService {
event.returnValue = localize('lifecycleVeto', "Changes that you made may not be saved. Please check press 'Cancel' and try again.");
}
withExpectedShutdown(reason: ShutdownReason): void;
withExpectedShutdown(reason: { disableShutdownHandling: true }, callback: Function): void;
withExpectedShutdown(reason: ShutdownReason | { disableShutdownHandling: true }, callback?: Function): void {
withExpectedShutdown(reason: ShutdownReason): Promise<void>;
withExpectedShutdown(reason: { disableShutdownHandling: true }, callback: Function): Promise<void>;
withExpectedShutdown(reason: ShutdownReason | { disableShutdownHandling: true }, callback?: Function): Promise<void> {
// Standard shutdown
if (typeof reason === 'number') {
@ -117,6 +117,9 @@ export class BrowserLifecycleService extends AbstractLifecycleService {
this.disableBeforeUnloadVeto = false;
}
}
// Ensure UI state is persisted
return this.storageService.flush(WillSaveStateReason.SHUTDOWN);
}
shutdown(): void {
@ -135,6 +138,13 @@ export class BrowserLifecycleService extends AbstractLifecycleService {
this.didBeforeUnload = true;
// Optimistically trigger a UI state flush
// without waiting for it. The browser does
// not guarantee that this is being executed
// but if a dialog opens, we have a chance
// to succeed.
this.storageService.flush(WillSaveStateReason.SHUTDOWN);
let veto = false;
// Before Shutdown

2
src/vs/workbench/services/lifecycle/common/lifecycleService.ts
View file

@ -38,7 +38,7 @@ export abstract class AbstractLifecycleService extends Disposable implements ILi
constructor(
@ILogService protected readonly logService: ILogService,
@IStorageService private readonly storageService: IStorageService
@IStorageService protected readonly storageService: IStorageService
) {
super();