Merge pull request microsoft#225423 from microsoft/rebornix/blushing-…

…muskox Fix microsoft#225388. Dispose quick pick and its event listenrs properly.
hoodiiens-anvil · Aug 19, 2024 · dd7e51a · dd7e51a
1 parent bafd430
commit dd7e51a
Showing 1 changed file with 22 additions and 16 deletions.
diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelQuickPickStrategy.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelQuickPickStrategy.ts
@@ -136,11 +136,15 @@ abstract class KernelPickerStrategyBase implements IKernelPickerStrategy {
 			return true;
 		}
 
-		const quickPick = this._quickInputService.createQuickPick<KernelQuickPickItem>({ useSeparators: true });
+
+		const localDisposableStore = new DisposableStore();
+		const quickPick = localDisposableStore.add(this._quickInputService.createQuickPick<KernelQuickPickItem>({ useSeparators: true }));
 		const quickPickItems = this._getKernelPickerQuickPickItems(notebook, matchResult, this._notebookKernelService, scopedContextKeyService);
 
 		if (quickPickItems.length === 1 && supportAutoRun(quickPickItems[0]) && !skipAutoRun) {
-			return await this._handleQuickPick(editor, quickPickItems[0], quickPickItems as KernelQuickPickItem[]);
+			const picked = await this._handleQuickPick(editor, quickPickItems[0], quickPickItems as KernelQuickPickItem[]);
+			localDisposableStore.dispose();
+			return picked;
 		}
 
 		quickPick.items = quickPickItems;
@@ -201,7 +205,7 @@ abstract class KernelPickerStrategyBase implements IKernelPickerStrategy {
 		}, this);
 
 		const pick = await new Promise<{ selected: KernelQuickPickItem | undefined; items: KernelQuickPickItem[] }>((resolve, reject) => {
-			quickPick.onDidAccept(() => {
+			localDisposableStore.add(quickPick.onDidAccept(() => {
 				const item = quickPick.selectedItems[0];
 				if (item) {
 					resolve({ selected: item, items: quickPick.items as KernelQuickPickItem[] });
@@ -210,17 +214,19 @@ abstract class KernelPickerStrategyBase implements IKernelPickerStrategy {
 				}
 
 				quickPick.hide();
-			});
+			}));
 
-			quickPick.onDidHide(() => {
+			localDisposableStore.add(quickPick.onDidHide(() => {
 				kernelDetectionTaskListener.dispose();
 				kernelChangeEventListener.dispose();
 				quickPick.dispose();
 				resolve({ selected: undefined, items: quickPick.items as KernelQuickPickItem[] });
-			});
+			}));
 			quickPick.show();
 		});
 
+		localDisposableStore.dispose();
+
 		if (pick.selected) {
 			return await this._handleQuickPick(editor, pick.selected, pick.items);
 		}
@@ -519,7 +525,7 @@ export class KernelPickerMRUStrategy extends KernelPickerStrategyBase {
 	private async displaySelectAnotherQuickPick(editor: IActiveNotebookEditor, kernelListEmpty: boolean): Promise<boolean> {
 		const notebook: NotebookTextModel = editor.textModel;
 		const disposables = new DisposableStore();
-		const quickPick = this._quickInputService.createQuickPick<KernelQuickPickItem>({ useSeparators: true });
+		const quickPick = disposables.add(this._quickInputService.createQuickPick<KernelQuickPickItem>({ useSeparators: true }));
 		const quickPickItem = await new Promise<KernelQuickPickItem | IQuickInputButton | undefined>(resolve => {
 			// select from kernel sources
 			quickPick.title = kernelListEmpty ? localize('select', "Select Kernel") : localize('selectAnotherKernel', "Select Another Kernel");
@@ -533,13 +539,12 @@ export class KernelPickerMRUStrategy extends KernelPickerStrategyBase {
 					resolve(button);
 				}
 			}));
-			quickPick.onDidTriggerItemButton(async (e) => {
-
+			disposables.add(quickPick.onDidTriggerItemButton(async (e) => {
 				if (isKernelSourceQuickPickItem(e.item) && e.item.documentation !== undefined) {
 					const uri = URI.isUri(e.item.documentation) ? URI.parse(e.item.documentation) : await this._commandService.executeCommand(e.item.documentation);
 					void this._openerService.open(uri, { openExternal: true });
 				}
-			});
+			}));
 			disposables.add(quickPick.onDidAccept(async () => {
 				resolve(quickPick.selectedItems[0]);
 			}));
@@ -699,24 +704,25 @@ export class KernelPickerMRUStrategy extends KernelPickerStrategyBase {
 
 	private async _selectOneKernel(notebook: NotebookTextModel, source: string, kernels: INotebookKernel[]) {
 		const quickPickItems: QuickPickInput<KernelPick>[] = kernels.map(kernel => toKernelQuickPick(kernel, undefined));
-		const quickPick = this._quickInputService.createQuickPick<KernelQuickPickItem>({ useSeparators: true });
+		const localDisposableStore = new DisposableStore();
+		const quickPick = localDisposableStore.add(this._quickInputService.createQuickPick<KernelQuickPickItem>({ useSeparators: true }));
 		quickPick.items = quickPickItems;
 		quickPick.canSelectMany = false;
 
 		quickPick.title = localize('selectKernelFromExtension', "Select Kernel from {0}", source);
 
-		quickPick.onDidAccept(async () => {
+		localDisposableStore.add(quickPick.onDidAccept(async () => {
 			if (quickPick.selectedItems && quickPick.selectedItems.length > 0 && isKernelPick(quickPick.selectedItems[0])) {
 				await this._selecteKernel(notebook, quickPick.selectedItems[0].kernel);
 			}
 
 			quickPick.hide();
 			quickPick.dispose();
-		});
+		}));
 
-		quickPick.onDidHide(() => {
-			quickPick.dispose();
-		});
+		localDisposableStore.add(quickPick.onDidHide(() => {
+			localDisposableStore.dispose();
+		}));
 
 		quickPick.show();
 	}