From 82a5ecd2d8926105ba7e76457b0638753ec711aa Mon Sep 17 00:00:00 2001 From: Kevin Eger Date: Fri, 17 Oct 2025 18:30:18 +0000 Subject: [PATCH 1/2] fix: dispose a torn down NB controller's kernel Fixes https://github.com/microsoft/vscode-jupyter/issues/17094 It's my first time looking at any of this code and the lifecycle of everything is a little challenging to discover at a glance. This change passed unit tests and manually verifying, it solves the issue. I'll lay out the problem and solution independently below to make reviewing easier. If a server/kernel is removed, then another was created for the active notebook editor, cell executions against the new kernel would fail. The issue with clear repro steps and the corresponding logs: https://github.com/microsoft/vscode-jupyter/issues/17094. The root cause is improper resource cleanup during the disposal of a `VSCodeNotebookController`. When a `JupyterServerProvider` is removed, the extension correctly disposes of the associated controllers. However, the `dispose` method for the controller did not explicitly dispose of the underlying `IKernel` instance that it had created and managed. This left the kernel in an orphaned or "zombie" state. When a new server and controller were subsequently created for the same notebook, it'd try to use the old, still-lingering kernel, which was already in a started state, leading to the "Cannot call start again" error. The fix enhances the dispose method within the `VSCodeNotebookController`. The new logic ensures that when a controller is disposed, it actively cleans up any kernels it was responsible for. It iterates through all currently open NB docs and siposes any kernels who's connection metadata IDs match. From 45a27042d754995d205a90c1ac8aaea9d59e2239 Mon Sep 17 00:00:00 2001 From: Kevin Eger Date: Wed, 22 Oct 2025 00:50:24 +0000 Subject: [PATCH 2/2] fix: dispose kernels associated with a controller This change modifies the `VSCodeNotebookController`'s `dispose` method to ensure that when a controller is disposed, it disposes the kernels that are directly associated with that specific controller instance. --- .../controllers/vscodeNotebookController.ts | 11 +++ .../vscodeNotebookController.unit.test.ts | 68 +++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/src/notebooks/controllers/vscodeNotebookController.ts b/src/notebooks/controllers/vscodeNotebookController.ts index 4bae4114f29..300941523fe 100644 --- a/src/notebooks/controllers/vscodeNotebookController.ts +++ b/src/notebooks/controllers/vscodeNotebookController.ts @@ -334,6 +334,17 @@ export class VSCodeNotebookController implements Disposable, IVSCodeNotebookCont } called from ${new Error('').stack}` ); this.isDisposed = true; + // Dispose kernels associated with this controller. + workspace.notebookDocuments + .filter((doc) => this.associatedDocuments.has(doc)) + .forEach((doc) => { + const kernel = this.kernelProvider.get(doc); + // If the kernel is associated with this document, and the controller is being disposed, + // then the kernel for this document must be disposed. + if (kernel) { + kernel.dispose().catch(noop); + } + }); this._onNotebookControllerSelectionChanged.dispose(); this._onConnecting.dispose(); this.controller.dispose(); diff --git a/src/notebooks/controllers/vscodeNotebookController.unit.test.ts b/src/notebooks/controllers/vscodeNotebookController.unit.test.ts index cd835e0e787..aef3287a665 100644 --- a/src/notebooks/controllers/vscodeNotebookController.unit.test.ts +++ b/src/notebooks/controllers/vscodeNotebookController.unit.test.ts @@ -278,6 +278,74 @@ suite(`Notebook Controller`, function () { verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).once(); }); + suite('dispose', () => { + let controllerInstance: VSCodeNotebookController; + setup(() => { + controllerInstance = new VSCodeNotebookController( + instance(kernelConnection), + '1', + 'jupyter-notebook', + instance(kernelProvider), + instance(context), + disposables, + instance(languageService), + instance(configService), + instance(extensionChecker), + instance(serviceContainer), + displayDataProvider + ); + notebook = new TestNotebookDocument(undefined, 'jupyter-notebook'); + }); + test('Disposes the kernel of the associated notebook', async function () { + const kernel1 = mock(); + const kernel2 = mock(); + when(kernel1.dispose()).thenResolve(); + when(kernel2.dispose()).thenResolve(); + const notebook1 = new TestNotebookDocument(); + const notebook2 = new TestNotebookDocument(); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook1, notebook2]); + when(kernelProvider.get(notebook1)).thenReturn(instance(kernel1)); + when(kernelProvider.get(notebook2)).thenReturn(instance(kernel2)); + when(kernel1.kernelConnectionMetadata).thenReturn(instance(kernelConnection)); + when(kernel2.kernelConnectionMetadata).thenReturn(instance(kernelConnection)); + + // Associate the controller with notebook1 + onDidChangeSelectedNotebooks.fire({ notebook: notebook1, selected: true }); + await clock.runAllAsync(); + + // Dispose the controller + controllerInstance.dispose(); + + // Verify that only the kernel associated with notebook1 is disposed + verify(kernel1.dispose()).once(); + verify(kernel2.dispose()).never(); + }); + test('Disposes all kernels associated with this controller', async function () { + const kernel1 = mock(); + const kernel2 = mock(); + when(kernel1.dispose()).thenResolve(); + when(kernel2.dispose()).thenResolve(); + const notebook1 = new TestNotebookDocument(); + const notebook2 = new TestNotebookDocument(); + when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook1, notebook2]); + when(kernelProvider.get(notebook1)).thenReturn(instance(kernel1)); + when(kernelProvider.get(notebook2)).thenReturn(instance(kernel2)); + when(kernel1.kernelConnectionMetadata).thenReturn(instance(kernelConnection)); + when(kernel2.kernelConnectionMetadata).thenReturn(instance(kernelConnection)); + + // Associate the controller with both notebooks + onDidChangeSelectedNotebooks.fire({ notebook: notebook1, selected: true }); + onDidChangeSelectedNotebooks.fire({ notebook: notebook2, selected: true }); + await clock.runAllAsync(); + + // Dispose the controller + controllerInstance.dispose(); + + // Verify that both kernels are disposed + verify(kernel1.dispose()).once(); + verify(kernel2.dispose()).once(); + }); + }); suite('Unsupported Python Versions', () => { let disposables: IDisposable[] = []; let environments: PythonExtension['environments'];