From ad675c1f7b8e6144cc4b15dae892c2adcdc8d52e Mon Sep 17 00:00:00 2001
From: Kartik Raj <karraj@microsoft.com>
Date: Tue, 3 Nov 2020 16:12:57 -0800
Subject: [PATCH 01/10] Add FSWatching locator base class

---
 .../common/platform/fileSystemWatcher.ts      | 47 ++++++++++++-------
 src/client/pythonEnvironments/base/locator.ts | 14 +++++-
 .../base/locators/lowLevel/locator.ts         | 14 ++++++
 .../common/pythonBinariesWatcher.ts           | 13 +++--
 .../globalVirtualEnvronmentLocator.ts         | 37 +++++++++------
 .../locators/services/windowsStoreLocator.ts  | 21 +++++----
 6 files changed, 101 insertions(+), 45 deletions(-)
 create mode 100644 src/client/pythonEnvironments/base/locators/lowLevel/locator.ts

diff --git a/src/client/common/platform/fileSystemWatcher.ts b/src/client/common/platform/fileSystemWatcher.ts
index 61016b77fcd0..97b7720deb0c 100644
--- a/src/client/common/platform/fileSystemWatcher.ts
+++ b/src/client/common/platform/fileSystemWatcher.ts
@@ -5,7 +5,7 @@
 
 import * as chokidar from 'chokidar';
 import * as path from 'path';
-import { RelativePattern, workspace } from 'vscode';
+import { Disposable, RelativePattern, workspace } from 'vscode';
 import { traceError, traceVerbose, traceWarning } from '../logger';
 import { normCasePath } from './fs-paths';
 
@@ -22,35 +22,40 @@ const POLLING_INTERVAL = 5000;
 export function watchLocationForPattern(
     baseDir: string,
     pattern: string,
-    callback: (type: FileChangeType, absPath: string) => void
-): void {
+    callback: (type: FileChangeType, absPath: string) => void,
+): Disposable {
     // Use VSCode API iff base directory to exists within the current workspace folders
     const found = workspace.workspaceFolders?.find((e) => normCasePath(baseDir).startsWith(normCasePath(e.uri.fsPath)));
     if (found) {
-        watchLocationUsingVSCodeAPI(baseDir, pattern, callback);
-    } else {
-        // Fallback to chokidar as base directory to lookup doesn't exist within the current workspace folders
-        watchLocationUsingChokidar(baseDir, pattern, callback);
+        return watchLocationUsingVSCodeAPI(baseDir, pattern, callback);
     }
+    // Fallback to chokidar as base directory to lookup doesn't exist within the current workspace folders
+    return watchLocationUsingChokidar(baseDir, pattern, callback);
 }
 
 function watchLocationUsingVSCodeAPI(
     baseDir: string,
     pattern: string,
-    callback: (type: FileChangeType, absPath: string) => void
+    callback: (type: FileChangeType, absPath: string) => void,
 ) {
     const globPattern = new RelativePattern(baseDir, pattern);
+    const disposables: Disposable[] = [];
     traceVerbose(`Start watching: ${baseDir} with pattern ${pattern} using VSCode API`);
     const watcher = workspace.createFileSystemWatcher(globPattern);
-    watcher.onDidCreate((e) => callback(FileChangeType.Created, e.fsPath));
-    watcher.onDidChange((e) => callback(FileChangeType.Changed, e.fsPath));
-    watcher.onDidDelete((e) => callback(FileChangeType.Deleted, e.fsPath));
+    disposables.push(watcher.onDidCreate((e) => callback(FileChangeType.Created, e.fsPath)));
+    disposables.push(watcher.onDidChange((e) => callback(FileChangeType.Changed, e.fsPath)));
+    disposables.push(watcher.onDidDelete((e) => callback(FileChangeType.Deleted, e.fsPath)));
+    return {
+        dispose: async () => {
+            disposables.forEach((d) => d.dispose());
+        },
+    };
 }
 
 function watchLocationUsingChokidar(
     baseDir: string,
     pattern: string,
-    callback: (type: FileChangeType, absPath: string) => void
+    callback: (type: FileChangeType, absPath: string) => void,
 ) {
     const watcherOpts: chokidar.WatchOptions = {
         cwd: baseDir,
@@ -69,9 +74,9 @@ function watchLocationUsingChokidar(
             '**/.hg/store/**',
             '/dev/**',
             '/proc/**',
-            '/sys/**'
+            '/sys/**',
         ], // https://github.com/microsoft/vscode/issues/23954
-        followSymlinks: false
+        followSymlinks: false,
     };
     traceVerbose(`Start watching: ${baseDir} with pattern ${pattern} using chokidar`);
     let watcher: chokidar.FSWatcher | null = chokidar.watch(pattern, watcherOpts);
@@ -96,6 +101,13 @@ function watchLocationUsingChokidar(
         callback(eventType, absPath);
     });
 
+    const dispose = async () => {
+        if (watcher) {
+            await watcher.close();
+            watcher = null;
+        }
+    };
+
     watcher.on('error', async (error: NodeJS.ErrnoException) => {
         if (error) {
             // Specially handle ENOSPC errors that can happen when
@@ -105,13 +117,12 @@ function watchLocationUsingChokidar(
             // See https://github.com/Microsoft/vscode/issues/7950
             if (error.code === 'ENOSPC') {
                 traceError(`Inotify limit reached (ENOSPC) for ${baseDir} with pattern ${pattern}`);
-                if (watcher) {
-                    await watcher.close();
-                    watcher = null;
-                }
+                await dispose();
             } else {
                 traceWarning(error.toString());
             }
         }
     });
+
+    return { dispose };
 }
diff --git a/src/client/pythonEnvironments/base/locator.ts b/src/client/pythonEnvironments/base/locator.ts
index a1b372fb6988..b8e03fb860ea 100644
--- a/src/client/pythonEnvironments/base/locator.ts
+++ b/src/client/pythonEnvironments/base/locator.ts
@@ -1,7 +1,10 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+// eslint-disable-next-line max-classes-per-file
 import { Event, Uri } from 'vscode';
