Skip to content

Commit 765c483

Browse files
authored
Fix disposed execution factory to not have binding failures. (microsoft#12431)
* Fix disposed execution factory to not have binding failures. * Fix linting tests
1 parent 834449f commit 765c483

File tree

2 files changed

+15
-12
lines changed

2 files changed

+15
-12
lines changed

src/client/common/process/pythonExecutionFactory.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
4242
private readonly daemonsPerPythonService = new Map<string, Promise<IPythonDaemonExecutionService>>();
4343
private readonly disposables: IDisposableRegistry;
4444
private readonly logger: IProcessLogger;
45+
private readonly fileSystem: IFileSystem;
4546
constructor(
4647
@inject(IServiceContainer) private serviceContainer: IServiceContainer,
4748
@inject(IEnvironmentActivationService) private readonly activationHelper: IEnvironmentActivationService,
@@ -54,19 +55,19 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
5455
// Acquire other objects here so that if we are called during dispose they are available.
5556
this.disposables = this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
5657
this.logger = this.serviceContainer.get<IProcessLogger>(IProcessLogger);
58+
this.fileSystem = this.serviceContainer.get<IFileSystem>(IFileSystem);
5759
}
5860
public async create(options: ExecutionFactoryCreationOptions): Promise<IPythonExecutionService> {
5961
const pythonPath = options.pythonPath
6062
? options.pythonPath
6163
: this.configService.getSettings(options.resource).pythonPath;
6264
const processService: IProcessService = await this.processServiceFactory.create(options.resource);
63-
const processLogger = this.serviceContainer.get<IProcessLogger>(IProcessLogger);
64-
processService.on('exec', processLogger.logProcess.bind(processLogger));
65+
processService.on('exec', this.logger.logProcess.bind(this.logger));
6566

6667
return createPythonService(
6768
pythonPath,
6869
processService,
69-
this.serviceContainer.get<IFileSystem>(IFileSystem),
70+
this.fileSystem,
7071
undefined,
7172
this.windowsStoreInterpreter.isWindowsStoreInterpreter(pythonPath)
7273
);
@@ -164,11 +165,10 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
164165
? options.interpreter.path
165166
: this.configService.getSettings(options.resource).pythonPath;
166167
const processService: IProcessService = new ProcessService(this.decoder, { ...envVars });
167-
const processLogger = this.serviceContainer.get<IProcessLogger>(IProcessLogger);
168-
processService.on('exec', processLogger.logProcess.bind(processLogger));
169-
this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry).push(processService);
168+
processService.on('exec', this.logger.logProcess.bind(this.logger));
169+
this.disposables.push(processService);
170170

171-
return createPythonService(pythonPath, processService, this.serviceContainer.get<IFileSystem>(IFileSystem));
171+
return createPythonService(pythonPath, processService, this.fileSystem);
172172
}
173173
// Not using this function for now because there are breaking issues with conda run (conda 4.8, PVSC 2020.1).
174174
// See https://github.com/microsoft/vscode-python/issues/9490
@@ -190,14 +190,13 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
190190
if (condaVersion && gte(condaVersion, CONDA_RUN_VERSION) && condaEnvironment && condaFile && procService) {
191191
// Add logging to the newly created process service
192192
if (!processService) {
193-
const processLogger = this.serviceContainer.get<IProcessLogger>(IProcessLogger);
194-
procService.on('exec', processLogger.logProcess.bind(processLogger));
195-
this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry).push(procService);
193+
procService.on('exec', this.logger.logProcess.bind(this.logger));
194+
this.disposables.push(procService);
196195
}
197196
return createPythonService(
198197
pythonPath,
199198
procService,
200-
this.serviceContainer.get<IFileSystem>(IFileSystem),
199+
this.fileSystem,
201200
// This is what causes a CondaEnvironment to be returned:
202201
[condaFile, condaEnvironment]
203202
);

src/test/linters/lint.functional.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { CancellationTokenSource, TextDocument, TextLine, Uri } from 'vscode';
1313
import { Product } from '../../client/common/installer/productInstaller';
1414
import { FileSystem } from '../../client/common/platform/fileSystem';
1515
import { PlatformService } from '../../client/common/platform/platformService';
16+
import { IFileSystem } from '../../client/common/platform/types';
1617
import { BufferDecoder } from '../../client/common/process/decoder';
1718
import { ProcessServiceFactory } from '../../client/common/process/processFactory';
1819
import { PythonExecutionFactory } from '../../client/common/process/pythonExecutionFactory';
@@ -622,6 +623,7 @@ class TestFixture extends BaseTestFixture {
622623
const serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>(undefined, TypeMoq.MockBehavior.Strict);
623624
const configService = TypeMoq.Mock.ofType<IConfigurationService>(undefined, TypeMoq.MockBehavior.Strict);
624625
const processLogger = TypeMoq.Mock.ofType<IProcessLogger>(undefined, TypeMoq.MockBehavior.Strict);
626+
const filesystem = new FileSystem();
625627
processLogger
626628
.setup((p) => p.logProcess(TypeMoq.It.isAnyString(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
627629
.returns(() => {
@@ -630,9 +632,11 @@ class TestFixture extends BaseTestFixture {
630632
serviceContainer
631633
.setup((s) => s.get(TypeMoq.It.isValue(IProcessLogger), TypeMoq.It.isAny()))
632634
.returns(() => processLogger.object);
635+
serviceContainer
636+
.setup((s) => s.get(TypeMoq.It.isValue(IFileSystem), TypeMoq.It.isAny()))
637+
.returns(() => filesystem);
633638

634639
const platformService = new PlatformService();
635-
const filesystem = new FileSystem();
636640

637641
super(
638642
platformService,

0 commit comments

Comments
 (0)