diff --git a/news/2 Fixes/13258.md b/news/2 Fixes/13258.md new file mode 100644 index 000000000000..f0dafc1af18c --- /dev/null +++ b/news/2 Fixes/13258.md @@ -0,0 +1 @@ +Prevent daemon from trying to prewarm an execution service. \ No newline at end of file diff --git a/src/client/common/process/pythonExecutionFactory.ts b/src/client/common/process/pythonExecutionFactory.ts index 81a1bb89d968..04e8b09cb9e0 100644 --- a/src/client/common/process/pythonExecutionFactory.ts +++ b/src/client/common/process/pythonExecutionFactory.ts @@ -76,7 +76,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory { public async createDaemon( options: DaemonExecutionFactoryCreationOptions - ): Promise { + ): Promise { const pythonPath = options.pythonPath ? options.pythonPath : this.configService.getSettings(options.resource).pythonPath; @@ -93,7 +93,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory { }); // No daemon support in Python 2.7 or during shutdown if (!interpreterService || (interpreter?.version && interpreter.version.major < 3)) { - return (activatedProcPromise! as unknown) as T; + return activatedProcPromise; } // Ensure we do not start multiple daemons for the same interpreter. diff --git a/src/client/common/process/types.ts b/src/client/common/process/types.ts index 3a1c8b75e3f0..9e104cfeddbe 100644 --- a/src/client/common/process/types.ts +++ b/src/client/common/process/types.ts @@ -154,7 +154,7 @@ export interface IPythonExecutionFactory { */ createDaemon( options: DaemonExecutionFactoryCreationOptions - ): Promise; + ): Promise; createActivatedEnvironment(options: ExecutionFactoryCreateWithEnvironmentOptions): Promise; createCondaExecutionService( pythonPath: string, diff --git a/src/client/datascience/kernel-launcher/kernelDaemonPool.ts b/src/client/datascience/kernel-launcher/kernelDaemonPool.ts index cba5c6d1d093..fab00cbbd9d5 100644 --- a/src/client/datascience/kernel-launcher/kernelDaemonPool.ts +++ b/src/client/datascience/kernel-launcher/kernelDaemonPool.ts @@ -7,7 +7,7 @@ import { inject, injectable } from 'inversify'; import { IWorkspaceService } from '../../common/application/types'; import { traceError } from '../../common/logger'; -import { IPythonExecutionFactory } from '../../common/process/types'; +import { IPythonExecutionFactory, IPythonExecutionService } from '../../common/process/types'; import { IDisposable, Resource } from '../../common/types'; import { noop } from '../../common/utils/misc'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; @@ -23,7 +23,7 @@ type IKernelDaemonInfo = { workspaceResource: Resource; workspaceFolderIdentifier: string; interpreterPath: string; - daemon: Promise; + daemon: Promise; }; @injectable() @@ -68,7 +68,7 @@ export class KernelDaemonPool implements IDisposable { resource: Resource, kernelSpec: IJupyterKernelSpec, interpreter?: PythonInterpreter - ): Promise { + ): Promise { const pythonPath = interpreter?.path || kernelSpec.argv[0]; // If we have environment variables in the kernel.json, then its not we support. // Cuz there's no way to know before hand what kernelspec can be used, hence no way to know what envs are required. @@ -104,14 +104,26 @@ export class KernelDaemonPool implements IDisposable { dedicated: true, resource }); - daemon.then((d) => this.disposables.push(d)).catch(noop); + daemon + .then((d) => { + if ('dispose' in d) { + this.disposables.push(d); + } + }) + .catch(noop); return daemon; } private async onDidEnvironmentVariablesChange(affectedResoruce: Resource) { const workspaceFolderIdentifier = this.workspaceService.getWorkspaceFolderIdentifier(affectedResoruce); this.daemonPool = this.daemonPool.filter((item) => { if (item.workspaceFolderIdentifier === workspaceFolderIdentifier) { - item.daemon.then((d) => d.dispose()).catch(noop); + item.daemon + .then((d) => { + if ('dispose' in d) { + d.dispose(); + } + }) + .catch(noop); return false; } return true; @@ -132,7 +144,16 @@ export class KernelDaemonPool implements IDisposable { const daemon = this.createDaemon(resource, interpreter.path); // Once a daemon is created ensure we pre-warm it (will load ipykernel and start the kernker process waiting to start the actual kernel code). // I.e. we'll start python process thats the kernel, but will not start the kernel module (`python -m ipykernel`). - daemon.then((d) => d.preWarm()).catch(traceError.bind(`Failed to prewarm kernel daemon ${interpreter.path}`)); + daemon + .then((d) => { + // Prewarm if we support prewarming + if ('preWarm' in d) { + d.preWarm().catch(traceError.bind(`Failed to prewarm kernel daemon ${interpreter.path}`)); + } + }) + .catch((e) => { + traceError(`Failed to load deamon: ${e}`); + }); this.daemonPool.push({ daemon, interpreterPath: interpreter.path, @@ -172,7 +193,13 @@ export class KernelDaemonPool implements IDisposable { this.daemonPool = this.daemonPool.filter((item) => { const interpreterForWorkspace = currentInterpreterInEachWorksapce.get(item.key); if (!interpreterForWorkspace || !this.fs.areLocalPathsSame(interpreterForWorkspace, item.interpreterPath)) { - item.daemon.then((d) => d.dispose()).catch(noop); + item.daemon + .then((d) => { + if ('dispose' in d) { + d.dispose(); + } + }) + .catch(noop); return false; } diff --git a/src/client/datascience/kernel-launcher/kernelLauncherDaemon.ts b/src/client/datascience/kernel-launcher/kernelLauncherDaemon.ts index b29601c17990..a34ee3066830 100644 --- a/src/client/datascience/kernel-launcher/kernelLauncherDaemon.ts +++ b/src/client/datascience/kernel-launcher/kernelLauncherDaemon.ts @@ -6,7 +6,7 @@ import { ChildProcess } from 'child_process'; import { inject, injectable } from 'inversify'; import { IDisposable } from 'monaco-editor'; -import { IPythonExecutionService, ObservableExecutionResult } from '../../common/process/types'; +import { ObservableExecutionResult } from '../../common/process/types'; import { Resource } from '../../common/types'; import { noop } from '../../common/utils/misc'; import { PythonInterpreter } from '../../pythonEnvironments/info'; @@ -47,14 +47,9 @@ export class PythonKernelLauncherDaemon implements IDisposable { // The daemon pool can return back a non-IPythonKernelDaemon if daemon service is not supported or for Python 2. // Use a check for the daemon.start function here before we call it. - if (!daemon.start) { + if (!('start' in daemon)) { // If we don't have a KernelDaemon here then we have an execution service and should use that to launch - // Typing is a bit funk here, as createDaemon can return an execution service instead of the requested - // daemon class - // tslint:disable-next-line:no-any - const executionService = (daemon as any) as IPythonExecutionService; - - const observableOutput = executionService.execModuleObservable(moduleName, moduleArgs, { + const observableOutput = daemon.execModuleObservable(moduleName, moduleArgs, { env, cwd: workingDirectory }); diff --git a/src/test/datascience/kernel-launcher/kernelDaemonPool.unit.test.ts b/src/test/datascience/kernel-launcher/kernelDaemonPool.unit.test.ts index 85c5bedff3b3..7276c21043c4 100644 --- a/src/test/datascience/kernel-launcher/kernelDaemonPool.unit.test.ts +++ b/src/test/datascience/kernel-launcher/kernelDaemonPool.unit.test.ts @@ -66,6 +66,9 @@ suite('DataScience - Kernel Daemon Pool', () => { when(daemon1.preWarm()).thenResolve(); when(daemon2.preWarm()).thenResolve(); when(daemon3.preWarm()).thenResolve(); + when(daemon1.dispose()).thenResolve(); + when(daemon2.dispose()).thenResolve(); + when(daemon3.dispose()).thenResolve(); when(envVars.onDidEnvironmentVariablesChange).thenReturn(didEnvVarsChange.event); when(interpeterService.onDidChangeInterpreter).thenReturn(didChangeInterpreter.event); diff --git a/src/test/datascience/kernel-launcher/kernelLauncherDaemon.unit.test.ts b/src/test/datascience/kernel-launcher/kernelLauncherDaemon.unit.test.ts index ff6cf752f6b4..aff8ea373288 100644 --- a/src/test/datascience/kernel-launcher/kernelLauncherDaemon.unit.test.ts +++ b/src/test/datascience/kernel-launcher/kernelLauncherDaemon.unit.test.ts @@ -66,8 +66,6 @@ suite('DataScience - Kernel Launcher Daemon', () => { when( executionService.execModuleObservable('ipkernel_launcher', deepEqual(['-f', 'file.json']), anything()) ).thenReturn(instance(observableOutputForDaemon)); - // Make sure that it doesn't have a start function. Normally the proxy will pretend to have a start function, which we are checking for non-existance - when((executionService as any).start).thenReturn(false); // Else ts-mockit doesn't allow us to return an instance of a mock as a return value from an async function. (instance(executionService) as any).then = undefined; when(daemonPool.get(anything(), anything(), anything())).thenResolve(instance(executionService) as any);