Inline chat widget not disposed when closing untitled editor with pending edits

[joyceerhl]

1. Open an untitled text editor
2. Start an inline chat session
3. Generate some code for the editor
4. Close the editor, choose Don't Save when prompted
5. Open another untitled text editor
6. 🐛 inline chat is open and cannot be dismissed with esc, throws the following error

Model is disposed!: Error: Model is disposed!
    at TextModel._assertNotDisposed (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/editor/common/model/textModel.js:265:23)
    at TextModel.getVersionId (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/editor/common/model/textModel.js:475:18)
    at DecorationsTrees.getNodeRange (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/editor/common/model/textModel.js:1622:36)
    at TextModel.getDecorationRange (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/editor/common/model/textModel.js:1286:42)
    at get value [as value] (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/inlineChat/browser/inlineChatSession.js:50:47)
    at updateWholeRangeDecoration (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/inlineChat/browser/inlineChatController.js:226:63)
    at InlineChatController.INIT_UI (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/inlineChat/browser/inlineChatController.js:232:13)
    at InlineChatController._nextState (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/inlineChat/browser/inlineChatController.js:166:50)
    at async UniqueContainer.value (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/inlineChat/browser/inlineChatController.js:90:17)
Unexpected type: Error: Unexpected type
    at assertType (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/base/common/types.js:92:19)
    at InlineChatController.CREATE_SESSION (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/inlineChat/browser/inlineChatController.js:170:36)
    at InlineChatController._nextState (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/inlineChat/browser/inlineChatController.js:166:50)
    at InlineChatController.run (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/inlineChat/browser/inlineChatController.js:137:37)
    at StartSessionAction.runEditorCommand (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/inlineChat/browser/inlineChatActions.js:42:70)
    at vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/editor/browser/editorExtensions.js:303:29
    at InstantiationService.invokeFunction (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/platform/instantiation/common/instantiationService.js:46:24)
    at CodeEditorWidget.invokeWithinContext (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/editor/browser/widget/codeEditorWidget.js:246:47)
    at StartSessionAction.run (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/editor/browser/editorExtensions.js:295:27)
    at handler (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/platform/actions/common/actions.js:369:52)
    at InstantiationService.invokeFunction (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/platform/instantiation/common/instantiationService.js:46:24)
    at CommandService._tryExecuteCommand (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/services/commands/common/commandService.js:81:59)
    at CommandService.executeCommand (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/services/commands/common/commandService.js:50:33)
    at HTMLAnchorElement.handleClick (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/codeEditor/browser/emptyTextEditorHint/emptyTextEditorHint.js:142:42)
Model is disposed!: Error: Model is disposed!
    at TextModel._assertNotDisposed (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/editor/common/model/textModel.js:265:23)
    at TextModel.getValueInRange (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/editor/common/model/textModel.js:545:18)
    at Session.asChangedText (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/inlineChat/browser/inlineChatSession.js:139:36)
    at InlineChatController.cancelSession (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/inlineChat/browser/inlineChatController.js:682:49)
    at DiscardAction.runInlineChatCommand (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/inlineChat/browser/inlineChatActions.js:290:18)
    at DiscardAction.runEditorCommand (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/contrib/inlineChat/browser/inlineChatActions.js:97:18)
    at vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/editor/browser/editorExtensions.js:303:29
    at InstantiationService.invokeFunction (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/platform/instantiation/common/instantiationService.js:46:24)
    at EmbeddedCodeEditorWidget.invokeWithinContext (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/editor/browser/widget/codeEditorWidget.js:246:47)
    at DiscardAction.run (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/editor/browser/editorExtensions.js:295:27)
    at handler (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/platform/actions/common/actions.js:369:52)
    at InstantiationService.invokeFunction (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/platform/instantiation/common/instantiationService.js:46:24)
    at CommandService._tryExecuteCommand (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/services/commands/common/commandService.js:81:59)
    at CommandService.executeCommand (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/services/commands/common/commandService.js:50:33)
    at WorkbenchKeybindingService._doDispatch (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/platform/keybinding/common/abstractKeybindingService.js:256:50)
    at WorkbenchKeybindingService._dispatch (vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/platform/keybinding/common/abstractKeybindingService.js:142:25)
    at vscode-file://vscode-app/c:/Users/huer/Source/vscode/out/vs/workbench/services/keybinding/browser/keybindingService.js:166:51
log.ts:441

[jrieken]

@bpasero I keep a reference onto text models (via textModelService.createModelReference here) but it seems that untitled text model get disposed anyways. What's the deal with that? Is that expected?

[bpasero]

@jrieken yeah, untitled working copies are always self-disposing upon revert (which is what happens when you close an untitled editor without saving):

vscode/src/vs/workbench/services/untitled/common/untitledTextEditorModel.ts

Lines 282 to 285 in 0c5c400

	// A reverted untitled model is invalid because it has
	// no actual source on disk to revert to. As such we
	// dispose the model.
	this.dispose();

Would it make sense to listen to the dispose of models as well to clean up? How does it work today when you open a normal text file and close the editor?

[jrieken]

How does it work today when you open a normal text file and close the editor?

When that happens the controller gets disposed which can then do the cleanup of the resources it acquired. (There is actually a corner-case in which this doesn't happen but it can be fixed at the right place)

Would it make sense to listen to the dispose of models as well to clean up?

This would be very hard because the whole design is based on acquiring a reference and releasing that. Assuming "bottom up freeing" isn't a good fit in that model, it basically raises the question why we acquire and release references in the first place. Also the code must than assume that disposal can happen at any time and for instance teardown the UI, discard edits etc pp.

So, I'd rather prefer to see untitled files/editor behave like other models and also use the ref-counting model so that down-level code doesn't have to know about special cases like this

[bpasero]

The current behaviour is historic and from a time before we had the resolver service and ref-counted models. We can try to change this behaviour but I am bit worried about regressions. This opens up the ability to have a untitled model in memory without associated editor, which we never had so far. On the other hand, we allow this for normal file based editors, so it might be just fine. We still forcefully open any dirty model as editor to indicate unsaved work to the user, and we need to ensure that this still works when we introduce ref-counting down there.

I am exploring what this would mean in https://github.com/microsoft/vscode/tree/ben/untitled-ref-count but will probably not make more progress until next debt week.

An idea would be to let revert simply set the editor model to an empty string (or null?) and clear the dirty state.

[bpasero]

A consequence of this change (in #194371) is that when you open an untitled document via:

vscode.workspace.openTextDocument({
	content: ''
}).then((doc) => {
	vscode.window.showTextDocument(doc);
});

And then close it without making a change, the model will stay around because it is not being reverted in that case because it is not dirty. As such this never fires:

vscode/src/vs/workbench/api/browser/mainThreadDocuments.ts

Line 282 in c97d74f

Event.once(model.onDidRevert)(() => this._modelReferenceCollection.remove(resource));

Not sure how bad this is in real world scenarios, but we might need to revisit that particular case.

[jrieken]

Yeah, let's see but we always and everywhere stated that closing documents in entirely up to the editor