Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes to flaky Installer Prompt tests #9358

Merged
merged 3 commits into from
Mar 21, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fixes
  • Loading branch information
DonJayamanne committed Mar 21, 2022
commit 043a74e7ad5a9b8e7108f7546ff33b3c2234d57f
61 changes: 33 additions & 28 deletions src/kernels/helpers.ts
Original file line number Diff line number Diff line change
@@ -2003,6 +2003,38 @@ const connections = new WeakMap<
}
>();

async function verifyKernelState(
serviceContainer: IServiceContainer,
resource: Resource,
notebook: NotebookDocument,
options: IDisplayOptions = new DisplayOptions(false),
promise: Promise<{
kernel: IKernel;
controller: VSCodeNotebookController;
deadKernelAction?: 'deadKernelWasRestarted' | 'deadKernelWasNoRestarted';
}>
): Promise<IKernel> {
const { kernel, controller, deadKernelAction } = await promise;
// Before returning, but without disposing the kernel, double check it's still valid
// If a restart didn't happen, then we can't connect. Throw an error.
// Do this outside of the loop so that subsequent calls will still ask because the kernel isn't disposed
if (kernel.status === 'dead' || (kernel.status === 'terminating' && !kernel.disposed && !kernel.disposing)) {
// If the kernel is dead, then remove the cached promise, & try to get the kernel again.
// At that point, it will get restarted.
if (connections.get(notebook)?.kernel.promise === promise) {
connections.delete(notebook);
}
if (deadKernelAction === 'deadKernelWasNoRestarted') {
throw new KernelDeadError(kernel.kernelConnectionMetadata);
} else if (deadKernelAction === 'deadKernelWasRestarted') {
return kernel;
}
// Kernel is dead and we didn't prompt the user to restart it, hence re-run the code that will prompt the user for a restart.
return wrapKernelMethod(controller, 'start', serviceContainer, resource, notebook, options);
}
return kernel;
}

export async function wrapKernelMethod(
initialController: VSCodeNotebookController,
initialContext: 'start' | 'interrupt' | 'restart',
@@ -2025,37 +2057,10 @@ export async function wrapKernelMethod(
connections.delete(notebook);
currentPromise = undefined;
}
const verifyKernelState = async (
promise: Promise<{
kernel: IKernel;
controller: VSCodeNotebookController;
deadKernelAction?: 'deadKernelWasRestarted' | 'deadKernelWasNoRestarted';
}>
): Promise<IKernel> => {
const { kernel, controller, deadKernelAction } = await promise;
// Before returning, but without disposing the kernel, double check it's still valid
// If a restart didn't happen, then we can't connect. Throw an error.
// Do this outside of the loop so that subsequent calls will still ask because the kernel isn't disposed
if (kernel.status === 'dead' || (kernel.status === 'terminating' && !kernel.disposed && !kernel.disposing)) {
// If the kernel is dead, then remove the cached promise, & try to get the kernel again.
// At that point, it will get restarted.
if (connections.get(notebook)?.kernel.promise === promise) {
connections.delete(notebook);
}
if (deadKernelAction === 'deadKernelWasNoRestarted') {
throw new KernelDeadError(kernel.kernelConnectionMetadata);
} else if (deadKernelAction === 'deadKernelWasRestarted') {
return kernel;
}
// Kernel is dead and we didn't prompt the user to restart it, hence re-run the code that will prompt the user for a restart.
return wrapKernelMethod(controller, 'start', serviceContainer, resource, notebook, options);
}
return kernel;
};

// Wrap the kernel method again to interrupt/restart this kernel.
if (currentPromise && initialContext !== 'restart' && initialContext !== 'interrupt') {
return verifyKernelState(currentPromise.kernel.promise);
return verifyKernelState(serviceContainer, resource, notebook, options, currentPromise.kernel.promise);
}

const promise = wrapKernelMethodImpl(