+import { AsyncDisposableRegistry } from '../../common/asyncDisposableRegistry';
+import { IAsyncDisposableRegistry, IDisposable } from '../../common/types';
 import { iterEmpty } from '../../common/utils/async';
 import { PythonEnvInfo, PythonEnvKind } from './info';
 import {
@@ -178,9 +181,14 @@ interface IEmitter<E extends BasicPythonEnvsChangedEvent> {
  * should be used.  Only in low-level cases should you consider using
  * `BasicPythonEnvsChangedEvent`.
  */
-export abstract class LocatorBase<E extends BasicPythonEnvsChangedEvent = PythonEnvsChangedEvent> implements ILocator<E> {
+export abstract class LocatorBase<E extends BasicPythonEnvsChangedEvent = PythonEnvsChangedEvent>
+implements IDisposable, ILocator<E> {
     public readonly onChanged: Event<E>;
+
     protected readonly emitter: IEmitter<E>;
+
+    protected readonly disposables: IAsyncDisposableRegistry = new AsyncDisposableRegistry();
+
     constructor(watcher: IPythonEnvsWatcher<E> & IEmitter<E>) {
         this.emitter = watcher;
         this.onChanged = watcher.onChanged;
@@ -191,6 +199,10 @@ export abstract class LocatorBase<E extends BasicPythonEnvsChangedEvent = Python
     public async resolveEnv(_env: string | PythonEnvInfo): Promise<PythonEnvInfo | undefined> {
         return undefined;
     }
+
+    public dispose(): void {
+        this.disposables.dispose().ignoreErrors();
+    }
 }
 
 /**
diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/locator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/locator.ts
new file mode 100644
index 000000000000..87602b59cee8
--- /dev/null
+++ b/src/client/pythonEnvironments/base/locators/lowLevel/locator.ts
@@ -0,0 +1,14 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import { Locator } from '../../locator';
+
+/**
+ * The base for Python envs locators who watch the file system.
+ * Most low-level locators should be using this.
+ *
+ * Subclasses can call `this.emitter.fire()` * to emit events.
+ */
+export abstract class FSWatchingLocator extends Locator {
+    public async abstract initialize(): Promise<void>;
+}
diff --git a/src/client/pythonEnvironments/common/pythonBinariesWatcher.ts b/src/client/pythonEnvironments/common/pythonBinariesWatcher.ts
index 3008fb20366a..d4abc81d4659 100644
--- a/src/client/pythonEnvironments/common/pythonBinariesWatcher.ts
+++ b/src/client/pythonEnvironments/common/pythonBinariesWatcher.ts
@@ -5,6 +5,7 @@
 
 import * as minimatch from 'minimatch';
 import * as path from 'path';
+import { Disposable } from 'vscode';
 import { FileChangeType, watchLocationForPattern } from '../../common/platform/fileSystemWatcher';
 import { getOSType, OSType } from '../../common/utils/platform';
 
@@ -14,10 +15,11 @@ export function watchLocationForPythonBinaries(
     baseDir: string,
     callback: (type: FileChangeType, absPath: string) => void,
     executableGlob: string = executable,
-): void {
+): Disposable {
     const patterns = [executableGlob, `*/${executableGlob}`, `*/${binName}/${executableGlob}`];
+    const disposables: Disposable[] = [];
     for (const pattern of patterns) {
-        watchLocationForPattern(baseDir, pattern, (type: FileChangeType, e: string) => {
+        disposables.push(watchLocationForPattern(baseDir, pattern, (type: FileChangeType, e: string) => {
             const isMatch = minimatch(e, path.join('**', executableGlob), { nocase: getOSType() === OSType.Windows });
             if (!isMatch) {
                 // When deleting the file for some reason path to all directories leading up to python are reported
@@ -25,6 +27,11 @@ export function watchLocationForPythonBinaries(
                 return;
             }
             callback(type, e);
-        });
+        }));
     }
+    return {
+        dispose: async () => {
+            disposables.forEach((d) => d.dispose());
+        },
+    };
 }
diff --git a/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts
index 7662a746b67e..5bb7e753a899 100644
--- a/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts
+++ b/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts
@@ -7,11 +7,12 @@ import { traceVerbose } from '../../../../common/logger';
 import { FileChangeType } from '../../../../common/platform/fileSystemWatcher';
 import { chain, iterable, sleep } from '../../../../common/utils/async';
 import {
-    getEnvironmentVariable, getOSType, getUserHomeDir, OSType
+    getEnvironmentVariable, getOSType, getUserHomeDir, OSType,
 } from '../../../../common/utils/platform';
 import { PythonEnvInfo, PythonEnvKind, UNKNOWN_PYTHON_VERSION } from '../../../base/info';
 import { buildEnvInfo } from '../../../base/info/env';
-import { IPythonEnvsIterator, Locator } from '../../../base/locator';
+import { IPythonEnvsIterator } from '../../../base/locator';
+import { FSWatchingLocator } from '../../../base/locators/lowLevel/locator';
 import { findInterpretersInDir } from '../../../common/commonUtils';
 import { getFileInfo, pathExists } from '../../../common/externalDependencies';
 import { watchLocationForPythonBinaries } from '../../../common/pythonBinariesWatcher';
@@ -19,7 +20,7 @@ import { isPipenvEnvironment } from './pipEnvHelper';
 import {
     isVenvEnvironment,
     isVirtualenvEnvironment,
-    isVirtualenvwrapperEnvironment
+    isVirtualenvwrapperEnvironment,
 } from './virtualEnvironmentIdentifier';
 
 const DEFAULT_SEARCH_DEPTH = 2;
@@ -80,7 +81,7 @@ async function getVirtualEnvKind(interpreterPath: string): Promise<PythonEnvKind
 /**
  * Finds and resolves virtual environments created in known global locations.
  */
-export class GlobalVirtualEnvironmentLocator extends Locator {
+export class GlobalVirtualEnvironmentLocator extends FSWatchingLocator {
     private virtualEnvKinds = [
         PythonEnvKind.Venv,
         PythonEnvKind.VirtualEnv,
@@ -90,7 +91,10 @@ export class GlobalVirtualEnvironmentLocator extends Locator {
 
     public constructor(private readonly searchDepth?: number) {
         super();
-        this.registerWatchers().ignoreErrors();
+    }
+
+    public async initialize(): Promise<void> {
+        await this.startWatchers();
     }
 
     public iterEnvs(): IPythonEnvsIterator {
@@ -168,15 +172,20 @@ export class GlobalVirtualEnvironmentLocator extends Locator {
         return undefined;
     }
 
-    private async registerWatchers(): Promise<void> {
+    private async startWatchers(): Promise<void> {
         const dirs = await getGlobalVirtualEnvDirs();
-        dirs.forEach((d) => watchLocationForPythonBinaries(d, async (type: FileChangeType, executablePath: string) => {
-            // Note detecting kind of virtual env depends on the file structure around the executable, so we need to
-            // wait before attempting to detect it. However even if the type detected is incorrect, it doesn't do any
-            // practical harm as kinds in this locator are used in the same way (same activation commands etc.)
-            await sleep(1000);
-            const kind = await getVirtualEnvKind(executablePath);
-            this.emitter.fire({ type, kind });
-        }));
+        dirs.forEach(
+            (d) => this.disposables.push(
+                watchLocationForPythonBinaries(d, async (type: FileChangeType, executablePath: string) => {
+                    // Note detecting kind of virtual env depends on the file structure around the
+                    // executable, so we need to wait before attempting to detect it. However even
+                    // if the type detected is incorrect, it doesn't do any practical harm as kinds
+                    // in this locator are used in the same way (same activation commands etc.)
+                    await sleep(1000);
+                    const kind = await getVirtualEnvKind(executablePath);
+                    this.emitter.fire({ type, kind });
+                }),
+            ),
+        );
     }
 }
diff --git a/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts
index a095d06e1023..7aa4ccd33c91 100644
--- a/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts
+++ b/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts
@@ -10,7 +10,8 @@ import { Architecture, getEnvironmentVariable } from '../../../../common/utils/p
 import { PythonEnvInfo, PythonEnvKind } from '../../../base/info';
 import { buildEnvInfo } from '../../../base/info/env';
 import { getPythonVersionFromPath } from '../../../base/info/pythonVersion';
-import { IPythonEnvsIterator, Locator } from '../../../base/locator';
+import { IPythonEnvsIterator } from '../../../base/locator';
+import { FSWatchingLocator } from '../../../base/locators/lowLevel/locator';
 import { getFileInfo } from '../../../common/externalDependencies';
 import { watchLocationForPythonBinaries } from '../../../common/pythonBinariesWatcher';
 
@@ -137,10 +138,10 @@ export async function getWindowsStorePythonExes(): Promise<string[]> {
         .filter(isWindowsStorePythonExe);
 }
 
-export class WindowsStoreLocator extends Locator {
+export class WindowsStoreLocator extends FSWatchingLocator {
     private readonly kind: PythonEnvKind = PythonEnvKind.WindowsStore;
 
-    public initialize(): void {
+    public async initialize(): Promise<void> {
         this.startWatcher();
     }
 
@@ -176,12 +177,14 @@ export class WindowsStoreLocator extends Locator {
 
     private startWatcher(): void {
         const windowsAppsRoot = getWindowsStoreAppsRoot();
-        watchLocationForPythonBinaries(
-            windowsAppsRoot,
-            (type: FileChangeType) => {
-                this.emitter.fire({ type, kind: this.kind });
-            },
-            pythonExeGlob,
+        this.disposables.push(
+            watchLocationForPythonBinaries(
+                windowsAppsRoot,
+                (type: FileChangeType) => {
+                    this.emitter.fire({ type, kind: this.kind });
+                },
+                pythonExeGlob,
+            ),
         );
     }
 }

From 06e6f5b75028699fa78596f1487d0dfb3a2c1df9 Mon Sep 17 00:00:00 2001
From: Kartik Raj <karraj@microsoft.com>
Date: Tue, 3 Nov 2020 17:14:42 -0800
Subject: [PATCH 02/10] Correct glob pattern to not match python3.2whoa

---
 .../discovery/locators/services/windowsStoreLocator.ts      | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts
index 7aa4ccd33c91..1363918fe69b 100644
--- a/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts
+++ b/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts
@@ -93,14 +93,16 @@ export async function isWindowsStoreEnvironment(interpreterPath: string): Promis
  * This is a glob pattern which matches following file names:
  * python3.8.exe
  * python3.9.exe
+ * python3.10.exe
  * This pattern does not match:
  * python.exe
  * python2.7.exe
  * python3.exe
  * python38.exe
- * 'python.exe', 'python3.exe', and 'python3.8.exe' can point to the same executable, hence only capture python3.*.exes.
+ * Note chokidar fails to match multiple digits using +([0-9]), even though the underlying glob pattern matcher
+ * they use (picomatch), or any other glob matcher does. Hence why we had to use {[0-9],[0-9][0-9]} instead.
  */
-const pythonExeGlob = 'python3\.[0-9]*\.exe';
+const pythonExeGlob = 'python3\.{[0-9],[0-9][0-9]}\.exe';
 
 /**
  * Checks if a given path ends with python3.*.exe. Not all python executables are matched as

From c167f57c138ea7bafc48a4e16f79b1fa2bbaaa6b Mon Sep 17 00:00:00 2001
From: Kartik Raj <karraj@microsoft.com>
Date: Tue, 3 Nov 2020 21:48:30 -0800
Subject: [PATCH 03/10] Add documentation of python binary watcher

---
 .../common/pythonBinariesWatcher.ts                   | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/client/pythonEnvironments/common/pythonBinariesWatcher.ts b/src/client/pythonEnvironments/common/pythonBinariesWatcher.ts
index d4abc81d4659..5ee460070c0a 100644
--- a/src/client/pythonEnvironments/common/pythonBinariesWatcher.ts
+++ b/src/client/pythonEnvironments/common/pythonBinariesWatcher.ts
@@ -11,16 +11,21 @@ import { getOSType, OSType } from '../../common/utils/platform';
 
 const [executable, binName] = getOSType() === OSType.Windows ? ['python.exe', 'Scripts'] : ['python', 'bin'];
 
+/**
+ * @param baseDir The base directory from which watch paths are to be derived.
+ * @param callback The listener function will be called when the event happens.
+ * @param executableSuffixGlob Glob which represents suffix of the full executable file path to watch.
+ */
 export function watchLocationForPythonBinaries(
     baseDir: string,
     callback: (type: FileChangeType, absPath: string) => void,
-    executableGlob: string = executable,
+    executableSuffixGlob: string = executable,
 ): Disposable {
-    const patterns = [executableGlob, `*/${executableGlob}`, `*/${binName}/${executableGlob}`];
+    const patterns = [executableSuffixGlob, `*/${executableSuffixGlob}`, `*/${binName}/${executableSuffixGlob}`];
     const disposables: Disposable[] = [];
     for (const pattern of patterns) {
         disposables.push(watchLocationForPattern(baseDir, pattern, (type: FileChangeType, e: string) => {
-            const isMatch = minimatch(e, path.join('**', executableGlob), { nocase: getOSType() === OSType.Windows });
+            const isMatch = minimatch(e, path.join('**', executableSuffixGlob), { nocase: getOSType() === OSType.Windows });
             if (!isMatch) {
                 // When deleting the file for some reason path to all directories leading up to python are reported
                 // Skip those events

From 375df6bc75074be2fcbc696ebe2b546d081deb0c Mon Sep 17 00:00:00 2001
From: Kartik Raj <karraj@microsoft.com>
Date: Tue, 3 Nov 2020 21:57:06 -0800
Subject: [PATCH 04/10] Fix lint errors

---
 src/client/common/platform/fileSystemWatcher.ts      | 12 ++++++------
 .../discovery/locators/windowsStoreLocator.test.ts   |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/client/common/platform/fileSystemWatcher.ts b/src/client/common/platform/fileSystemWatcher.ts
index 97b7720deb0c..829514e9db89 100644
--- a/src/client/common/platform/fileSystemWatcher.ts
+++ b/src/client/common/platform/fileSystemWatcher.ts
@@ -22,7 +22,7 @@ const POLLING_INTERVAL = 5000;
 export function watchLocationForPattern(
     baseDir: string,
     pattern: string,
-    callback: (type: FileChangeType, absPath: string) => void,
+    callback: (type: FileChangeType, absPath: string) => void
 ): Disposable {
     // Use VSCode API iff base directory to exists within the current workspace folders
     const found = workspace.workspaceFolders?.find((e) => normCasePath(baseDir).startsWith(normCasePath(e.uri.fsPath)));
@@ -36,7 +36,7 @@ export function watchLocationForPattern(
 function watchLocationUsingVSCodeAPI(
     baseDir: string,
     pattern: string,
-    callback: (type: FileChangeType, absPath: string) => void,
+    callback: (type: FileChangeType, absPath: string) => void
 ) {
     const globPattern = new RelativePattern(baseDir, pattern);
     const disposables: Disposable[] = [];
@@ -48,14 +48,14 @@ function watchLocationUsingVSCodeAPI(
     return {
         dispose: async () => {
             disposables.forEach((d) => d.dispose());
-        },
+        }
     };
 }
 
 function watchLocationUsingChokidar(
     baseDir: string,
     pattern: string,
-    callback: (type: FileChangeType, absPath: string) => void,
+    callback: (type: FileChangeType, absPath: string) => void
 ) {
     const watcherOpts: chokidar.WatchOptions = {
         cwd: baseDir,
@@ -74,9 +74,9 @@ function watchLocationUsingChokidar(
             '**/.hg/store/**',
             '/dev/**',
             '/proc/**',
-            '/sys/**',
+            '/sys/**'
         ], // https://github.com/microsoft/vscode/issues/23954
-        followSymlinks: false,
+        followSymlinks: false
     };
     traceVerbose(`Start watching: ${baseDir} with pattern ${pattern} using chokidar`);
     let watcher: chokidar.FSWatcher | null = chokidar.watch(pattern, watcherOpts);
diff --git a/src/test/pythonEnvironments/discovery/locators/windowsStoreLocator.test.ts b/src/test/pythonEnvironments/discovery/locators/windowsStoreLocator.test.ts
index cb71c063c981..491bcd8065a7 100644
--- a/src/test/pythonEnvironments/discovery/locators/windowsStoreLocator.test.ts
+++ b/src/test/pythonEnvironments/discovery/locators/windowsStoreLocator.test.ts
@@ -83,7 +83,7 @@ suite('Windows Store Locator', async () => {
 
     async function setupLocator(onChanged: (e: PythonEnvsChangedEvent) => Promise<void>) {
         locator = new WindowsStoreLocator();
-        locator.initialize();
+        await locator.initialize();
         // Wait for watchers to get ready
         await sleep(1000);
         locator.onChanged(onChanged);

From 3ef70e303c6978aab7ade5aa9283fea0187dba34 Mon Sep 17 00:00:00 2001
From: Kartik Raj <karraj@microsoft.com>
Date: Tue, 3 Nov 2020 22:07:34 -0800
Subject: [PATCH 05/10] Update ignore list

---
 src/client/common/platform/fileSystemWatcher.ts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/client/common/platform/fileSystemWatcher.ts b/src/client/common/platform/fileSystemWatcher.ts
index 829514e9db89..334249637576 100644
--- a/src/client/common/platform/fileSystemWatcher.ts
+++ b/src/client/common/platform/fileSystemWatcher.ts
@@ -74,7 +74,9 @@ function watchLocationUsingChokidar(
             '**/.hg/store/**',
             '/dev/**',
             '/proc/**',
-            '/sys/**'
+            '/sys/**',
+            '**/lib/**',
+            '**/includes/**'
         ], // https://github.com/microsoft/vscode/issues/23954
         followSymlinks: false
     };

From 5925ab164327d1f1b73856ce9f41ab4fe95947e0 Mon Sep 17 00:00:00 2001
From: Kartik Raj <karraj@microsoft.com>
Date: Wed, 4 Nov 2020 16:31:47 -0800
Subject: [PATCH 06/10] Add disposable registry

---
 .../common/platform/fileSystemWatcher.ts      | 31 +++++++------
 src/client/common/syncDisposableRegistry.ts   | 32 ++++++++++++++
 src/client/pythonEnvironments/base/locator.ts | 44 +++++++------------
 .../{locator.ts => fsWatchingLocator.ts}      |  0
 .../common/pythonBinariesWatcher.ts           | 24 +++++-----
 .../globalVirtualEnvronmentLocator.ts         |  6 +--
 .../locators/services/windowsStoreLocator.ts  |  2 +-
 7 files changed, 80 insertions(+), 59 deletions(-)
 create mode 100644 src/client/common/syncDisposableRegistry.ts
 rename src/client/pythonEnvironments/base/locators/lowLevel/{locator.ts => fsWatchingLocator.ts} (100%)

diff --git a/src/client/common/platform/fileSystemWatcher.ts b/src/client/common/platform/fileSystemWatcher.ts
index 334249637576..596214832a29 100644
--- a/src/client/common/platform/fileSystemWatcher.ts
+++ b/src/client/common/platform/fileSystemWatcher.ts
@@ -5,8 +5,10 @@
 
 import * as chokidar from 'chokidar';
 import * as path from 'path';
-import { Disposable, RelativePattern, workspace } from 'vscode';
+import { RelativePattern, workspace } from 'vscode';
 import { traceError, traceVerbose, traceWarning } from '../logger';
+import { DisposableRegistry } from '../syncDisposableRegistry';
+import { IDisposable } from '../types';
 import { normCasePath } from './fs-paths';
 
 /**
@@ -23,7 +25,7 @@ export function watchLocationForPattern(
     baseDir: string,
     pattern: string,
     callback: (type: FileChangeType, absPath: string) => void
-): Disposable {
+): IDisposable {
     // Use VSCode API iff base directory to exists within the current workspace folders
     const found = workspace.workspaceFolders?.find((e) => normCasePath(baseDir).startsWith(normCasePath(e.uri.fsPath)));
     if (found) {
@@ -37,26 +39,22 @@ function watchLocationUsingVSCodeAPI(
     baseDir: string,
     pattern: string,
     callback: (type: FileChangeType, absPath: string) => void
-) {
+): IDisposable {
     const globPattern = new RelativePattern(baseDir, pattern);
-    const disposables: Disposable[] = [];
+    const disposables = new DisposableRegistry();
     traceVerbose(`Start watching: ${baseDir} with pattern ${pattern} using VSCode API`);
     const watcher = workspace.createFileSystemWatcher(globPattern);
     disposables.push(watcher.onDidCreate((e) => callback(FileChangeType.Created, e.fsPath)));
     disposables.push(watcher.onDidChange((e) => callback(FileChangeType.Changed, e.fsPath)));
     disposables.push(watcher.onDidDelete((e) => callback(FileChangeType.Deleted, e.fsPath)));
-    return {
-        dispose: async () => {
-            disposables.forEach((d) => d.dispose());
-        }
-    };
+    return disposables;
 }
 
 function watchLocationUsingChokidar(
     baseDir: string,
     pattern: string,
     callback: (type: FileChangeType, absPath: string) => void
-) {
+): IDisposable {
     const watcherOpts: chokidar.WatchOptions = {
         cwd: baseDir,
         ignoreInitial: true,
@@ -103,10 +101,15 @@ function watchLocationUsingChokidar(
         callback(eventType, absPath);
     });
 
-    const dispose = async () => {
+    const stopWatcher = async () => {
         if (watcher) {
-            await watcher.close();
+            const obj = watcher;
             watcher = null;
+            try {
+                await obj.close();
+            } catch (err) {
+                traceError(`Failed to close FS watcher (${err})`);
+            }
         }
     };
 
@@ -119,12 +122,12 @@ function watchLocationUsingChokidar(
             // See https://github.com/Microsoft/vscode/issues/7950
             if (error.code === 'ENOSPC') {
                 traceError(`Inotify limit reached (ENOSPC) for ${baseDir} with pattern ${pattern}`);
-                await dispose();
+                await stopWatcher();
             } else {
                 traceWarning(error.toString());
             }
         }
     });
 
-    return { dispose };
+    return { dispose: () => stopWatcher().ignoreErrors() };
 }
diff --git a/src/client/common/syncDisposableRegistry.ts b/src/client/common/syncDisposableRegistry.ts
new file mode 100644
index 000000000000..f0fe4bfac6f1
--- /dev/null
+++ b/src/client/common/syncDisposableRegistry.ts
@@ -0,0 +1,32 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+'use strict';
+
+import { traceWarning } from './logger';
+import { IDisposable } from './types';
+
+export class DisposableRegistry implements IDisposable {
+    private _list: IDisposable[] = [];
+
+    public dispose(): void {
+        this._list.forEach((l, index) => {
+            try {
+                l.dispose();
+            } catch (err) {
+                traceWarning(`dispose() #${index} failed (${err})`);
+            }
+        });
+        this._list = [];
+    }
+
+    public push(disposable?: IDisposable): void {
+        if (disposable) {
+            this._list.push(disposable);
+        }
+    }
+
+    public get list(): IDisposable[] {
+        return this._list;
+    }
+}
diff --git a/src/client/pythonEnvironments/base/locator.ts b/src/client/pythonEnvironments/base/locator.ts
index b8e03fb860ea..e021e286b07c 100644
--- a/src/client/pythonEnvironments/base/locator.ts
+++ b/src/client/pythonEnvironments/base/locator.ts
@@ -3,8 +3,8 @@
 
 // eslint-disable-next-line max-classes-per-file
 import { Event, Uri } from 'vscode';
-import { AsyncDisposableRegistry } from '../../common/asyncDisposableRegistry';
-import { IAsyncDisposableRegistry, IDisposable } from '../../common/types';
+import { DisposableRegistry } from '../../common/syncDisposableRegistry';
+import { IDisposable } from '../../common/types';
 import { iterEmpty } from '../../common/utils/async';
 import { PythonEnvInfo, PythonEnvKind } from './info';
 import {
@@ -170,28 +170,32 @@ interface IEmitter<E extends BasicPythonEnvsChangedEvent> {
 }
 
 /**
- * The generic base for Python envs locators.
+ * The base for most Python envs locators.
  *
  * By default `resolveEnv()` returns undefined.  Subclasses may override
  * the method to provide an implementation.
  *
- * Subclasses will call `this.emitter.fire()` to emit events.
+ * Subclasses will call `this.emitter.fire()` * to emit events.
+ *
+ * In most cases this is the class you will want to subclass.
+ * Only in low-level cases should you consider subclassing `LocatorBase`
+ * using `BasicPythonEnvsChangedEvent.
  *
  * Also, in most cases the default event type (`PythonEnvsChangedEvent`)
  * should be used.  Only in low-level cases should you consider using
  * `BasicPythonEnvsChangedEvent`.
  */
-export abstract class LocatorBase<E extends BasicPythonEnvsChangedEvent = PythonEnvsChangedEvent>
+export abstract class Locator<E extends BasicPythonEnvsChangedEvent = PythonEnvsChangedEvent>
 implements IDisposable, ILocator<E> {
     public readonly onChanged: Event<E>;
 
-    protected readonly emitter: IEmitter<E>;
+    protected readonly emitter: IPythonEnvsWatcher<E> & IEmitter<E>;
 
-    protected readonly disposables: IAsyncDisposableRegistry = new AsyncDisposableRegistry();
+    protected readonly disposables = new DisposableRegistry();
 
-    constructor(watcher: IPythonEnvsWatcher<E> & IEmitter<E>) {
-        this.emitter = watcher;
-        this.onChanged = watcher.onChanged;
+    constructor() {
+        this.emitter = new PythonEnvsWatcher();
+        this.onChanged = this.emitter.onChanged;
     }
 
     public abstract iterEnvs(query?: QueryForEvent<E>): IPythonEnvsIterator;
@@ -201,24 +205,6 @@ implements IDisposable, ILocator<E> {
     }
 
     public dispose(): void {
-        this.disposables.dispose().ignoreErrors();
-    }
-}
-
-/**
- * The base for most Python envs locators.
- *
- * By default `resolveEnv()` returns undefined.  Subclasses may override
- * the method to provide an implementation.
- *
- * Subclasses will call `this.emitter.fire()` * to emit events.
- *
- * In most cases this is the class you will want to subclass.
- * Only in low-level cases should you consider subclassing `LocatorBase`
- * using `BasicPythonEnvsChangedEvent.
- */
-export abstract class Locator extends LocatorBase {
-    constructor() {
-        super(new PythonEnvsWatcher());
+        this.disposables.dispose();
     }
 }
diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/locator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts
similarity index 100%
rename from src/client/pythonEnvironments/base/locators/lowLevel/locator.ts
rename to src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts
diff --git a/src/client/pythonEnvironments/common/pythonBinariesWatcher.ts b/src/client/pythonEnvironments/common/pythonBinariesWatcher.ts
index 5ee460070c0a..95878967a386 100644
--- a/src/client/pythonEnvironments/common/pythonBinariesWatcher.ts
+++ b/src/client/pythonEnvironments/common/pythonBinariesWatcher.ts
@@ -5,8 +5,9 @@
 
 import * as minimatch from 'minimatch';
 import * as path from 'path';
-import { Disposable } from 'vscode';
 import { FileChangeType, watchLocationForPattern } from '../../common/platform/fileSystemWatcher';
+import { DisposableRegistry } from '../../common/syncDisposableRegistry';
+import { IDisposable } from '../../common/types';
 import { getOSType, OSType } from '../../common/utils/platform';
 
 const [executable, binName] = getOSType() === OSType.Windows ? ['python.exe', 'Scripts'] : ['python', 'bin'];
@@ -14,18 +15,21 @@ const [executable, binName] = getOSType() === OSType.Windows ? ['python.exe', 'S
 /**
  * @param baseDir The base directory from which watch paths are to be derived.
  * @param callback The listener function will be called when the event happens.
- * @param executableSuffixGlob Glob which represents suffix of the full executable file path to watch.
+ * @param executableBaseGlob Glob which represents basename of the executable to watch.
  */
 export function watchLocationForPythonBinaries(
     baseDir: string,
     callback: (type: FileChangeType, absPath: string) => void,
-    executableSuffixGlob: string = executable,
-): Disposable {
-    const patterns = [executableSuffixGlob, `*/${executableSuffixGlob}`, `*/${binName}/${executableSuffixGlob}`];
-    const disposables: Disposable[] = [];
+    executableBaseGlob: string = executable,
+): IDisposable {
+    if (executableBaseGlob.includes(path.sep)) {
+        throw new Error('Glob basename contains invalid characters');
+    }
+    const patterns = [executableBaseGlob, `*/${executableBaseGlob}`, `*/${binName}/${executableBaseGlob}`];
+    const disposables = new DisposableRegistry();
     for (const pattern of patterns) {
         disposables.push(watchLocationForPattern(baseDir, pattern, (type: FileChangeType, e: string) => {
-            const isMatch = minimatch(e, path.join('**', executableSuffixGlob), { nocase: getOSType() === OSType.Windows });
+            const isMatch = minimatch(e, path.join('**', executableBaseGlob), { nocase: getOSType() === OSType.Windows });
             if (!isMatch) {
                 // When deleting the file for some reason path to all directories leading up to python are reported
                 // Skip those events
@@ -34,9 +38,5 @@ export function watchLocationForPythonBinaries(
             callback(type, e);
         }));
     }
-    return {
-        dispose: async () => {
-            disposables.forEach((d) => d.dispose());
-        },
-    };
+    return disposables;
 }
diff --git a/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts
index 5bb7e753a899..2ee12d19f3ca 100644
--- a/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts
+++ b/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts
@@ -7,12 +7,12 @@ import { traceVerbose } from '../../../../common/logger';
 import { FileChangeType } from '../../../../common/platform/fileSystemWatcher';
 import { chain, iterable, sleep } from '../../../../common/utils/async';
 import {
-    getEnvironmentVariable, getOSType, getUserHomeDir, OSType,
+    getEnvironmentVariable, getOSType, getUserHomeDir, OSType
 } from '../../../../common/utils/platform';
 import { PythonEnvInfo, PythonEnvKind, UNKNOWN_PYTHON_VERSION } from '../../../base/info';
 import { buildEnvInfo } from '../../../base/info/env';
 import { IPythonEnvsIterator } from '../../../base/locator';
-import { FSWatchingLocator } from '../../../base/locators/lowLevel/locator';
+import { FSWatchingLocator } from '../../../base/locators/lowLevel/fsWatchingLocator';
 import { findInterpretersInDir } from '../../../common/commonUtils';
 import { getFileInfo, pathExists } from '../../../common/externalDependencies';
 import { watchLocationForPythonBinaries } from '../../../common/pythonBinariesWatcher';
@@ -20,7 +20,7 @@ import { isPipenvEnvironment } from './pipEnvHelper';
 import {
     isVenvEnvironment,
     isVirtualenvEnvironment,
-    isVirtualenvwrapperEnvironment,
+    isVirtualenvwrapperEnvironment
 } from './virtualEnvironmentIdentifier';
 
 const DEFAULT_SEARCH_DEPTH = 2;
diff --git a/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts
index 1363918fe69b..59bbb762e8e4 100644
--- a/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts
+++ b/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts
@@ -11,7 +11,7 @@ import { PythonEnvInfo, PythonEnvKind } from '../../../base/info';
 import { buildEnvInfo } from '../../../base/info/env';
 import { getPythonVersionFromPath } from '../../../base/info/pythonVersion';
 import { IPythonEnvsIterator } from '../../../base/locator';
-import { FSWatchingLocator } from '../../../base/locators/lowLevel/locator';
+import { FSWatchingLocator } from '../../../base/locators/lowLevel/fsWatchingLocator';
 import { getFileInfo } from '../../../common/externalDependencies';
 import { watchLocationForPythonBinaries } from '../../../common/pythonBinariesWatcher';
 

From 31f7c543282d7ab0fda76c10f6b8bf3451b76ad4 Mon Sep 17 00:00:00 2001
From: Kartik Raj <karraj@microsoft.com>
Date: Wed, 4 Nov 2020 16:43:18 -0800
Subject: [PATCH 07/10] Modify FSWatching Locator

---
 src/client/pythonEnvironments/base/locator.ts |  2 +-
 .../locators/lowLevel/fsWatchingLocator.ts    | 70 ++++++++++++++++++-
 .../globalVirtualEnvronmentLocator.ts         | 35 +++-------
 .../locators/services/windowsStoreLocator.ts  | 23 ++----
 4 files changed, 85 insertions(+), 45 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locator.ts b/src/client/pythonEnvironments/base/locator.ts
index e021e286b07c..e7666eafbe76 100644
--- a/src/client/pythonEnvironments/base/locator.ts
+++ b/src/client/pythonEnvironments/base/locator.ts
@@ -175,7 +175,7 @@ interface IEmitter<E extends BasicPythonEnvsChangedEvent> {
  * By default `resolveEnv()` returns undefined.  Subclasses may override
  * the method to provide an implementation.
  *
- * Subclasses will call `this.emitter.fire()` * to emit events.
+ * Subclasses will call `this.emitter.fire()` to emit events.
  *
  * In most cases this is the class you will want to subclass.
  * Only in low-level cases should you consider subclassing `LocatorBase`
diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts
index 87602b59cee8..d17e064ea0b5 100644
--- a/src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts
@@ -1,6 +1,10 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+import { FileChangeType } from '../../../../common/platform/fileSystemWatcher';
+import { sleep } from '../../../../common/utils/async';
+import { watchLocationForPythonBinaries } from '../../../common/pythonBinariesWatcher';
+import { PythonEnvKind } from '../../info';
 import { Locator } from '../../locator';
 
 /**
@@ -10,5 +14,69 @@ import { Locator } from '../../locator';
  * Subclasses can call `this.emitter.fire()` * to emit events.
  */
 export abstract class FSWatchingLocator extends Locator {
-    public async abstract initialize(): Promise<void>;
+    private initialized = false;
+
+    constructor(
+        /**
+         * Location(s) to watch for python binaries.
+         */
+        private readonly getRoots: () => Promise<string[]> | string,
+        /**
+         * Returns the kind of environment specific to locator given the path to exectuable.
+         */
+        private readonly getKind: (executable: string) => Promise<PythonEnvKind>,
+        private readonly opts: {
+            /**
+             * Glob which represents basename of the executable to watch.
+             */
+            executableBaseGlob?: string,
+            /**
+             * Time to wait before attempting to identify kind when detected that an environment has been created.
+             */
+            delayOnCreated?: number, // milliseconds
+        } = {},
+    ) {
+        super();
+    }
+
+    public async initialize(): Promise<void> {
+        if (this.initialized) {
+            return;
+        }
+        this.initialized = true;
+        await this.startWatchers();
+    }
+
+    public dispose(): void {
+        super.dispose();
+        this.initialized = false;
+    }
+
+    private async startWatchers(): Promise<void> {
+        let roots = await this.getRoots();
+        if (typeof roots === 'string') {
+            roots = [roots];
+        }
+        roots.forEach((root) => this.startWatcher(root));
+    }
+
+    private startWatcher(root: string): void {
+        this.disposables.push(
+            watchLocationForPythonBinaries(
+                root,
+                async (type: FileChangeType, executable: string) => {
+                    if (type === FileChangeType.Created) {
+                        if (this.opts.delayOnCreated !== undefined) {
+                            // Note detecting kind of env depends on the file structure around the
+                            // executable, so we need to wait before attempting to detect it.
+                            await sleep(this.opts.delayOnCreated);
+                        }
+                    }
+                    const kind = await this.getKind(executable);
+                    this.emitter.fire({ type, kind });
+                },
+                this.opts.executableBaseGlob,
+            ),
+        );
+    }
 }
diff --git a/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts
index 2ee12d19f3ca..b7cdfbcd60cf 100644
--- a/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts
+++ b/src/client/pythonEnvironments/discovery/locators/services/globalVirtualEnvronmentLocator.ts
@@ -4,8 +4,7 @@
 import { uniq } from 'lodash';
 import * as path from 'path';
 import { traceVerbose } from '../../../../common/logger';
-import { FileChangeType } from '../../../../common/platform/fileSystemWatcher';
-import { chain, iterable, sleep } from '../../../../common/utils/async';
+import { chain, iterable } from '../../../../common/utils/async';
 import {
     getEnvironmentVariable, getOSType, getUserHomeDir, OSType
 } from '../../../../common/utils/platform';
@@ -15,7 +14,6 @@ import { IPythonEnvsIterator } from '../../../base/locator';
 import { FSWatchingLocator } from '../../../base/locators/lowLevel/fsWatchingLocator';
 import { findInterpretersInDir } from '../../../common/commonUtils';
 import { getFileInfo, pathExists } from '../../../common/externalDependencies';
-import { watchLocationForPythonBinaries } from '../../../common/pythonBinariesWatcher';
 import { isPipenvEnvironment } from './pipEnvHelper';
 import {
     isVenvEnvironment,
@@ -89,12 +87,14 @@ export class GlobalVirtualEnvironmentLocator extends FSWatchingLocator {
         PythonEnvKind.Pipenv,
     ];
 
-    public constructor(private readonly searchDepth?: number) {
-        super();
-    }
-
-    public async initialize(): Promise<void> {
-        await this.startWatchers();
+    constructor(private readonly searchDepth?: number) {
+        super(getGlobalVirtualEnvDirs, getVirtualEnvKind, {
+            // Note detecting kind of virtual env depends on the file structure around the
+            // executable, so we need to wait before attempting to detect it. However even
+            // if the type detected is incorrect, it doesn't do any practical harm as kinds
+            // in this locator are used in the same way (same activation commands etc.)
+            delayOnCreated: 1000,
+        });
     }
 
     public iterEnvs(): IPythonEnvsIterator {
@@ -171,21 +171,4 @@ export class GlobalVirtualEnvironmentLocator extends FSWatchingLocator {
         }
         return undefined;
     }
-
-    private async startWatchers(): Promise<void> {
-        const dirs = await getGlobalVirtualEnvDirs();
-        dirs.forEach(
-            (d) => this.disposables.push(
-                watchLocationForPythonBinaries(d, async (type: FileChangeType, executablePath: string) => {
-                    // Note detecting kind of virtual env depends on the file structure around the
-                    // executable, so we need to wait before attempting to detect it. However even
-                    // if the type detected is incorrect, it doesn't do any practical harm as kinds
-                    // in this locator are used in the same way (same activation commands etc.)
-                    await sleep(1000);
-                    const kind = await getVirtualEnvKind(executablePath);
-                    this.emitter.fire({ type, kind });
-                }),
-            ),
-        );
-    }
 }
diff --git a/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts b/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts
index 59bbb762e8e4..f4f2697df274 100644
--- a/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts
+++ b/src/client/pythonEnvironments/discovery/locators/services/windowsStoreLocator.ts
@@ -5,7 +5,6 @@ import * as fsapi from 'fs-extra';
 import * as minimatch from 'minimatch';
 import * as path from 'path';
 import { traceWarning } from '../../../../common/logger';
-import { FileChangeType } from '../../../../common/platform/fileSystemWatcher';
 import { Architecture, getEnvironmentVariable } from '../../../../common/utils/platform';
 import { PythonEnvInfo, PythonEnvKind } from '../../../base/info';
 import { buildEnvInfo } from '../../../base/info/env';
@@ -13,7 +12,6 @@ import { getPythonVersionFromPath } from '../../../base/info/pythonVersion';
 import { IPythonEnvsIterator } from '../../../base/locator';
 import { FSWatchingLocator } from '../../../base/locators/lowLevel/fsWatchingLocator';
 import { getFileInfo } from '../../../common/externalDependencies';
-import { watchLocationForPythonBinaries } from '../../../common/pythonBinariesWatcher';
 
 /**
  * Gets path to the Windows Apps directory.
@@ -143,8 +141,12 @@ export async function getWindowsStorePythonExes(): Promise<string[]> {
 export class WindowsStoreLocator extends FSWatchingLocator {
     private readonly kind: PythonEnvKind = PythonEnvKind.WindowsStore;
 
-    public async initialize(): Promise<void> {
-        this.startWatcher();
+    constructor() {
+        super(
+            getWindowsStoreAppsRoot,
+            async () => this.kind,
+            { executableBaseGlob: pythonExeGlob },
+        );
     }
 
     public iterEnvs(): IPythonEnvsIterator {
@@ -176,17 +178,4 @@ export class WindowsStoreLocator extends FSWatchingLocator {
         }
         return undefined;
     }
-
-    private startWatcher(): void {
-        const windowsAppsRoot = getWindowsStoreAppsRoot();
-        this.disposables.push(
-            watchLocationForPythonBinaries(
-                windowsAppsRoot,
-                (type: FileChangeType) => {
-                    this.emitter.fire({ type, kind: this.kind });
-                },
-                pythonExeGlob,
-            ),
-        );
-    }
 }

From fec8c1b7b76d4f7297b6e9d297d12c988311eeef Mon Sep 17 00:00:00 2001
From: Kartik Raj <karraj@microsoft.com>
Date: Thu, 5 Nov 2020 15:48:55 -0800
Subject: [PATCH 08/10] Code reviews

---
 src/client/common/syncDisposableRegistry.ts   |  3 ++
 src/client/pythonEnvironments/base/locator.ts | 30 ++++++++++++++-----
 .../locators/lowLevel/fsWatchingLocator.ts    |  2 +-
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/src/client/common/syncDisposableRegistry.ts b/src/client/common/syncDisposableRegistry.ts
index f0fe4bfac6f1..7ccf02a2bd79 100644
--- a/src/client/common/syncDisposableRegistry.ts
+++ b/src/client/common/syncDisposableRegistry.ts
@@ -6,6 +6,9 @@
 import { traceWarning } from './logger';
 import { IDisposable } from './types';
 
+/**
+ * Responsible for disposing a list of disposables synchronusly.
+ */
 export class DisposableRegistry implements IDisposable {
     private _list: IDisposable[] = [];
 
diff --git a/src/client/pythonEnvironments/base/locator.ts b/src/client/pythonEnvironments/base/locator.ts
index e7666eafbe76..0824f2130753 100644
--- a/src/client/pythonEnvironments/base/locator.ts
+++ b/src/client/pythonEnvironments/base/locator.ts
@@ -170,22 +170,18 @@ interface IEmitter<E extends BasicPythonEnvsChangedEvent> {
 }
 
 /**
- * The base for most Python envs locators.
+ * The generic base for Python envs locators.
  *
  * By default `resolveEnv()` returns undefined.  Subclasses may override
  * the method to provide an implementation.
  *
  * Subclasses will call `this.emitter.fire()` to emit events.
  *
- * In most cases this is the class you will want to subclass.
- * Only in low-level cases should you consider subclassing `LocatorBase`
- * using `BasicPythonEnvsChangedEvent.
- *
  * Also, in most cases the default event type (`PythonEnvsChangedEvent`)
  * should be used.  Only in low-level cases should you consider using
  * `BasicPythonEnvsChangedEvent`.
  */
-export abstract class Locator<E extends BasicPythonEnvsChangedEvent = PythonEnvsChangedEvent>
+abstract class LocatorBase<E extends BasicPythonEnvsChangedEvent = PythonEnvsChangedEvent>
 implements IDisposable, ILocator<E> {
     public readonly onChanged: Event<E>;
 
@@ -193,8 +189,8 @@ implements IDisposable, ILocator<E> {
 
     protected readonly disposables = new DisposableRegistry();
 
-    constructor() {
-        this.emitter = new PythonEnvsWatcher();
+    constructor(watcher: IPythonEnvsWatcher<E> & IEmitter<E>) {
+        this.emitter = watcher;
         this.onChanged = this.emitter.onChanged;
     }
 
@@ -208,3 +204,21 @@ implements IDisposable, ILocator<E> {
         this.disposables.dispose();
     }
 }
+
+/**
+ * The base for most Python envs locators.
+ *
+ * By default `resolveEnv()` returns undefined.  Subclasses may override
+ * the method to provide an implementation.
+ *
+ * Subclasses will call `this.emitter.fire()` * to emit events.
+ *
+ * In most cases this is the class you will want to subclass.
+ * Only in low-level cases should you consider subclassing `LocatorBase`
+ * using `BasicPythonEnvsChangedEvent.
+ */
+export abstract class Locator extends LocatorBase {
+    constructor() {
+        super(new PythonEnvsWatcher());
+    }
+}
diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts
index d17e064ea0b5..1b5dcbc88788 100644
--- a/src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts
@@ -31,7 +31,7 @@ export abstract class FSWatchingLocator extends Locator {
              */
             executableBaseGlob?: string,
             /**
-             * Time to wait before attempting to identify kind when detected that an environment has been created.
+             * Time to wait before handling an environment-created event.
              */
             delayOnCreated?: number, // milliseconds
         } = {},

From 21b740a28e43a66a1233b6206571e571df6b9421 Mon Sep 17 00:00:00 2001
From: Kartik Raj <karraj@microsoft.com>
Date: Thu, 5 Nov 2020 15:52:15 -0800
Subject: [PATCH 09/10] Use string[]

---
 .../base/locators/lowLevel/fsWatchingLocator.ts                 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts
index 1b5dcbc88788..bbdb78cfdd5f 100644
--- a/src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/lowLevel/fsWatchingLocator.ts
@@ -20,7 +20,7 @@ export abstract class FSWatchingLocator extends Locator {
         /**
          * Location(s) to watch for python binaries.
          */
-        private readonly getRoots: () => Promise<string[]> | string,
+        private readonly getRoots: () => Promise<string[]> | string | string[],
         /**
          * Returns the kind of environment specific to locator given the path to exectuable.
          */

From 2260b176ced0639d4727e64bec0a00b50388d8b3 Mon Sep 17 00:00:00 2001
From: Kartik Raj <karraj@microsoft.com>
Date: Thu, 5 Nov 2020 15:55:51 -0800
Subject: [PATCH 10/10] Remove list disposable getter

---
 src/client/common/syncDisposableRegistry.ts | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/src/client/common/syncDisposableRegistry.ts b/src/client/common/syncDisposableRegistry.ts
index 7ccf02a2bd79..bacffe51dccd 100644
--- a/src/client/common/syncDisposableRegistry.ts
+++ b/src/client/common/syncDisposableRegistry.ts
@@ -28,8 +28,4 @@ export class DisposableRegistry implements IDisposable {
             this._list.push(disposable);
         }
     }
-
-    public get list(): IDisposable[] {
-        return this._list;
-    }
 }