fix: guard against disposed model in quick diff diff() (fixes #324537)

[vs-code-engineering]

Summary

QuickDiffModel.diff() throws TypeError: Cannot read properties of null (reading 'getVersionId') at src/vs/workbench/contrib/scm/browser/quickDiffModel.ts:259. The method runs inside a progressService.withProgress(...) task that is scheduled by a 200ms ThrottledDelayer. The first statement of that task dereferences this._model.textEditorModel.getVersionId() — before the method's existing disposal guard. When the underlying TextFileEditorModel is disposed during the throttle window (file closed / reverted, editor destroyed), its textEditorModel getter returns null, and the synchronous getVersionId() call throws.

The declared type IResolvedTextFileEditorModel.textEditorModel: ITextModel is a compile-time snapshot; disposal invalidates it at runtime. So this is a use-after-dispose lifecycle race that surfaces as a type-contract TypeError.

Scenario in one sentence: the TextFileEditorModel is disposed during the ThrottledDelayer/withProgress window, its textEditorModel getter returns null, and the already-scheduled diff() task then dereferences it at line 259 before reaching the disposal guard at line 261.

Fixes #324537
Recommended reviewer: @hediet

Culprit Commit

a9f0f574fced779121cfbe595a039f420346894d — "adds proposed createQuickDiffInformation API, adopts it in markdown editor. (#323470)" by @hediet (Henning Dieterichs), 2026-06-29.

Evidence (verified by diffing the file at 61f0639c, 2026-03-29, and a9daa80e, 2026-01-20, against the shipped insider commit 3fbc14e6, 1.128.0-insider):

• Before this commit, the withProgress task began with const originalURIs = await this.getQuickDiffsPromise(); — there was no synchronous textEditorModel dereference and no versionId tracking, and _model was typed ITextFileEditorModel.
• This commit changed the _model field type to IResolvedTextFileEditorModel, added the changesVersionId getter, and inserted const versionId = this._model.textEditorModel.getVersionId(); as the first line of the task — above the existing this._disposed || this._model.isDisposed() guard.
• It also added | null to diff()'s return type and the !result branch in triggerDiff(), but never actually returned null — the disposed path was declared but never wired up. That is exactly the missing piece this fix completes.

All three revisions predate the shipped build, confirming the crash line ships in 3fbc14e6.

Code Flow

flowchart TD
    Edit[User edits / closes / reverts file] --> Trigger[triggerDiff schedules ThrottledDelayer 200ms]
    Dispose[TextFileEditorModel disposed during throttle window] --> Getter[textEditorModel getter returns null]
    Trigger --> Fire{Delayer fires}
    Fire --> Diff[diff runs inside progressService.withProgress]
    Diff --> Line259[line 259: this._model.textEditorModel.getVersionId]
    Getter --> Line259
    Line259 --> Crash[TypeError: Cannot read properties of null reading getVersionId]

Loading

Affected Files

• src/vs/workbench/contrib/scm/browser/quickDiffModel.ts — crash site (diff(), line 259) and the fix.
• src/vs/workbench/services/textfile/common/textfiles.ts — declares IResolvedTextFileEditorModel.textEditorModel: ITextModel (non-null narrowing of base ITextEditorModel.textEditorModel: ITextModel | null).
• src/vs/workbench/common/editor/textEditorModel.ts — the null mechanism: the textEditorModel getter returns null once the handle is cleared on dispose.

Repro Steps

1. Open a file tracked by an SCM / quick-diff provider so a QuickDiffModel is active.
2. Trigger a diff (edit the document, or change quick-diff providers) so triggerDiff() schedules the 200ms ThrottledDelayer.
3. Within that throttle window, cause the underlying TextFileEditorModel to be disposed (close / revert the editor, or otherwise destroy the model).
4. When the delayer fires, diff() runs and evaluates this._model.textEditorModel.getVersionId() on the disposed model → textEditorModel is null → TypeError.

Timing-dependent (dispose must land inside the throttle/progress window), which matches a low-frequency telemetry error.

How the Fix Works

Chosen approach (quickDiffModel.ts): add a disposal guard clause as the first statement inside the withProgress task, before the getVersionId() dereference at the bypass site (quickDiffModel.ts:259):

return this.progressService.withProgress({ location, delay: 250 }, async () => {
    if (this._disposed || this._model.isDisposed()) {
        return null; // the model was disposed while the diff was throttled/scheduled
    }

    const versionId = this._model.textEditorModel.getVersionId();
    ...

This fixes the lifecycle race at its source — the point where a disposed model is dereferenced — rather than guarding the crash-site symptom or coercing the null value downstream. It completes the disposal-safety design the culprit commit half-added: diff() already declares a | null return, and triggerDiff() already treats !result exactly like the disposed case (if (!result || this._disposed || ...) { return; }), so returning null here is the intended, already-handled signal. It reuses the established this._disposed || this._model.isDisposed() predicate that the same method applies two lines below, so no new contract or state is introduced. After this change, the getVersionId() line cannot run when the model is disposed, because the guard returns null first and the getter is never dereferenced on a disposed model.

Alternatives considered:

• Optional-chain / coerce at the crash site (textEditorModel?.getVersionId() ?? 0): rejected — it coerces the violating null to a benign substitute in the consumer and masks the use-after-dispose race instead of preventing it; a stale diff would still be computed against a dead model.
• Wrap the task body in try/catch: rejected — hides the error from telemetry and does not address why a disposed model is being processed.
• Return the empty-changes object instead of null: workable but redundant; null is already the method's declared disposed signal and is handled identically by the caller, so null is the minimal, most faithful choice.

Recommended Owner

@hediet (Henning Dieterichs) — author of the culprit commit a9f0f574 / PR #323470.

• Team Membership: core VS Code maintainer (hdieterichs@microsoft.com); authored the shipped release commit 3fbc14e6 itself, which requires write access. Pass.
• Liveness: multiple commits to microsoft/vscode within the last 90 days (e.g. 2026-07-03). Pass.

First candidate in the Step 5 cascade and passes both gates, so no fall-through to the SCM area owner (lszomoru) was needed.

Generated by errors-fix · 1.2K AIC · ⌖ 111.7 AIC · ⊞ 69.6K · ◷

[Copilot]

Copilot can't review bot-authored pull requests automatically. A user with Copilot access can request a review manually.

[Copilot]

Copilot can't review bot-authored pull requests automatically. A user with Copilot access can request a review manually.

[vs-code-engineering]

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@lszomoru

Matched files:

• src/vs/workbench/contrib/scm/browser/quickDiffModel.ts