fix: guard against disposed model in inline completions (fixes #318217)#318218
Open
vs-code-engineering[bot] wants to merge 1 commit into
Open
fix: guard against disposed model in inline completions (fixes #318217)#318218vs-code-engineering[bot] wants to merge 1 commit into
vs-code-engineering[bot] wants to merge 1 commit into
Conversation
When the text model is disposed during an observable update transaction, the _inlineSuggestionItems derived would call isVisible() which cascades into model.getValueInRange() on the disposed model. Add an early return guard to prevent this. Fixes #318217 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔧 Error Fix
Summary
Error:
Model is disposed!thrown fromtextModel._assertNotDisposedwhen_inlineSuggestionItemsderived observable recomputes during an observable update transaction after the text model has already been disposed.Impact: Hits across Windows, Mac, and Linux on VS Code 1.121.0 (commit
f6cfa2ea). The error fires during command execution (cursor edit) when the observable derivation chain propagates changes throughreportChanges→endUpdateand reaches the inline completions model’s visibility check.Fixes #318217
Recommended reviewer:
@hedietCulprit Commit
No single culprit commit identified — this is a latent race condition where the model disposal can occur mid-transaction while derived observables are still recomputing. The inline completions model (
inlineCompletionsModel.ts) has been maintained primarily by@hediet.Code Flow
sequenceDiagram participant CMD as EditorCommand participant VM as ViewModel participant TX as TransactionUpdate participant DRV as DerivedObservable participant ICM as InlineCompletionsModel participant TM as TextModel CMD->>VM: executeCommands VM->>TX: batchChanges / endUpdate TX->>DRV: reportChanges DRV->>ICM: _inlineSuggestionItems recompute ICM->>TM: isVisible → getValueInRange TM-->>TM: _assertNotDisposed → THROWSAffected Files
src/vs/editor/contrib/inlineCompletions/browser/model/inlineCompletionsModel.tsisVisibleon disposed modelsrc/vs/editor/contrib/inlineCompletions/browser/model/inlineCompletionIsVisible.tssingleTextRemoveCommonPrefixwhich hitsgetValueInRangesrc/vs/editor/contrib/inlineCompletions/browser/model/singleTextEditHelpers.tsmodel.getValueInRange(crash site)src/vs/editor/common/model/textModel.tsModel is disposed!in_assertNotDisposedRepro Steps
endUpdate→reportChangesand reaches_inlineSuggestionItemsafter the model is already disposedmodel.getValueInRange()on the disposed modelHow the Fix Works
Chosen approach (
inlineCompletionsModel.ts:524): Addedif (this.textModel.isDisposed()) { return undefined; }as a guard clause at the top of the_inlineSuggestionItemsderived computation, after checking for completions but before accessing the model. This prevents the derived from attempting visibility calculations on a disposed model — fixing at the point where the invalid state (disposed model) is first consumed, not at the crash site (textModel._assertNotDisposed). This follows the principle of adding a guard clause upstream of the crash site so the error path is never reached.Alternatives considered: Wrapping the
isVisiblecall in try/catch would hide the error from telemetry without addressing the root cause. Adding disposal checks insidesingleTextRemoveCommonPrefixorinlineCompletionIsVisiblewould be patching at the crash site rather than the data consumer that initiates the invalid access.Recommended Owner
@hediet— primary author and maintainer of the inline completions model and observable-based architecture in this area.