Remove reference from disposed TextModel when keeping undo-redo elements

alexdima · alexdima · commit b867b319cdc2 · 2020-03-23T11:35:10.000+01:00
diff --git a/src/vs/editor/common/model/editStack.ts b/src/vs/editor/common/model/editStack.ts
@@ -137,14 +137,17 @@ class SingleModelEditStackData {
 
 export class SingleModelEditStackElement implements IResourceUndoRedoElement {
 
-	public model: ITextModel;
+	public model: ITextModel | URI;
 	private _data: SingleModelEditStackData | ArrayBuffer;
 
 	public get type(): UndoRedoElementType.Resource {
 		return UndoRedoElementType.Resource;
 	}
 
 	public get resource(): URI {
+		if (URI.isUri(this.model)) {
+			return this.model;
+		}
 		return this.model.uri;
 	}
 
@@ -157,7 +160,7 @@ export class SingleModelEditStackElement implements IResourceUndoRedoElement {
 		this._data = SingleModelEditStackData.create(model, beforeCursorState);
 	}
 
-	public setModel(model: ITextModel): void {
+	public setModel(model: ITextModel | URI): void {
 		this.model = model;
 	}
 
@@ -178,6 +181,10 @@ export class SingleModelEditStackElement implements IResourceUndoRedoElement {
 	}
 
 	public undo(): void {
+		if (URI.isUri(this.model)) {
+			// don't have a model
+			throw new Error(`Invalid SingleModelEditStackElement`);
+		}
 		if (this._data instanceof SingleModelEditStackData) {
 			this._data = this._data.serialize();
 		}
@@ -186,6 +193,10 @@ export class SingleModelEditStackElement implements IResourceUndoRedoElement {
 	}
 
 	public redo(): void {
+		if (URI.isUri(this.model)) {
+			// don't have a model
+			throw new Error(`Invalid SingleModelEditStackElement`);
+		}
 		if (this._data instanceof SingleModelEditStackData) {
 			this._data = this._data.serialize();
 		}
@@ -211,7 +222,7 @@ export class MultiModelEditStackElement implements IWorkspaceUndoRedoElement {
 	private readonly _editStackElementsMap: Map<string, SingleModelEditStackElement>;
 
 	public get resources(): readonly URI[] {
-		return this._editStackElementsArr.map(editStackElement => editStackElement.model.uri);
+		return this._editStackElementsArr.map(editStackElement => editStackElement.resource);
 	}
 
 	constructor(
@@ -223,13 +234,13 @@ export class MultiModelEditStackElement implements IWorkspaceUndoRedoElement {
 		this._editStackElementsArr = editStackElements.slice(0);
 		this._editStackElementsMap = new Map<string, SingleModelEditStackElement>();
 		for (const editStackElement of this._editStackElementsArr) {
-			const key = uriGetComparisonKey(editStackElement.model.uri);
+			const key = uriGetComparisonKey(editStackElement.resource);
 			this._editStackElementsMap.set(key, editStackElement);
 		}
 	}
 
-	public setModel(model: ITextModel): void {
-		const key = uriGetComparisonKey(model.uri);
+	public setModel(model: ITextModel | URI): void {
+		const key = uriGetComparisonKey(URI.isUri(model) ? model : model.uri);
 		if (this._editStackElementsMap.has(key)) {
 			this._editStackElementsMap.get(key)!.setModel(model);
 		}
diff --git a/src/vs/editor/common/services/modelServiceImpl.ts b/src/vs/editor/common/services/modelServiceImpl.ts
@@ -477,9 +477,11 @@ export class ModelServiceImpl extends Disposable implements IModelService {
 				maintainUndoRedoStack = true;
 				for (const element of elements.past) {
 					heapSize += element.heapSize(resource);
+					element.setModel(resource); // remove reference from text buffer instance
 				}
 				for (const element of elements.future) {
 					heapSize += element.heapSize(resource);
+					element.setModel(resource); // remove reference from text buffer instance
 				}
 			} else {
 				maintainUndoRedoStack = false;

Original file line number	Diff line number	Diff line change
`@@ -137,14 +137,17 @@ class SingleModelEditStackData {`
`137`	`137`
`138`	`138`	`export class SingleModelEditStackElement implements IResourceUndoRedoElement {`
`139`	`139`
`140`		`- public model: ITextModel;`
	`140`	`+ public model: ITextModel \| URI;`
`141`	`141`	`private _data: SingleModelEditStackData \| ArrayBuffer;`
`142`	`142`
`143`	`143`	`public get type(): UndoRedoElementType.Resource {`
`144`	`144`	`return UndoRedoElementType.Resource;`
`145`	`145`	`}`
`146`	`146`
`147`	`147`	`public get resource(): URI {`
	`148`	`+ if (URI.isUri(this.model)) {`
	`149`	`+ return this.model;`
	`150`	`+ }`
`148`	`151`	`return this.model.uri;`
`149`	`152`	`}`
`150`	`153`
`@@ -157,7 +160,7 @@ export class SingleModelEditStackElement implements IResourceUndoRedoElement {`
`157`	`160`	`this._data = SingleModelEditStackData.create(model, beforeCursorState);`
`158`	`161`	`}`
`159`	`162`
`160`		`- public setModel(model: ITextModel): void {`
	`163`	`+ public setModel(model: ITextModel \| URI): void {`
`161`	`164`	`this.model = model;`
`162`	`165`	`}`
`163`	`166`
`@@ -178,6 +181,10 @@ export class SingleModelEditStackElement implements IResourceUndoRedoElement {`
`178`	`181`	`}`
`179`	`182`
`180`	`183`	`public undo(): void {`
	`184`	`+ if (URI.isUri(this.model)) {`
	`185`	`+ // don't have a model`
	`186`	+ throw new Error(`Invalid SingleModelEditStackElement`);
	`187`	`+ }`
`181`	`188`	`if (this._data instanceof SingleModelEditStackData) {`
`182`	`189`	`this._data = this._data.serialize();`
`183`	`190`	`}`
`@@ -186,6 +193,10 @@ export class SingleModelEditStackElement implements IResourceUndoRedoElement {`
`186`	`193`	`}`
`187`	`194`
`188`	`195`	`public redo(): void {`
	`196`	`+ if (URI.isUri(this.model)) {`
	`197`	`+ // don't have a model`
	`198`	+ throw new Error(`Invalid SingleModelEditStackElement`);
	`199`	`+ }`
`189`	`200`	`if (this._data instanceof SingleModelEditStackData) {`
`190`	`201`	`this._data = this._data.serialize();`
`191`	`202`	`}`
`@@ -211,7 +222,7 @@ export class MultiModelEditStackElement implements IWorkspaceUndoRedoElement {`
`211`	`222`	`private readonly _editStackElementsMap: Map<string, SingleModelEditStackElement>;`
`212`	`223`
`213`	`224`	`public get resources(): readonly URI[] {`
`214`		`- return this._editStackElementsArr.map(editStackElement => editStackElement.model.uri);`
	`225`	`+ return this._editStackElementsArr.map(editStackElement => editStackElement.resource);`
`215`	`226`	`}`
`216`	`227`
`217`	`228`	`constructor(`
`@@ -223,13 +234,13 @@ export class MultiModelEditStackElement implements IWorkspaceUndoRedoElement {`
`223`	`234`	`this._editStackElementsArr = editStackElements.slice(0);`
`224`	`235`	`this._editStackElementsMap = new Map<string, SingleModelEditStackElement>();`
`225`	`236`	`for (const editStackElement of this._editStackElementsArr) {`
`226`		`- const key = uriGetComparisonKey(editStackElement.model.uri);`
	`237`	`+ const key = uriGetComparisonKey(editStackElement.resource);`
`227`	`238`	`this._editStackElementsMap.set(key, editStackElement);`
`228`	`239`	`}`
`229`	`240`	`}`
`230`	`241`
`231`		`- public setModel(model: ITextModel): void {`
`232`		`- const key = uriGetComparisonKey(model.uri);`
	`242`	`+ public setModel(model: ITextModel \| URI): void {`
	`243`	`+ const key = uriGetComparisonKey(URI.isUri(model) ? model : model.uri);`
`233`	`244`	`if (this._editStackElementsMap.has(key)) {`
`234`	`245`	`this._editStackElementsMap.get(key)!.setModel(model);`
`235`	`246`	`}`
Original file line number	Diff line number	Diff line change
`@@ -477,9 +477,11 @@ export class ModelServiceImpl extends Disposable implements IModelService {`
`477`	`477`	`maintainUndoRedoStack = true;`
`478`	`478`	`for (const element of elements.past) {`
`479`	`479`	`heapSize += element.heapSize(resource);`
	`480`	`+ element.setModel(resource); // remove reference from text buffer instance`
`480`	`481`	`}`
`481`	`482`	`for (const element of elements.future) {`
`482`	`483`	`heapSize += element.heapSize(resource);`
	`484`	`+ element.setModel(resource); // remove reference from text buffer instance`
`483`	`485`	`}`
`484`	`486`	`} else {`
`485`	`487`	`maintainUndoRedoStack = false;`