diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts index 1d19bff6902..5382fb5777e 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts @@ -103,4 +103,143 @@ suite('vscode API - tree', () => { assert.fail(error.message); } }); + + test('TreeView - element already registered after refresh', async function () { + this.timeout(60_000); + + type ParentElement = { readonly kind: 'parent' }; + type ChildElement = { readonly kind: 'leaf'; readonly version: number }; + type TreeElement = ParentElement | ChildElement; + + class ParentRefreshTreeDataProvider implements vscode.TreeDataProvider { + private readonly changeEmitter = new vscode.EventEmitter(); + private readonly rootRequestEmitter = new vscode.EventEmitter(); + private readonly childRequestEmitter = new vscode.EventEmitter(); + private readonly rootRequests: DeferredPromise[] = []; + private readonly childRequests: DeferredPromise[] = []; + private readonly parentElement: ParentElement = { kind: 'parent' }; + private childVersion = 0; + private currentChild: ChildElement = { kind: 'leaf', version: 0 }; + + readonly onDidChangeTreeData = this.changeEmitter.event; + + getChildren(element?: TreeElement): Thenable { + if (!element) { + const deferred = new DeferredPromise(); + this.rootRequests.push(deferred); + this.rootRequestEmitter.fire(this.rootRequests.length); + return deferred.p; + } + if (element.kind === 'parent') { + const deferred = new DeferredPromise(); + this.childRequests.push(deferred); + this.childRequestEmitter.fire(this.childRequests.length); + return deferred.p; + } + return Promise.resolve([]); + } + + getTreeItem(element: TreeElement): vscode.TreeItem { + if (element.kind === 'parent') { + const item = new vscode.TreeItem('parent', vscode.TreeItemCollapsibleState.Collapsed); + item.id = 'parent'; + return item; + } + const item = new vscode.TreeItem('duplicate', vscode.TreeItemCollapsibleState.None); + item.id = 'dup'; + return item; + } + + getParent(element: TreeElement): TreeElement | undefined { + if (element.kind === 'leaf') { + return this.parentElement; + } + return undefined; + } + + getCurrentChild(): ChildElement { + return this.currentChild; + } + + replaceChild(): ChildElement { + this.childVersion++; + this.currentChild = { kind: 'leaf', version: this.childVersion }; + return this.currentChild; + } + + async waitForRootRequestCount(count: number): Promise { + while (this.rootRequests.length < count) { + await asPromise(this.rootRequestEmitter.event); + } + } + + async waitForChildRequestCount(count: number): Promise { + while (this.childRequests.length < count) { + await asPromise(this.childRequestEmitter.event); + } + } + + async resolveNextRootRequest(elements?: TreeElement[]): Promise { + const next = this.rootRequests.shift(); + if (!next) { + return; + } + await next.complete(elements ?? [this.parentElement]); + } + + async resolveChildRequestAt(index: number, elements?: TreeElement[]): Promise { + const request = this.childRequests[index]; + if (!request) { + return; + } + this.childRequests.splice(index, 1); + await request.complete(elements ?? [this.currentChild]); + } + + dispose(): void { + this.changeEmitter.dispose(); + this.rootRequestEmitter.dispose(); + this.childRequestEmitter.dispose(); + while (this.rootRequests.length) { + this.rootRequests.shift()!.complete([]); + } + while (this.childRequests.length) { + this.childRequests.shift()!.complete([]); + } + } + } + + const provider = new ParentRefreshTreeDataProvider(); + disposables.push(provider); + + const treeView = vscode.window.createTreeView('test.treeRefresh', { treeDataProvider: provider }); + disposables.push(treeView); + + const initialChild = provider.getCurrentChild(); + const firstReveal = (treeView.reveal(initialChild, { expand: true }) + .then(() => ({ error: undefined as Error | undefined })) as Promise<{ error: Error | undefined }>) + .catch(error => ({ error })); + + await provider.waitForRootRequestCount(1); + await provider.resolveNextRootRequest(); + + await provider.waitForChildRequestCount(1); + const staleChild = provider.getCurrentChild(); + const refreshedChild = provider.replaceChild(); + const secondReveal = (treeView.reveal(refreshedChild, { expand: true }) + .then(() => ({ error: undefined as Error | undefined })) as Promise<{ error: Error | undefined }>) + .catch(error => ({ error })); + + await provider.waitForChildRequestCount(2); + + await provider.resolveChildRequestAt(1, [refreshedChild]); + await delay(0); + await provider.resolveChildRequestAt(0, [staleChild]); + + const [firstResult, secondResult] = await Promise.all([firstReveal, secondReveal]); + const error = firstResult.error ?? secondResult.error; + if (error && /Element with id .+ is already registered/.test(error.message)) { + assert.fail(error.message); + } + }); }); diff --git a/src/vs/workbench/api/common/extHostTreeViews.ts b/src/vs/workbench/api/common/extHostTreeViews.ts index 55bd6ffe2d2..f50257579be 100644 --- a/src/vs/workbench/api/common/extHostTreeViews.ts +++ b/src/vs/workbench/api/common/extHostTreeViews.ts @@ -314,6 +314,7 @@ class ExtHostTreeView extends Disposable { private static readonly LABEL_HANDLE_PREFIX = '0'; private static readonly ID_HANDLE_PREFIX = '1'; + private static readonly ROOT_FETCH_KEY = Symbol('extHostTreeViewRoot'); private readonly _dataProvider: vscode.TreeDataProvider; private readonly _dndController: vscode.TreeDragAndDropController | undefined; @@ -321,6 +322,9 @@ class ExtHostTreeView extends Disposable { private _roots: TreeNode[] | undefined = undefined; private _elements: Map = new Map(); private _nodes: Map = new Map(); + // Track the latest child-fetch per element so that refresh-triggered cache clears ignore stale results. + // Without these tokens, an earlier getChildren promise resolving after refresh would re-register handles and hit the duplicate-id guard. + private readonly _childrenFetchTokens = new Map(); private _visible: boolean = false; get visible(): boolean { return this._visible; } @@ -725,14 +729,25 @@ class ExtHostTreeView extends Disposable { return this._roots; } + private _getFetchKey(parentElement?: T): T | typeof ExtHostTreeView.ROOT_FETCH_KEY { + return parentElement ?? ExtHostTreeView.ROOT_FETCH_KEY; + } + private async _fetchChildrenNodes(parentElement?: T): Promise { // clear children cache this._addChildrenToClear(parentElement); + const fetchKey = this._getFetchKey(parentElement); + let requestId = this._childrenFetchTokens.get(fetchKey) ?? 0; + requestId++; + this._childrenFetchTokens.set(fetchKey, requestId); const cts = new CancellationTokenSource(this._refreshCancellationSource.token); try { const elements = await this._dataProvider.getChildren(parentElement); + if (this._childrenFetchTokens.get(fetchKey) !== requestId) { + return undefined; + } const parentNode = parentElement ? this._nodes.get(parentElement) : undefined; if (cts.token.isCancellationRequested) { @@ -743,12 +758,18 @@ class ExtHostTreeView extends Disposable { const treeItems = await Promise.all(coalesce(coalescedElements).map(element => { return this._dataProvider.getTreeItem(element); })); + if (this._childrenFetchTokens.get(fetchKey) !== requestId) { + return undefined; + } if (cts.token.isCancellationRequested) { return undefined; } // createAndRegisterTreeNodes adds the nodes to a cache. This must be done sync so that they get added in the correct order. const items = treeItems.map((item, index) => item ? this._createAndRegisterTreeNode(coalescedElements[index], item, parentNode) : null); + if (this._childrenFetchTokens.get(fetchKey) !== requestId) { + return undefined; + } return coalesce(items); } finally { @@ -1062,6 +1083,7 @@ class ExtHostTreeView extends Disposable { }); this._nodes.clear(); this._elements.clear(); + this._childrenFetchTokens.clear(); } private _clearNodes(nodes: TreeNode[]): void { @@ -1075,6 +1097,7 @@ class ExtHostTreeView extends Disposable { this._nodes.clear(); dispose(this._nodesToClear); this._nodesToClear.clear(); + this._childrenFetchTokens.clear(); } override dispose() {