fix: memory leak in search editor (#331014)

* fix: memory leak in search editor

* fix: guard search editor model disposal

* test: check search editor model suite for disposable leaks

---------

Co-authored-by: Dmitriy Vasyura <dmitriv@microsoft.com>

src/vs/workbench/contrib/searchEditor/browser/searchEditor.ts

@@ -93,6 +93,7 @@ export class SearchEditor extends AbstractTextCodeEditor<SearchEditorViewState>
 	private searchOperation: LongRunningOperation;
 	private searchHistoryDelayer: Delayer<void>;
 	private readonly messageDisposables: DisposableStore;
+	private readonly inputDisposables = this._register(new DisposableStore());
 	private container: HTMLElement;
 	private searchModel: SearchModelImpl;
 	private ongoingOperations: number = 0;
@@ -711,6 +712,8 @@ export class SearchEditor extends AbstractTextCodeEditor<SearchEditorViewState>
 		if (token.isCancellationRequested) {
 			return;
 		}
+		// A new input can replace the current one without clearInput being called first.
+		this.inputDisposables.clear();
 
 		const { configurationModel, resultsModel } = await newInput.resolveModels();
 		if (token.isCancellationRequested) { return; }
@@ -722,7 +725,7 @@ export class SearchEditor extends AbstractTextCodeEditor<SearchEditorViewState>
 
 		this.setSearchConfig(configurationModel.config);
 
-		this._register(configurationModel.onConfigDidUpdate(newConfig => {
+		this.inputDisposables.add(configurationModel.onConfigDidUpdate(newConfig => {
 			if (newConfig !== this.priorConfig) {
 				this.pauseSearching = true;
 				this.setSearchConfig(newConfig);
@@ -746,6 +749,12 @@ export class SearchEditor extends AbstractTextCodeEditor<SearchEditorViewState>
 		}
 	}
 
+	override clearInput(): void {
+		// An input can be cleared without another input being set.
+		this.inputDisposables.clear();
+		super.clearInput();
+	}
+
 	private toggleIncludesExcludes(_shouldShow?: boolean): void {
 		const cls = 'expanded';
 		const shouldShow = _shouldShow ?? !this.includesExcludesContainer.classList.contains(cls);

src/vs/workbench/contrib/searchEditor/browser/searchEditorInput.ts

@@ -242,6 +242,7 @@ export class SearchEditorInput extends EditorInput {
 	}
 
 	override dispose() {
+		this.model.dispose();
 		this.modelService.destroyModel(this.modelUri);
 		super.dispose();
 	}

src/vs/workbench/contrib/searchEditor/browser/searchEditorModel.ts

@@ -19,6 +19,10 @@ import { SEARCH_RESULT_LANGUAGE_ID } from '../../../services/search/common/searc
 
 export type SearchEditorData = { resultsModel: ITextModel; configurationModel: SearchConfigurationModel };
 
+interface ISearchEditorModelReference {
+	resolve(): Promise<SearchEditorData>;
+}
+
 export class SearchConfigurationModel {
 	private _onConfigDidUpdate = new Emitter<SearchConfiguration>();
 	public readonly onConfigDidUpdate = this._onConfigDidUpdate.event;
@@ -28,17 +32,27 @@ export class SearchConfigurationModel {
 }
 
 export class SearchEditorModel {
+	private readonly modelReference: ISearchEditorModelReference;
+
 	constructor(
-		private resource: URI,
-	) { }
+		private readonly resource: URI,
+	) {
+		this.modelReference = assertReturnsDefined(searchEditorModelFactory.models.get(this.resource));
+	}
 
 	async resolve(): Promise<SearchEditorData> {
-		return assertReturnsDefined(searchEditorModelFactory.models.get(this.resource)).resolve();
+		return this.modelReference.resolve();
+	}
+
+	dispose(): void {
+		if (searchEditorModelFactory.models.get(this.resource) === this.modelReference) {
+			searchEditorModelFactory.models.delete(this.resource);
+		}
 	}
 }
 
 class SearchEditorModelFactory {
-	models = new ResourceMap<{ resolve: () => Promise<SearchEditorData> }>();
+	models = new ResourceMap<ISearchEditorModelReference>();
 
 	constructor() { }
 

src/vs/workbench/contrib/searchEditor/test/browser/searchEditorModel.test.ts

@@ -0,0 +1,29 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import assert from 'assert';
+import { URI } from '../../../../../base/common/uri.js';
+import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
+import { SearchEditorModel, searchEditorModelFactory } from '../../browser/searchEditorModel.js';
+
+suite('SearchEditorModel', () => {
+	ensureNoDisposablesAreLeakedInTestSuite();
+
+	const resource = URI.from({ scheme: 'search-editor', fragment: 'test' });
+
+	teardown(() => searchEditorModelFactory.models.delete(resource));
+
+	test('does not delete a replacement model when disposed', () => {
+		const modelReference = { resolve: () => Promise.reject(new Error('Not implemented')) };
+		searchEditorModelFactory.models.set(resource, modelReference);
+		const model = new SearchEditorModel(resource);
+
+		const replacementModelReference = { resolve: () => Promise.reject(new Error('Not implemented')) };
+		searchEditorModelFactory.models.set(resource, replacementModelReference);
+		model.dispose();
+
+		assert.strictEqual(searchEditorModelFactory.models.get(resource), replacementModelReference);
+	});
+});