Fix MdDocumentInfoCache computing values twice (#152799)

* Fix MdDocumentInfoCache computing values twice

Fixes a race where values could be computed twice before being cached

* Remove only
This commit is contained in:
Matt Bierner 2022-06-21 16:22:07 -07:00 committed by GitHub
parent 3406329190
commit 389aa8a935
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 72 additions and 17 deletions

View file

@ -0,0 +1,33 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import * as assert from 'assert';
import 'mocha';
import { InMemoryDocument } from '../util/inMemoryDocument';
import { MdDocumentInfoCache } from '../util/workspaceCache';
import { InMemoryWorkspaceMarkdownDocuments } from './inMemoryWorkspace';
import { workspacePath } from './util';
suite('DocumentInfoCache', () => {
test('Repeated calls should only compute value once', async () => {
const doc = workspacePath('doc.md');
const workspace = new InMemoryWorkspaceMarkdownDocuments([
new InMemoryDocument(doc, '')
]);
let i = 0;
const cache = new MdDocumentInfoCache<number>(workspace, async () => {
return ++i;
});
const a = cache.get(doc);
const b = cache.get(doc);
assert.strictEqual(await a, 1);
assert.strictEqual(i, 1);
assert.strictEqual(await b, 1);
assert.strictEqual(i, 1);
});
});

View file

@ -43,6 +43,7 @@ class LazyResourceMap<T> {
export class MdDocumentInfoCache<T> extends Disposable {
private readonly _cache = new LazyResourceMap<T>();
private readonly _loadingDocuments = new ResourceMap<Promise<SkinnyTextDocument | undefined>>();
public constructor(
private readonly workspaceContents: MdWorkspaceContents,
@ -50,19 +51,28 @@ export class MdDocumentInfoCache<T> extends Disposable {
) {
super();
this._register(this.workspaceContents.onDidChangeMarkdownDocument(doc => this.onDidChangeDocument(doc)));
this._register(this.workspaceContents.onDidCreateMarkdownDocument(doc => this.onDidChangeDocument(doc)));
this._register(this.workspaceContents.onDidChangeMarkdownDocument(doc => this.invalidate(doc)));
this._register(this.workspaceContents.onDidDeleteMarkdownDocument(this.onDidDeleteDocument, this));
}
public async get(resource: vscode.Uri): Promise<T | undefined> {
const existing = this._cache.get(resource);
let existing = this._cache.get(resource);
if (existing) {
return existing;
}
const doc = await this.workspaceContents.getOrLoadMarkdownDocument(resource);
return doc && this.onDidChangeDocument(doc, true)?.value;
const doc = await this.loadDocument(resource);
if (!doc) {
return undefined;
}
// Check if we have invalidated
existing = this._cache.get(resource);
if (existing) {
return existing;
}
return this.resetEntry(doc)?.value;
}
public async getForDocument(document: SkinnyTextDocument): Promise<T> {
@ -70,21 +80,33 @@ export class MdDocumentInfoCache<T> extends Disposable {
if (existing) {
return existing;
}
return this.onDidChangeDocument(document, true)!.value;
return this.resetEntry(document).value;
}
public async entries(): Promise<Array<[vscode.Uri, T]>> {
return this._cache.entries();
}
private onDidChangeDocument(document: SkinnyTextDocument, forceAdd = false): Lazy<Promise<T>> | undefined {
if (forceAdd || this._cache.has(document.uri)) {
const value = lazy(() => this.getValue(document));
this._cache.set(document.uri, value);
return value;
private loadDocument(resource: vscode.Uri): Promise<SkinnyTextDocument | undefined> {
const existing = this._loadingDocuments.get(resource);
if (existing) {
return existing;
}
const p = this.workspaceContents.getOrLoadMarkdownDocument(resource);
this._loadingDocuments.set(resource, p);
p.finally(() => {
this._loadingDocuments.delete(resource);
});
return p;
}
private resetEntry(document: SkinnyTextDocument): Lazy<Promise<T>> {
const value = lazy(() => this.getValue(document));
this._cache.set(document.uri, value);
return value;
}
private invalidate(document: SkinnyTextDocument): void {
if (this._cache.has(document.uri)) {
this.resetEntry(document);
}
return undefined;
}
private onDidDeleteDocument(resource: vscode.Uri) {