Skip to content

fix: guard ListView dynamic-height probe against shrunk model (fixes #333550) #333561

Description

@vs-code-engineering

Summary

ListView.probeDynamicHeights dereferences this.items[index] for every index in the render range before the caller's shrink-detection guard runs. When a synchronous, reentrant model update (e.g. userDataProfilesEditor.ts set templates replacing the whole tree during a rerender) leaves the render range's end beyond the current item count, item is undefined and shouldProbeDynamicHeight throws TypeError: Cannot read properties of undefined (reading 'hasDynamicHeight'). It affects 843 users on stable 1.135.0 across all platforms.

Fixes #333550
Recommended reviewer: @connor4312

Culprit Commit

Field Value
Commit c2b336da
Author @connor4312
PR #330967
Message list: batch dynamic height measurements (#330967)
Why It rewrote _rerender to call probeDynamicHeights(renderRange, ...) (which eagerly reads this.items[index] across the whole range) before evaluating the new modelDidChange guard (this.items.length < renderRange.end). Prior to this change each index was probed one-at-a-time via probeDynamicHeight(i), so a shrunk model could not be read out of bounds.

Code Flow

sequenceDiagram
    participant Editor as userDataProfilesEditor (set templates)
    participant Tree as AsyncDataTree.rerender
    participant View as ListView._rerender
    participant Probe as probeDynamicHeights
    participant Guard as shouldProbeDynamicHeight

    Editor->>Tree: synchronous rerender after model replace
    Note over View: ⚠️ Root cause:<br/>renderRange.end > this.items.length<br/>after reentrant shrink
    View->>Probe: probeDynamicHeights(renderRange)
    Probe->>Guard: shouldProbeDynamicHeight(this.items[index]=undefined)
    Note over Guard: 💥 TypeError:<br/>reading 'hasDynamicHeight' of undefined
Loading

Affected Files

File Role Evidence
src/vs/base/browser/ui/list/listView.ts crash site L1853 (from stack): `if (!item.hasDynamicHeight
src/vs/base/browser/ui/list/listView.ts root cause L1768-1769: for (...; index < range.end; ...) { const item = this.items[index]; ...probeDynamicHeightFromDelegate(item) — probes full range before shrink guard
src/vs/base/browser/ui/list/listView.ts recovery guard (runs too late) L1618: const modelDidChange = this.items.length < renderRange.end — designed to restart the loop, but only checked after probeDynamicHeights already threw

Repro Steps

Non-deterministic (timing/reentrancy dependent). It occurs when a list/tree with dynamic heights has its model synchronously shrunk during a rerender while the render range still extends past the previous end:

  1. Open the User Data Profiles editor with a tree that has more rows visible than after an update.
  2. Trigger set templates (profile/template refresh) so the tree contents are replaced synchronously during an in-flight rerender.
  3. If the new model has fewer items than the prior render range end, probeDynamicHeights reads this.items[index] === undefined and throws.

To increase likelihood: rapidly refresh/replace tree contents while scrolled so the render range covers rows near the end of the list.

How the Fix Works

Chosen approach (src/vs/base/browser/ui/list/listView.ts): Clamp the probe loop's upper bound to the current item count — const end = Math.min(range.end, this.items.length) — so probeDynamicHeights never dereferences a non-existent item. This is a fix at the data producer (the loop that reads this.items), not a guard bolted onto the crash site shouldProbeDynamicHeight. The existing recovery machinery already handles the shrink: after probeDynamicHeights returns, the caller's modelDidChange check (this.items.length < renderRange.end) fires and restarts the measurement against the updated range. The premature out-of-bounds read was the only thing preventing that intended path from running. The diffs array keeps its original range.end - range.start length, so clamped-out entries stay 0 and the caller's index arithmetic is unchanged.

After this change, listView.ts:1774 cannot pass undefined to shouldProbeDynamicHeight because the loop only iterates indices < this.items.length, and this.items[index] for index < this.items.length is always a defined IItem<T>.

Alternatives considered: Adding if (!item) continue; or item?.hasDynamicHeight at the crash site — rejected because it guards the symptom at the bottom of the stack rather than the producer, and would silently skip valid entries without letting the modelDidChange recovery restart the measurement correctly.

Recommended Owner

@connor4312 — author of the culprit commit #330967 and an active core VS Code maintainer (recent commits within the last 90 days). Owns the batched dynamic-height measurement logic in listView.ts.

Generated by errors-fix · opus48 · 443 AIC · ⌖ 11.4 AIC · ⊞ 18.6K ·


Note

This was originally intended as a pull request, but PR creation failed. The changes have been pushed to the branch fix/listview-probe-dynamic-heights-oob-5428ade9ac259b23.

Original error: ERR_API: [2026-08-31T15:41:52.654Z] create pull request in microsoft/vscode failed (attempt 1)

Original error: Validation Failed: {"resource":"PullRequest","code":"custom","field":"fork_collab","message":"fork_collab Fork collab can't be granted by someone without permission"} - https://docs.github.com/rest/pulls/pulls#create-a-pull-request
Retryable: false
Suggestion: This error cannot be resolved by retrying. Please check the error details and fix the underlying issue.

To create the pull request manually:

gh pr create --title "fix: guard ListView dynamic-height probe against shrunk model (fixes #333550)" --base main --head vscodebot-pr:fix/listview-probe-dynamic-heights-oob-5428ade9ac259b23 --repo microsoft/vscode
Show patch (34 lines)
From 2c5d495513be9dab35126a620e113d0ce566e073 Mon Sep 17 00:00:00 2001
X-GH-AW-Base-Commit: dbc1fe965263c1716bbc5c0e54211e2ae33d5750
From: "github-actions[bot]" <github-actions[bot]@users.noreply.github.com>
Date: Mon, 31 Aug 2026 15:31:46 +0000
Subject: [PATCH] fix: guard dynamic-height probe against shrunk model in
 ListView probeDynamicHeights (fixes #333550)

---
 src/vs/base/browser/ui/list/listView.ts | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/vs/base/browser/ui/list/listView.ts b/src/vs/base/browser/ui/list/listView.ts
index aee77ac1234..beed0208b86 100644
--- a/src/vs/base/browser/ui/list/listView.ts
+++ b/src/vs/base/browser/ui/list/listView.ts
@@ -1765,7 +1765,13 @@ export class ListView<T> implements IListView<T> {
 		const diffs = new Array<number>(range.end - range.start).fill(0);
 		const measurements: IDynamicHeightMeasurement<T>[] = [];
 
-		for (let index = range.start; index < range.end; index++) {
+		// The model may have shrunk (e.g. a reentrant splice during a synchronous
+		// rerender) so that `range.end` now exceeds the current item count. Clamp
+		// the probe to the items that still exist; the caller's `modelDidChange`
+		// check (`this.items.length < range.end`) then detects the shrink and
+		// restarts the measurement against the updated range.
+		const end = Math.min(range.end, this.items.length);
+		for (let index = range.start; index < end; index++) {
 			const item = this.items[index];
 			const delegateHeightDiff = this.probeDynamicHeightFromDelegate(item);
 			if (delegateHeightDiff !== undefined) {
-- 
2.54.0

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions