correctly dispose of PythonRuntimeManager (#7337)

<!-- Thank you for submitting a pull request.
If this is your first pull request you can find information about
contributing here:
  * https://github.com/posit-dev/positron/blob/main/CONTRIBUTING.md

We recommend synchronizing your branch with the latest changes in the
main branch by either pulling or rebasing.
-->

<!--
  Describe briefly what problem this pull request resolves, or what
  new feature it introduces. Include screenshots of any new or altered
  UI. Link to any GitHub issues but avoid "magic" keywords that will
  automatically close the issue. If there are any details about your
  approach that are unintuitive or you want to draw attention to, please
  describe them here.
-->
Addresses #7206. There is a lot of discussion in that issue for context.

Basically, I think what was happening is that the `PythonRuntimeManager`
was not being correctly disposed when the user trusts a workspace and
everything gets deactivated/activated again. After this change, it does
get disposed during that flow, and I can't reproduce the bug anymore.

### Release Notes

<!--
Optionally, replace `N/A` with text to be included in the next release
notes.
The `N/A` bullets are ignored. If you refer to one or more Positron
issues,
these issues are used to collect information about the feature or
bugfix, such
as the relevant language pack as determined by Github labels of type
`lang: `.
  The note will automatically be tagged with the language.

These notes are typically filled by the Positron team. If you are an
external
  contributor, you may ignore this section.
-->

#### New Features

- N/A

#### Bug Fixes

- Fixed bug in Python startup (#7206)


### QA Notes

1. Create a new project. I used uv:
```
uv init proj
cd proj
uv venv
```
2. Open a fresh Positron with no state, but with PET and multiconsole
on. I do this with the following:
```
rm -rf ~/.vscode-oss-dev
rm -rf ~/.positron-dev
mkdir -p ~/.vscode-oss-dev/User
echo '{"console.multipleConsoleSessions":true,"python.locator":"native"}' > ~/.vscode-oss-dev/User/settings.json
```
4. No workspace should be selected. It should start "Discovering
interpreters".
5. Open Folder to the `proj` workspace
6. Click "trust"
7. The uv interpreter should successfully open a session.

Maybe try this a few times at varying levels of speed.

Author: austin3dickey

extensions/positron-python/src/client/positron/manager.ts

@@ -11,14 +11,21 @@ import * as vscode from 'vscode';
 import * as path from 'path';
 import * as os from 'os';
 
-import { Event, EventEmitter } from 'vscode';
+import { Event, EventEmitter, Disposable } from 'vscode';
 import { inject, injectable } from 'inversify';
 import * as fs from '../common/platform/fs-paths';
 import { IServiceContainer } from '../ioc/types';
 import { pythonRuntimeDiscoverer } from './discoverer';
 import { IInterpreterService } from '../interpreter/contracts';
 import { traceError, traceInfo, traceLog } from '../logging';
-import { IConfigurationService, IDisposable, IInstaller, InstallerResponse, Product } from '../common/types';
+import {
+    IConfigurationService,
+    IDisposable,
+    IDisposableRegistry,
+    IInstaller,
+    InstallerResponse,
+    Product,
+} from '../common/types';
 import { PythonRuntimeSession } from './session';
 import { createPythonRuntimeMetadata, PythonRuntimeExtraData } from './runtime';
 import { Commands, EXTENSION_ROOT_DIR } from '../common/constants';
@@ -51,7 +58,7 @@ export interface IPythonRuntimeManager extends positron.LanguageRuntimeManager {
  * implements positron.LanguageRuntimeManager.
  */
 @injectable()
-export class PythonRuntimeManager implements IPythonRuntimeManager, vscode.Disposable {
+export class PythonRuntimeManager implements IPythonRuntimeManager, Disposable {
     /**
      * A map of Python interpreter paths to their language runtime metadata.
      */
@@ -74,9 +81,11 @@ export class PythonRuntimeManager implements IPythonRuntimeManager, vscode.Dispo
         @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer,
         @inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
     ) {
-        positron.runtime.registerLanguageRuntimeManager('python', this);
+        const disposables = this.serviceContainer.get<Disposable[]>(IDisposableRegistry);
+        disposables.push(this);
 
         this.disposables.push(
+            positron.runtime.registerLanguageRuntimeManager('python', this),
             // When an interpreter is added, register a corresponding language runtime.
             interpreterService.onDidChangeInterpreters(async (event) => {
                 if (!event.old && event.new) {

extensions/positron-python/src/test/positron/manager.unit.test.ts

@@ -20,7 +20,12 @@ import * as interpreterSettings from '../../client/positron/interpreterSettings'
 import * as environmentTypeComparer from '../../client/interpreter/configuration/environmentTypeComparer';
 import * as util from '../../client/positron/util';
 import { IEnvironmentVariablesProvider } from '../../client/common/variables/types';
-import { IConfigurationService, IDisposable, InspectInterpreterSettingType } from '../../client/common/types';
+import {
+    IConfigurationService,
+    IDisposable,
+    IDisposableRegistry,
+    InspectInterpreterSettingType,
+} from '../../client/common/types';
 import { IServiceContainer } from '../../client/ioc/types';
 import { PythonRuntimeManager } from '../../client/positron/manager';
 import { IInterpreterService } from '../../client/interpreter/contracts';
@@ -38,6 +43,7 @@ suite('Python runtime manager', () => {
     let interpreterService: TypeMoq.IMock<IInterpreterService>;
     let serviceContainer: TypeMoq.IMock<IServiceContainer>;
     let workspaceConfig: TypeMoq.IMock<WorkspaceConfiguration>;
+    let disposableRegistry: TypeMoq.IMock<IDisposableRegistry>;
 
     let getConfigurationStub: sinon.SinonStub;
     let isVersionSupportedStub: sinon.SinonStub;
@@ -53,6 +59,7 @@ suite('Python runtime manager', () => {
         interpreterService = createTypeMoq<IInterpreterService>();
         serviceContainer = createTypeMoq<IServiceContainer>();
         workspaceConfig = createTypeMoq<WorkspaceConfiguration>();
+        disposableRegistry = createTypeMoq<IDisposableRegistry>();
 
         runtimeMetadata.setup((r) => r.runtimeId).returns(() => 'runtimeId');
         runtimeMetadata.setup((r) => r.extraRuntimeData).returns(() => ({ pythonPath }));
@@ -64,6 +71,7 @@ suite('Python runtime manager', () => {
         serviceContainer.setup((s) => s.get(IConfigurationService)).returns(() => configService.object);
         serviceContainer.setup((s) => s.get(IEnvironmentVariablesProvider)).returns(() => envVarsProvider.object);
         serviceContainer.setup((s) => s.get(IInterpreterService)).returns(() => interpreterService.object);
+        serviceContainer.setup((s) => s.get(IDisposableRegistry)).returns(() => disposableRegistry.object);
 
         getConfigurationStub = sinon.stub(workspaceApis, 'getConfiguration');
         getConfigurationStub.callsFake((section?: string, _scope?: any) => {
@@ -191,6 +199,7 @@ suite('Python runtime manager - recommendedWorkspaceRuntime', () => {
     let interpreterService: TypeMoq.IMock<IInterpreterService>;
     let interpreter: TypeMoq.IMock<PythonEnvironment>;
     let runtimeMetadata: positron.LanguageRuntimeMetadata;
+    let disposableRegistry: TypeMoq.IMock<IDisposableRegistry>;
 
     let getUserDefaultInterpreterStub: sinon.SinonStub;
     let hasFilesStub: sinon.SinonStub;
@@ -201,9 +210,11 @@ suite('Python runtime manager - recommendedWorkspaceRuntime', () => {
         serviceContainer = createTypeMoq<IServiceContainer>();
         interpreterService = createTypeMoq<IInterpreterService>();
         interpreter = createTypeMoq<PythonEnvironment>();
+        disposableRegistry = createTypeMoq<IDisposableRegistry>();
 
         // Setup interpreter service
         serviceContainer.setup((s) => s.get(IInterpreterService)).returns(() => interpreterService.object);
+        serviceContainer.setup((s) => s.get(IDisposableRegistry)).returns(() => disposableRegistry.object);
 
         getUserDefaultInterpreterStub = sinon.stub(interpreterSettings, 'getUserDefaultInterpreter');
         hasFilesStub = sinon.stub(util, 'hasFiles');

extensions/vscode-api-tests/src/singlefolder-tests/positron/runtime.test.ts

@@ -299,14 +299,11 @@ suite('positron API - runtime', () => {
 
 		managerDisposable.dispose();
 
-		// TODO: Unregistering a manager unregisters its runtimes, but doesn't remove them from
-		//       the list returned by positron.runtime.getRegisteredRuntimes. Is that a bug?
-		//       It also means that this test will currently fail if run out of order.
-		// await poll(
-		// 	getRegisteredRuntimes,
-		// 	(runtimes) => runtimes.length === 0,
-		// 	'test runtimes should be unregistered',
-		// );
+		await poll(
+			getRegisteredRuntimes,
+			(runtimes) => runtimes.length === 0,
+			'test runtimes should be unregistered',
+		);
 	});
 
 });
@@ -585,7 +582,7 @@ suite('positron API - executeCode', () => {
 			positron.runtime.onDidExecuteCode((e) => {
 				event = e;
 			})
-		)
+		);
 
 		// Execute the code
 		await positron.runtime.executeCode(

src/vs/workbench/api/common/positron/extHostLanguageRuntime.ts

@@ -1073,6 +1073,17 @@ export class ExtHostLanguageRuntime implements extHostProtocol.ExtHostLanguageRu
 			if (index >= 0) {
 				this._runtimeManagers.splice(index, 1);
 			}
+
+			// Remove its associated runtimes
+			this._runtimeManagersByRuntimeId.forEach((m, runtimeId) => {
+				if (m.manager === manager) {
+					this._runtimeManagersByRuntimeId.delete(runtimeId);
+					const index = this._registeredRuntimes.findIndex(runtime => runtime.runtimeId === runtimeId);
+					if (index >= 0) {
+						this._registeredRuntimes.splice(index, 1);
+					}
+				}
+			});
 		});
 	}