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 5382fb5777e..a9d9bc5aa34 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/tree.test.ts @@ -104,6 +104,114 @@ suite('vscode API - tree', () => { } }); + test('TreeView - element already registered after rapid root refresh', async function () { + this.timeout(60_000); + + // This test reproduces a race condition where rapid concurrent getChildren calls + // return different element object instances that have the same ID in their TreeItem, + // causing "Element with id ... is already registered" error. + // + // The bug: When _addChildrenToClear(undefined) is called, it clears _childrenFetchTokens. + // If two fetches are pending, both may reset the requestId counter to 1, so both think + // they are the current request. When both try to register elements with the same ID + // but different object instances, the error is thrown. + + type TreeElement = { readonly kind: 'leaf'; readonly instance: number }; + + class RapidRefreshTreeDataProvider implements vscode.TreeDataProvider { + private readonly changeEmitter = new vscode.EventEmitter(); + private readonly requestEmitter = new vscode.EventEmitter(); + private readonly pendingRequests: DeferredPromise[] = []; + // Return different element instance each time + private element1: TreeElement = { kind: 'leaf', instance: 1 }; + private element2: TreeElement = { kind: 'leaf', instance: 2 }; + + readonly onDidChangeTreeData = this.changeEmitter.event; + + getChildren(element?: TreeElement): Thenable { + if (!element) { + const deferred = new DeferredPromise(); + this.pendingRequests.push(deferred); + this.requestEmitter.fire(this.pendingRequests.length); + return deferred.p; + } + return Promise.resolve([]); + } + + getTreeItem(): vscode.TreeItem { + // Both element instances return the same id + const item = new vscode.TreeItem('test element', vscode.TreeItemCollapsibleState.None); + item.id = 'same-id-each-time'; + return item; + } + + getParent(): TreeElement | undefined { + return undefined; + } + + getElement1(): TreeElement { + return this.element1; + } + + getElement2(): TreeElement { + return this.element2; + } + + async waitForRequestCount(count: number): Promise { + while (this.pendingRequests.length < count) { + await asPromise(this.requestEmitter.event); + } + } + + resolveRequestWithElement(index: number, element: TreeElement): void { + const request = this.pendingRequests[index]; + if (request) { + request.complete([element]); + } + } + + dispose(): void { + this.changeEmitter.dispose(); + this.requestEmitter.dispose(); + while (this.pendingRequests.length) { + this.pendingRequests.shift()!.complete([]); + } + } + } + + const provider = new RapidRefreshTreeDataProvider(); + disposables.push(provider); + + const treeView = vscode.window.createTreeView('test.treeRapidRefresh', { treeDataProvider: provider }); + disposables.push(treeView); + + // Start two concurrent reveal operations - this should trigger two getChildren calls + // Similar to the first test + const firstReveal = (treeView.reveal(provider.getElement1(), { expand: true }) + .then(() => ({ error: undefined as Error | undefined })) as Promise<{ error: Error | undefined }>) + .catch(error => ({ error })); + + const secondReveal = (treeView.reveal(provider.getElement2(), { expand: true }) + .then(() => ({ error: undefined as Error | undefined })) as Promise<{ error: Error | undefined }>) + .catch(error => ({ error })); + + // Wait for both getChildren calls to be pending + await provider.waitForRequestCount(2); + + // Resolve requests returning DIFFERENT element instances with SAME id + // First request returns element1, second returns element2 + // Both elements have the same id 'same-id-each-time' in getTreeItem + provider.resolveRequestWithElement(0, provider.getElement1()); + await delay(0); + provider.resolveRequestWithElement(1, provider.getElement2()); + + 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); + } + }); + test('TreeView - element already registered after refresh', async function () { this.timeout(60_000); diff --git a/src/vs/workbench/api/common/extHostTreeViews.ts b/src/vs/workbench/api/common/extHostTreeViews.ts index f50257579be..f848dc043a1 100644 --- a/src/vs/workbench/api/common/extHostTreeViews.ts +++ b/src/vs/workbench/api/common/extHostTreeViews.ts @@ -325,6 +325,10 @@ class ExtHostTreeView extends Disposable { // 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(); + // Global counter for fetch tokens. Using a monotonically increasing counter ensures that even after + // _childrenFetchTokens.clear() during a root refresh, old in-flight fetches will have requestIds that + // can never match new fetches (e.g., old fetch has id=5, after clear new fetches get 6, 7, 8...). + private _globalFetchTokenCounter = 0; private _visible: boolean = false; get visible(): boolean { return this._visible; } @@ -737,8 +741,7 @@ class ExtHostTreeView extends Disposable { // clear children cache this._addChildrenToClear(parentElement); const fetchKey = this._getFetchKey(parentElement); - let requestId = this._childrenFetchTokens.get(fetchKey) ?? 0; - requestId++; + const requestId = ++this._globalFetchTokenCounter; this._childrenFetchTokens.set(fetchKey, requestId); const cts = new CancellationTokenSource(this._refreshCancellationSource.token);