Skip to content

Commit 45a2704

Browse files
committed
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.
1 parent 82a5ecd commit 45a2704

File tree

2 files changed

+79
-0
lines changed

2 files changed

+79
-0
lines changed

src/notebooks/controllers/vscodeNotebookController.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,17 @@ export class VSCodeNotebookController implements Disposable, IVSCodeNotebookCont
334334
} called from ${new Error('').stack}`
335335
);
336336
this.isDisposed = true;
337+
// Dispose kernels associated with this controller.
338+
workspace.notebookDocuments
339+
.filter((doc) => this.associatedDocuments.has(doc))
340+
.forEach((doc) => {
341+
const kernel = this.kernelProvider.get(doc);
342+
// If the kernel is associated with this document, and the controller is being disposed,
343+
// then the kernel for this document must be disposed.
344+
if (kernel) {
345+
kernel.dispose().catch(noop);
346+
}
347+
});
337348
this._onNotebookControllerSelectionChanged.dispose();
338349
this._onConnecting.dispose();
339350
this.controller.dispose();

src/notebooks/controllers/vscodeNotebookController.unit.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,74 @@ suite(`Notebook Controller`, function () {
278278

279279
verify(mockedVSCodeNamespaces.workspace.applyEdit(anything())).once();
280280
});
281+
suite('dispose', () => {
282+
let controllerInstance: VSCodeNotebookController;
283+
setup(() => {
284+
controllerInstance = new VSCodeNotebookController(
285+
instance(kernelConnection),
286+
'1',
287+
'jupyter-notebook',
288+
instance(kernelProvider),
289+
instance(context),
290+
disposables,
291+
instance(languageService),
292+
instance(configService),
293+
instance(extensionChecker),
294+
instance(serviceContainer),
295+
displayDataProvider
296+
);
297+
notebook = new TestNotebookDocument(undefined, 'jupyter-notebook');
298+
});
299+
test('Disposes the kernel of the associated notebook', async function () {
300+
const kernel1 = mock<IKernel>();
301+
const kernel2 = mock<IKernel>();
302+
when(kernel1.dispose()).thenResolve();
303+
when(kernel2.dispose()).thenResolve();
304+
const notebook1 = new TestNotebookDocument();
305+
const notebook2 = new TestNotebookDocument();
306+
when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook1, notebook2]);
307+
when(kernelProvider.get(notebook1)).thenReturn(instance(kernel1));
308+
when(kernelProvider.get(notebook2)).thenReturn(instance(kernel2));
309+
when(kernel1.kernelConnectionMetadata).thenReturn(instance(kernelConnection));
310+
when(kernel2.kernelConnectionMetadata).thenReturn(instance(kernelConnection));
311+
312+
// Associate the controller with notebook1
313+
onDidChangeSelectedNotebooks.fire({ notebook: notebook1, selected: true });
314+
await clock.runAllAsync();
315+
316+
// Dispose the controller
317+
controllerInstance.dispose();
318+
319+
// Verify that only the kernel associated with notebook1 is disposed
320+
verify(kernel1.dispose()).once();
321+
verify(kernel2.dispose()).never();
322+
});
323+
test('Disposes all kernels associated with this controller', async function () {
324+
const kernel1 = mock<IKernel>();
325+
const kernel2 = mock<IKernel>();
326+
when(kernel1.dispose()).thenResolve();
327+
when(kernel2.dispose()).thenResolve();
328+
const notebook1 = new TestNotebookDocument();
329+
const notebook2 = new TestNotebookDocument();
330+
when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([notebook1, notebook2]);
331+
when(kernelProvider.get(notebook1)).thenReturn(instance(kernel1));
332+
when(kernelProvider.get(notebook2)).thenReturn(instance(kernel2));
333+
when(kernel1.kernelConnectionMetadata).thenReturn(instance(kernelConnection));
334+
when(kernel2.kernelConnectionMetadata).thenReturn(instance(kernelConnection));
335+
336+
// Associate the controller with both notebooks
337+
onDidChangeSelectedNotebooks.fire({ notebook: notebook1, selected: true });
338+
onDidChangeSelectedNotebooks.fire({ notebook: notebook2, selected: true });
339+
await clock.runAllAsync();
340+
341+
// Dispose the controller
342+
controllerInstance.dispose();
343+
344+
// Verify that both kernels are disposed
345+
verify(kernel1.dispose()).once();
346+
verify(kernel2.dispose()).once();
347+
});
348+
});
281349
suite('Unsupported Python Versions', () => {
282350
let disposables: IDisposable[] = [];
283351
let environments: PythonExtension['environments'];

0 commit comments

Comments
 (0)