From 2b02bfc625299e9acba329e94327119af478e095 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Mon, 21 Sep 2020 16:43:15 -0600
Subject: [PATCH 01/39] Add a basic implementation of CachingLocator.

---
 .../base/locators/composite/cachingLocator.ts | 141 ++++++++++
 .../locators/composite/reducingLocator.ts     |  14 +
 src/test/pythonEnvironments/base/common.ts    |   4 +-
 .../composite/cachingLocator.unit.test.ts     | 258 ++++++++++++++++++
 4 files changed, 415 insertions(+), 2 deletions(-)
 create mode 100644 src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
 create mode 100644 src/client/pythonEnvironments/base/locators/composite/reducingLocator.ts
 create mode 100644 src/test/pythonEnvironments/base/locators/composite/cachingLocator.unit.test.ts

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
new file mode 100644
index 000000000000..b8ca55f049e1
--- /dev/null
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -0,0 +1,141 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import '../../../../common/extensions';
+import { createDeferred } from '../../../../common/utils/async';
+import { logWarning } from '../../../../logging';
+import { PythonEnvInfo } from '../../info';
+import { getMinimalPartialInfo } from '../../info/env';
+import {
+    ILocator,
+    IPythonEnvsIterator,
+    PythonLocatorQuery,
+} from '../../locator';
+import { getEnvs, getQueryFilter } from '../../locatorUtils';
+import { PythonEnvsWatcher } from '../../watcher';
+import { pickBestEnv } from './reducingLocator';
+
+// Note that we only export IEnvsCache for the sake of code navigation.
+
+/**
+ * The cache-related functionality used by CachingLocator.
+ */
+export interface IPythonEnvsCache {
+    initialize(): Promise<void>;
+    matchEnv(env: Partial<PythonEnvInfo>): PythonEnvInfo[];
+    getAllEnvs(): PythonEnvInfo[] | undefined;
+    setAllEnvs(envs: PythonEnvInfo[]): void;
+    flush(): Promise<void>;
+}
+
+/**
+ * A locator that stores the known environments in the given cache.
+ */
+export class CachingLocator extends PythonEnvsWatcher implements ILocator {
+    private readonly initializing = createDeferred<void>();
+
+    constructor(
+        private readonly cache: IPythonEnvsCache,
+        private readonly locator: ILocator,
+    ) {
+        super();
+        locator.onChanged((event) => {
+            this.refresh()
+                .then(() => this.fire(event))
+                .ignoreErrors();
+        });
+    }
+
+    /**
+     * Prepare the locator for use.
+     *
+     * This must be called before using the locator.  It is distinct
+     * from the constructor to avoid the problems that come from doing
+     * any serious work in constructors.  It also allows initialization
+     * to be asynchronous.
+     */
+    public async initialize(): Promise<void> {
+        await this.cache.initialize();
+        const envs = this.cache.getAllEnvs();
+        if (envs !== undefined) {
+            this.initializing.resolve();
+            await this.refresh();
+        } else {
+            // There is nothing in the cache, so we must wait for the
+            // initial refresh to finish before allowing iteration.
+            await this.refresh();
+            this.initializing.resolve();
+        }
+    }
+
+    public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator {
+        return this.iterFromCache(query);
+    }
+
+    public async resolveEnv(env: string | PythonEnvInfo): Promise<PythonEnvInfo | undefined> {
+        // If necessary we could be more aggressive about invalidating
+        // the cached value.
+        const query = getMinimalPartialInfo(env);
+        if (query === undefined) {
+            return undefined;
+        }
+        const candidates = this.cache.matchEnv(query);
+        if (candidates.length > 0) {
+            return pickBestEnv(candidates);
+        }
+        // Fall back to the underlying locator.
+        const resolved = await this.locator.resolveEnv(env);
+        if (resolved !== undefined) {
+            const envs = this.cache.getAllEnvs();
+            if (envs !== undefined) {
+                envs.push(resolved);
+                await this.update(envs);
+            }
+        }
+        return resolved;
+    }
+
+    private async* iterFromCache(query?: PythonLocatorQuery): IPythonEnvsIterator {
+        // XXX For now we wait for the initial refresh to finish...
+        await this.initializing.promise;
+
+        const envs = this.cache.getAllEnvs();
+        if (envs === undefined) {
+            logWarning('envs cache unexpectedly not initialized');
+            return;
+        }
+        if (await this.needsRefresh(envs)) {
+            // Refresh in the background.
+            this.refresh().ignoreErrors();
+        }
+        if (query !== undefined) {
+            const filter = getQueryFilter(query);
+            yield* envs.filter(filter);
+        } else {
+            yield* envs;
+        }
+    }
+
+    // eslint-disable-next-line class-methods-use-this,@typescript-eslint/no-unused-vars
+    private async needsRefresh(_envs: PythonEnvInfo[]): Promise<boolean> {
+        // XXX
+        // For now we never refresh.  Options:
+        // * every X minutes (via `initialize()`
+        // * if at least X minutes have elapsed
+        // * if some "stale" check on any known env fails
+        return false;
+    }
+
+    private async refresh(): Promise<void> {
+        const iterator = this.locator.iterEnvs();
+        const envs = await getEnvs(iterator);
+        await this.update(envs);
+    }
+
+    private async update(envs: PythonEnvInfo[]): Promise<void> {
+        // If necessary, we could skip if there are no changes.
+        this.cache.setAllEnvs(envs);
+        await this.cache.flush();
+        this.fire({}); // Emit an "onCHanged" event.
+    }
+}
diff --git a/src/client/pythonEnvironments/base/locators/composite/reducingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/reducingLocator.ts
new file mode 100644
index 000000000000..8dc71c28d699
--- /dev/null
+++ b/src/client/pythonEnvironments/base/locators/composite/reducingLocator.ts
@@ -0,0 +1,14 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import { PythonEnvInfo } from '../../info';
+
+/**
+ * Determine which of the given envs should be used.
+ *
+ * The candidates must be equivalent in some way.
+ */
+export function pickBestEnv(candidates: PythonEnvInfo[]): PythonEnvInfo {
+    // For the moment we take a naive approach.
+    return candidates[0];
+}
diff --git a/src/test/pythonEnvironments/base/common.ts b/src/test/pythonEnvironments/base/common.ts
index 62077e46aa20..fe3ddc1d1ff8 100644
--- a/src/test/pythonEnvironments/base/common.ts
+++ b/src/test/pythonEnvironments/base/common.ts
@@ -50,7 +50,7 @@ export class SimpleLocator extends Locator {
     private deferred = createDeferred<void>();
     constructor(
         private envs: PythonEnvInfo[],
-        private callbacks?: {
+        public callbacks: {
             resolve?: null | ((env: PythonEnvInfo) => Promise<PythonEnvInfo | undefined>);
             before?: Promise<void>;
             after?: Promise<void>;
@@ -58,7 +58,7 @@ export class SimpleLocator extends Locator {
             beforeEach?(e: PythonEnvInfo): Promise<void>;
             afterEach?(e: PythonEnvInfo): Promise<void>;
             onQuery?(query: PythonLocatorQuery | undefined, envs: PythonEnvInfo[]): Promise<PythonEnvInfo[]>;
-        }
+        } = {},
     ) {
         super();
     }
diff --git a/src/test/pythonEnvironments/base/locators/composite/cachingLocator.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/cachingLocator.unit.test.ts
new file mode 100644
index 000000000000..8f5dd8aee974
--- /dev/null
+++ b/src/test/pythonEnvironments/base/locators/composite/cachingLocator.unit.test.ts
@@ -0,0 +1,258 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import * as assert from 'assert';
+import * as path from 'path';
+import { Uri } from 'vscode';
+import { createDeferred } from '../../../../../client/common/utils/async';
+import { PythonEnvInfo, PythonEnvKind } from '../../../../../client/pythonEnvironments/base/info';
+import { areSameEnv } from '../../../../../client/pythonEnvironments/base/info/env';
+import { CachingLocator } from '../../../../../client/pythonEnvironments/base/locators/composite/cachingLocator';
+import { getEnvs } from '../../../../../client/pythonEnvironments/base/locatorUtils';
+import { PythonEnvsChangedEvent } from '../../../../../client/pythonEnvironments/base/watcher';
+import {
+    createLocatedEnv,
+    createNamedEnv,
+    SimpleLocator,
+} from '../../common';
+
+const env1 = createNamedEnv('env1', '2.7.11', PythonEnvKind.System, '/usr/bin/python');
+const env2 = createNamedEnv('env2', '3.8.1', PythonEnvKind.System, '/usr/bin/python3');
+const env3 = createLocatedEnv('/a/b/c/env5', '3.8.1', PythonEnvKind.Pipenv);
+env3.searchLocation = Uri.file(path.normalize('/a/b/c'));
+const env4 = createLocatedEnv('/x/y/z/env3', '2.7.11', PythonEnvKind.Venv);
+env4.searchLocation = Uri.file(path.normalize('/x/y/z'));
+const env5 = createLocatedEnv('/x/y/z/env4', '3.8.1', PythonEnvKind.Venv);
+env5.searchLocation = Uri.file(path.normalize('/x/y/z'));
+const envs = [env1, env2, env3, env4, env5];
+
+class FakeCache {
+    public initialized = false;
+
+    private envs: PythonEnvInfo[] | undefined;
+
+    constructor(
+        private readonly load: () => Promise<PythonEnvInfo[] | undefined>,
+        private readonly store: (e: PythonEnvInfo[]) => Promise<void>,
+    ) {}
+
+    public async initialize(): Promise<void> {
+        if (this.initialized) {
+            return;
+        }
+        this.envs = await this.load();
+        this.initialized = true;
+    }
+
+    public matchEnv(env: Partial<PythonEnvInfo>): PythonEnvInfo[] {
+        const cached = this.envs;
+        if (cached === undefined) {
+            return [];
+        }
+        return cached.filter((e) => areSameEnv(e, env));
+    }
+
+    public getAllEnvs(): PythonEnvInfo[] | undefined {
+        const cached = this.envs;
+        if (cached === undefined) {
+            return undefined;
+        }
+        return [...cached];
+    }
+
+    public setAllEnvs(setOfEnvs: PythonEnvInfo[]): void {
+        this.envs = [...setOfEnvs];
+    }
+
+    public async flush(): Promise<void> {
+        const cached = this.envs;
+        if (cached !== undefined) {
+            await this.store(cached);
+        }
+    }
+}
+
+async function getInitializedLocator(initialEnvs: PythonEnvInfo[]): Promise<[SimpleLocator, CachingLocator]> {
+    const cache = new FakeCache(
+        () => Promise.resolve(undefined),
+        () => Promise.resolve(undefined),
+    );
+    const subLocator = new SimpleLocator(initialEnvs, {
+        resolve: null,
+    });
+    const locator = new CachingLocator(cache, subLocator);
+    await locator.initialize();
+    return [subLocator, locator];
+}
+
+suite('Python envs locator - CachingLocator', () => {
+    suite('initialize', () => {
+        test('cache initialized', async () => {
+            const loadDeferred = createDeferred<void>();
+            const storeDeferred = createDeferred<void>();
+            let storedEnvs: PythonEnvInfo[] | undefined;
+            const cache = new FakeCache(
+                () => {
+                    const promise = Promise.resolve([env1]);
+                    promise.then(() => loadDeferred.resolve()).ignoreErrors();
+                    return promise;
+                },
+                async (e) => {
+                    storedEnvs = e;
+                    storeDeferred.resolve();
+                },
+            );
+            const subDeferred = createDeferred<void>();
+            const subLocator = new SimpleLocator([env2], {
+                before: (async () => {
+                    if (subDeferred.completed) {
+                        throw Error('called more than once!');
+                    }
+                    await subDeferred.promise;
+                })(),
+            });
+            const locator = new CachingLocator(cache, subLocator);
+
+            locator.initialize().ignoreErrors(); // in the background
+            await loadDeferred.promise; // This lets the load finish.
+            const resultBefore = await getEnvs(locator.iterEnvs());
+            subDeferred.resolve(); // This lets the refresh continue.
+            await storeDeferred.promise; // This lets the refresh finish.
+            const resultAfter = await getEnvs(locator.iterEnvs());
+
+            assert.deepEqual(storedEnvs, [env2]);
+            assert.deepEqual(resultBefore, [env1]);
+            assert.deepEqual(resultAfter, [env2]);
+        });
+    });
+
+    suite('onChanged', () => {
+        test('emitted after initial refresh', async () => {
+            const expected: PythonEnvsChangedEvent = {};
+            const cache = new FakeCache(
+                () => Promise.resolve(undefined),
+                () => Promise.resolve(undefined),
+            );
+            const subLocator = new SimpleLocator([env2]);
+            const locator = new CachingLocator(cache, subLocator);
+
+            let changeEvent: PythonEnvsChangedEvent | undefined;
+            locator.onChanged((e) => { changeEvent = e; });
+            await locator.initialize();
+
+            assert.deepEqual(changeEvent, expected);
+        });
+
+        test('propagated', async () => {
+            const expected: PythonEnvsChangedEvent = {};
+            const [subLocator, locator] = await getInitializedLocator([env2]);
+            let changeEvent: PythonEnvsChangedEvent | undefined;
+            const eventDeferred = createDeferred<void>();
+
+            locator.onChanged((e) => {
+                changeEvent = e;
+                eventDeferred.resolve();
+            });
+            subLocator.fire({});
+            await eventDeferred.promise;
+
+            assert.deepEqual(changeEvent, expected);
+        });
+    });
+
+    suite('iterEnvs()', () => {
+        test('no query', async () => {
+            const expected = envs;
+            const [, locator] = await getInitializedLocator(envs);
+
+            const iterator = locator.iterEnvs();
+            const discovered = await getEnvs(iterator);
+
+            assert.deepEqual(discovered, expected);
+        });
+
+        test('filter kinds', async () => {
+            const expected = [env1, env2, env4, env5];
+            const [, locator] = await getInitializedLocator(envs);
+            const query = {
+                kinds: [
+                    PythonEnvKind.Venv,
+                    PythonEnvKind.System,
+                ],
+            };
+
+            const iterator = locator.iterEnvs(query);
+            const discovered = await getEnvs(iterator);
+
+            assert.deepEqual(discovered, expected);
+        });
+
+        test('filter locations', async () => {
+            const expected = [env4, env5];
+            const query = {
+                searchLocations: {
+                    roots: [Uri.file(path.normalize('/x/y/z'))],
+                },
+            };
+            const [, locator] = await getInitializedLocator(envs);
+
+            const iterator = locator.iterEnvs(query);
+            const discovered = await getEnvs(iterator);
+
+            assert.deepEqual(discovered, expected);
+        });
+
+        test('cache empty', async () => {
+            const [, locator] = await getInitializedLocator([]);
+
+            const iterator = locator.iterEnvs();
+            const discovered = await getEnvs(iterator);
+
+            assert.deepEqual(discovered, []);
+        });
+    });
+
+    suite('resolveEnv()', () => {
+        test('full match in cache', async () => {
+            const expected = env5;
+            const [, locator] = await getInitializedLocator(envs);
+
+            const resolved = await locator.resolveEnv(env5);
+
+            assert.deepEqual(resolved, expected);
+        });
+
+        test('executable match in cache', async () => {
+            const expected = env5;
+            const [, locator] = await getInitializedLocator(envs);
+
+            const resolved = await locator.resolveEnv(env5.executable.filename);
+
+            assert.deepEqual(resolved, expected);
+        });
+
+        test('not in cache but found downstream', async () => {
+            const expected = env5;
+            const [subLocator, locator] = await getInitializedLocator([]);
+            subLocator.callbacks.resolve = () => Promise.resolve(env5);
+
+            const iterator1 = locator.iterEnvs();
+            const discoveredBefore = await getEnvs(iterator1);
+            const resolved = await locator.resolveEnv(env5);
+            const iterator2 = locator.iterEnvs();
+            const discoveredAfter = await getEnvs(iterator2);
+
+            assert.deepEqual(resolved, expected);
+            assert.deepEqual(discoveredBefore, []);
+            assert.deepEqual(discoveredAfter, [env5]);
+        });
+
+        test('not in cache nor downstream', async () => {
+            const [, locator] = await getInitializedLocator([]);
+
+            const resolved = await locator.resolveEnv(env5);
+
+            assert.equal(resolved, undefined);
+        });
+    });
+});

From 8fa4647967cdc573a052c04f91ad2b285cc7b8aa Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 23 Sep 2020 12:51:57 -0600
Subject: [PATCH 02/39] Add a noop envs cache implementation.

---
 src/client/pythonEnvironments/base/cache.ts | 36 +++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 src/client/pythonEnvironments/base/cache.ts

diff --git a/src/client/pythonEnvironments/base/cache.ts b/src/client/pythonEnvironments/base/cache.ts
new file mode 100644
index 000000000000..fb40d64f4c5c
--- /dev/null
+++ b/src/client/pythonEnvironments/base/cache.ts
@@ -0,0 +1,36 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import { PythonEnvInfo } from './info';
+import { IPythonEnvsCache } from './locators/composite/cachingLocator';
+
+/**
+ * A rudimentary empty cache.
+ */
+export class EmptyCache implements IPythonEnvsCache {
+    // tslint:disable-next-line: no-single-line-block-comment
+    /* eslint-disable class-methods-use-this */
+
+    public async initialize(): Promise<void> {
+        // Do nothing!
+    }
+
+    public getAllEnvs(): PythonEnvInfo[] | undefined {
+        return undefined;
+    }
+
+    public matchEnv(/* env: Partial<PythonEnvInfo> */): PythonEnvInfo[] {
+        return [];
+    }
+
+    public setAllEnvs(/* envs: PythonEnvInfo[] */): void {
+        // Do nothing!
+    }
+
+    public async flush(): Promise<void> {
+        // Do nothing!
+    }
+
+    // tslint:disable-next-line: no-single-line-block-comment
+    /* eslint-enable class-methods-use-this */
+}

From f7efae88ec03ebeb103e1e98f292e7d4c4218e67 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Mon, 21 Sep 2020 16:52:03 -0600
Subject: [PATCH 03/39] Use CachingLocator.

---
 src/client/pythonEnvironments/index.ts | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/src/client/pythonEnvironments/index.ts b/src/client/pythonEnvironments/index.ts
index 34089990d5fc..8d1235ddf942 100644
--- a/src/client/pythonEnvironments/index.ts
+++ b/src/client/pythonEnvironments/index.ts
@@ -3,9 +3,11 @@
 
 import * as vscode from 'vscode';
 import { IServiceContainer, IServiceManager } from '../ioc/types';
+import { EmptyCache } from './base/cache';
 import { PythonEnvInfoCache } from './base/envsCache';
 import { PythonEnvInfo } from './base/info';
 import { ILocator, IPythonEnvsIterator, PythonLocatorQuery } from './base/locator';
+import { CachingLocator } from './base/locators/composite/cachingLocator';
 import { PythonEnvsChangedEvent } from './base/watcher';
 import { ExtensionLocators, WorkspaceLocators } from './discovery/locators';
 import { registerForIOC } from './legacyIOC';
@@ -13,7 +15,7 @@ import { registerForIOC } from './legacyIOC';
 /**
  * Activate the Python environments component (during extension activation).'
  */
-export function activate(serviceManager: IServiceManager, serviceContainer: IServiceContainer) {
+export function activate(serviceManager: IServiceManager, serviceContainer: IServiceContainer): void {
     const [api, activateAPI] = createAPI();
     registerForIOC(serviceManager, serviceContainer, api);
     activateAPI();
@@ -27,7 +29,7 @@ export function activate(serviceManager: IServiceManager, serviceContainer: ISer
 export class PythonEnvironments implements ILocator {
     constructor(
         // These are the sub-components the full component is composed of:
-        private readonly locators: ILocator
+        private readonly locators: ILocator,
     ) {}
 
     public get onChanged(): vscode.Event<PythonEnvsChangedEvent> {
@@ -53,11 +55,15 @@ export function createAPI(): [PythonEnvironments, () => void] {
 
     // Update this to pass in an actual function that checks for env info completeness.
     const envsCache = new PythonEnvInfoCache(() => true);
+    // XXX For now we use a noop cache.
+    const cache = new EmptyCache();
+    const cachingLocator = new CachingLocator(cache, locators);
 
     return [
-        new PythonEnvironments(locators),
+        new PythonEnvironments(cachingLocator),
         () => {
             activateLocators();
+            cachingLocator.initialize().ignoreErrors();
             // Any other activation needed for the API will go here later.
             envsCache.initialize();
         },
@@ -81,7 +87,7 @@ function initLocators(): [ExtensionLocators, () => void] {
         () => {
             // Any non-workspace locator activation goes here.
             workspaceLocators.activate(getWorkspaceFolders());
-        }
+        },
     ];
 }
 
@@ -89,9 +95,11 @@ function getWorkspaceFolders() {
     const rootAdded = new vscode.EventEmitter<vscode.Uri>();
     const rootRemoved = new vscode.EventEmitter<vscode.Uri>();
     vscode.workspace.onDidChangeWorkspaceFolders((event) => {
+        // eslint-disable-next-line no-restricted-syntax
         for (const root of event.removed) {
             rootRemoved.fire(root.uri);
         }
+        // eslint-disable-next-line no-restricted-syntax
         for (const root of event.added) {
             rootAdded.fire(root.uri);
         }
@@ -100,6 +108,6 @@ function getWorkspaceFolders() {
     return {
         roots: folders ? folders.map((f) => f.uri) : [],
         onAdded: rootAdded.event,
-        onRemoved: rootRemoved.event
+        onRemoved: rootRemoved.event,
     };
 }

From 33b63ba6af6f731a247bde2b29a689c86ae70508 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Mon, 5 Oct 2020 12:58:14 -0600
Subject: [PATCH 04/39] Adjust PythonEnvInfoCache for use as a testing fake.

---
 .../pythonEnvironments/base/envsCache.ts      | 39 ++++++-----
 src/client/pythonEnvironments/index.ts        | 14 +++-
 .../base/envsCache.unit.test.ts               | 65 ++++++++++++-------
 3 files changed, 76 insertions(+), 42 deletions(-)

diff --git a/src/client/pythonEnvironments/base/envsCache.ts b/src/client/pythonEnvironments/base/envsCache.ts
index f0de585b3eff..4c763dc7b76d 100644
--- a/src/client/pythonEnvironments/base/envsCache.ts
+++ b/src/client/pythonEnvironments/base/envsCache.ts
@@ -2,7 +2,6 @@
 // Licensed under the MIT License.
 
 import { cloneDeep } from 'lodash';
-import { getGlobalPersistentStore, IPersistentStore } from '../common/externalDependencies';
 import { PythonEnvInfo } from './info';
 import { areSameEnv } from './info/env';
 
@@ -13,7 +12,7 @@ export interface IEnvsCache {
     /**
      * Initialization logic to be done outside of the constructor, for example reading from persistent storage.
      */
-    initialize(): void;
+    initialize(): Promise<void>;
 
     /**
      * Return all environment info currently in memory for this session.
@@ -40,7 +39,7 @@ export interface IEnvsCache {
      * @return The environment info objects matching the `env` param,
      * or `undefined` if the in-memory cache is not initialized.
      */
-    filterEnvs(env: PythonEnvInfo | string): PythonEnvInfo[] | undefined;
+    filterEnvs(query: Partial<PythonEnvInfo>): PythonEnvInfo[] | undefined;
 
     /**
      * Writes the content of the in-memory cache to persistent storage.
@@ -48,6 +47,11 @@ export interface IEnvsCache {
     flush(): Promise<void>;
 }
 
+export interface IPersistentStorage {
+    load(): Promise<PythonEnvInfo[] | undefined>;
+    store(envs: PythonEnvInfo[]): Promise<void>;
+}
+
 type CompleteEnvInfoFunction = (envInfo: PythonEnvInfo) => boolean;
 
 /**
@@ -58,18 +62,23 @@ export class PythonEnvInfoCache implements IEnvsCache {
 
     private envsList: PythonEnvInfo[] | undefined;
 
-    private persistentStorage: IPersistentStore<PythonEnvInfo[]> | undefined;
+    private persistentStorage: IPersistentStorage | undefined;
 
-    constructor(private readonly isComplete: CompleteEnvInfoFunction) {}
+    constructor(
+        private readonly isComplete: CompleteEnvInfoFunction,
+        private readonly getPersistentStorage?: () => IPersistentStorage,
+    ) {}
 
-    public initialize(): void {
+    public async initialize(): Promise<void> {
         if (this.initialized) {
             return;
         }
 
         this.initialized = true;
-        this.persistentStorage = getGlobalPersistentStore<PythonEnvInfo[]>('PYTHON_ENV_INFO_CACHE');
-        this.envsList = this.persistentStorage?.get();
+        if (this.getPersistentStorage !== undefined) {
+            this.persistentStorage = this.getPersistentStorage();
+            this.envsList = await this.persistentStorage.load();
+        }
     }
 
     public getAllEnvs(): PythonEnvInfo[] | undefined {
@@ -80,21 +89,19 @@ export class PythonEnvInfoCache implements IEnvsCache {
         this.envsList = cloneDeep(envs);
     }
 
-    public filterEnvs(env: PythonEnvInfo | string): PythonEnvInfo[] | undefined {
-        const result = this.envsList?.filter((info) => areSameEnv(info, env));
-
-        if (result) {
-            return cloneDeep(result);
+    public filterEnvs(query: Partial<PythonEnvInfo>): PythonEnvInfo[] | undefined {
+        if (this.envsList === undefined) {
+            return undefined;
         }
-
-        return undefined;
+        const result = this.envsList.filter((info) => areSameEnv(info, query));
+        return cloneDeep(result);
     }
 
     public async flush(): Promise<void> {
         const completeEnvs = this.envsList?.filter(this.isComplete);
 
         if (completeEnvs?.length) {
-            await this.persistentStorage?.set(completeEnvs);
+            await this.persistentStorage?.store(completeEnvs);
         }
     }
 }
diff --git a/src/client/pythonEnvironments/index.ts b/src/client/pythonEnvironments/index.ts
index 8d1235ddf942..8769b2638885 100644
--- a/src/client/pythonEnvironments/index.ts
+++ b/src/client/pythonEnvironments/index.ts
@@ -9,6 +9,7 @@ import { PythonEnvInfo } from './base/info';
 import { ILocator, IPythonEnvsIterator, PythonLocatorQuery } from './base/locator';
 import { CachingLocator } from './base/locators/composite/cachingLocator';
 import { PythonEnvsChangedEvent } from './base/watcher';
+import { getGlobalPersistentStore } from './common/externalDependencies';
 import { ExtensionLocators, WorkspaceLocators } from './discovery/locators';
 import { registerForIOC } from './legacyIOC';
 
@@ -54,7 +55,16 @@ export function createAPI(): [PythonEnvironments, () => void] {
     const [locators, activateLocators] = initLocators();
 
     // Update this to pass in an actual function that checks for env info completeness.
-    const envsCache = new PythonEnvInfoCache(() => true);
+    const envsCache = new PythonEnvInfoCache(
+        () => true, // "isComplete"
+        () => {
+            const storage = getGlobalPersistentStore<PythonEnvInfo[]>('PYTHON_ENV_INFO_CACHE');
+            return {
+                load: async () => storage.get(),
+                store: async (e) => storage.set(e),
+            };
+        },
+    );
     // XXX For now we use a noop cache.
     const cache = new EmptyCache();
     const cachingLocator = new CachingLocator(cache, locators);
@@ -65,7 +75,7 @@ export function createAPI(): [PythonEnvironments, () => void] {
             activateLocators();
             cachingLocator.initialize().ignoreErrors();
             // Any other activation needed for the API will go here later.
-            envsCache.initialize();
+            envsCache.initialize().ignoreErrors();
         },
     ];
 }
diff --git a/src/test/pythonEnvironments/base/envsCache.unit.test.ts b/src/test/pythonEnvironments/base/envsCache.unit.test.ts
index 530711063910..3ec81ff77e27 100644
--- a/src/test/pythonEnvironments/base/envsCache.unit.test.ts
+++ b/src/test/pythonEnvironments/base/envsCache.unit.test.ts
@@ -35,33 +35,41 @@ suite('Environment Info cache', () => {
         });
     });
 
+    function getGlobalPersistentStore() {
+        const store = externalDependencies.getGlobalPersistentStore<PythonEnvInfo[]>('PYTHON_ENV_INFO_CACHE');
+        return {
+            load: async () => store.get(),
+            store: (envs: PythonEnvInfo[]) => store.set(envs),
+        };
+    }
+
     teardown(() => {
         getGlobalPersistentStoreStub.restore();
         updatedValues = undefined;
     });
 
-    test('`initialize` reads from persistent storage', () => {
-        const envsCache = new PythonEnvInfoCache(allEnvsComplete);
+    test('`initialize` reads from persistent storage', async () => {
+        const envsCache = new PythonEnvInfoCache(allEnvsComplete, getGlobalPersistentStore);
 
-        envsCache.initialize();
+        await envsCache.initialize();
 
         assert.ok(getGlobalPersistentStoreStub.calledOnce);
     });
 
-    test('The in-memory env info array is undefined if there is no value in persistent storage when initializing the cache', () => {
-        const envsCache = new PythonEnvInfoCache(allEnvsComplete);
+    test('The in-memory env info array is undefined if there is no value in persistent storage when initializing the cache', async () => {
+        const envsCache = new PythonEnvInfoCache(allEnvsComplete, getGlobalPersistentStore);
 
         getGlobalPersistentStoreStub.returns({ get() { return undefined; } });
-        envsCache.initialize();
+        await envsCache.initialize();
         const result = envsCache.getAllEnvs();
 
         assert.strictEqual(result, undefined);
     });
 
-    test('`getAllEnvs` should return a deep copy of the environments currently in memory', () => {
-        const envsCache = new PythonEnvInfoCache(allEnvsComplete);
+    test('`getAllEnvs` should return a deep copy of the environments currently in memory', async () => {
+        const envsCache = new PythonEnvInfoCache(allEnvsComplete, getGlobalPersistentStore);
 
-        envsCache.initialize();
+        await envsCache.initialize();
         const envs = envsCache.getAllEnvs()!;
 
         envs[0].name = 'some-other-name';
@@ -70,7 +78,7 @@ suite('Environment Info cache', () => {
     });
 
     test('`getAllEnvs` should return undefined if nothing has been set', () => {
-        const envsCache = new PythonEnvInfoCache(allEnvsComplete);
+        const envsCache = new PythonEnvInfoCache(allEnvsComplete, getGlobalPersistentStore);
 
         const envs = envsCache.getAllEnvs();
 
@@ -78,7 +86,7 @@ suite('Environment Info cache', () => {
     });
 
     test('`setAllEnvs` should clone the environment info array passed as a parameter', () => {
-        const envsCache = new PythonEnvInfoCache(allEnvsComplete);
+        const envsCache = new PythonEnvInfoCache(allEnvsComplete, getGlobalPersistentStore);
 
         envsCache.setAllEnvs(envInfoArray);
         const envs = envsCache.getAllEnvs();
@@ -87,11 +95,11 @@ suite('Environment Info cache', () => {
         assert.strictEqual(envs === envInfoArray, false);
     });
 
-    test('`filterEnvs` should return environments that match its argument using areSameEnvironmnet', () => {
+    test('`filterEnvs` should return environments that match its argument using areSameEnvironmnet', async () => {
         const env:PythonEnvInfo = { executable: { filename: 'my-venv-env' } } as unknown as PythonEnvInfo;
-        const envsCache = new PythonEnvInfoCache(allEnvsComplete);
+        const envsCache = new PythonEnvInfoCache(allEnvsComplete, getGlobalPersistentStore);
 
-        envsCache.initialize();
+        await envsCache.initialize();
 
         const result = envsCache.filterEnvs(env);
 
@@ -105,7 +113,7 @@ suite('Environment Info cache', () => {
             kind: PythonEnvKind.System, executable: { filename: 'my-system-env' },
         } as unknown as PythonEnvInfo;
         const env:PythonEnvInfo = { executable: { filename: 'my-system-env' } } as unknown as PythonEnvInfo;
-        const envsCache = new PythonEnvInfoCache(allEnvsComplete);
+        const envsCache = new PythonEnvInfoCache(allEnvsComplete, getGlobalPersistentStore);
 
         envsCache.setAllEnvs([...envInfoArray, envToFind]);
 
@@ -115,11 +123,11 @@ suite('Environment Info cache', () => {
         assert.notDeepStrictEqual(result[0], envToFind);
     });
 
-    test('`filterEnvs` should return an empty array if no environment matches the properties of its argument', () => {
+    test('`filterEnvs` should return an empty array if no environment matches the properties of its argument', async () => {
         const env:PythonEnvInfo = { executable: { filename: 'my-nonexistent-env' } } as unknown as PythonEnvInfo;
-        const envsCache = new PythonEnvInfoCache(allEnvsComplete);
+        const envsCache = new PythonEnvInfoCache(allEnvsComplete, getGlobalPersistentStore);
 
-        envsCache.initialize();
+        await envsCache.initialize();
 
         const result = envsCache.filterEnvs(env);
 
@@ -128,7 +136,7 @@ suite('Environment Info cache', () => {
 
     test('`filterEnvs` should return undefined if the cache hasn\'t been initialized', () => {
         const env:PythonEnvInfo = { executable: { filename: 'my-nonexistent-env' } } as unknown as PythonEnvInfo;
-        const envsCache = new PythonEnvInfoCache(allEnvsComplete);
+        const envsCache = new PythonEnvInfoCache(allEnvsComplete, getGlobalPersistentStore);
 
         const result = envsCache.filterEnvs(env);
 
@@ -147,9 +155,12 @@ suite('Environment Info cache', () => {
         const expected = [
             otherEnv,
         ];
-        const envsCache = new PythonEnvInfoCache((env) => env.defaultDisplayName !== undefined);
+        const envsCache = new PythonEnvInfoCache(
+            (env) => env.defaultDisplayName !== undefined,
+            getGlobalPersistentStore,
+        );
 
-        envsCache.initialize();
+        await envsCache.initialize();
         envsCache.setAllEnvs(updatedEnvInfoArray);
         await envsCache.flush();
 
@@ -157,7 +168,10 @@ suite('Environment Info cache', () => {
     });
 
     test('`flush` should not write to persistent storage if there are no environment info objects in-memory', async () => {
-        const envsCache = new PythonEnvInfoCache((env) => env.kind === PythonEnvKind.MacDefault);
+        const envsCache = new PythonEnvInfoCache(
+            (env) => env.kind === PythonEnvKind.MacDefault,
+            getGlobalPersistentStore,
+        );
 
         await envsCache.flush();
 
@@ -165,9 +179,12 @@ suite('Environment Info cache', () => {
     });
 
     test('`flush` should not write to persistent storage if there are no complete environment info objects', async () => {
-        const envsCache = new PythonEnvInfoCache((env) => env.kind === PythonEnvKind.MacDefault);
+        const envsCache = new PythonEnvInfoCache(
+            (env) => env.kind === PythonEnvKind.MacDefault,
+            getGlobalPersistentStore,
+        );
 
-        envsCache.initialize();
+        await envsCache.initialize();
         await envsCache.flush();
 
         assert.strictEqual(updatedValues, undefined);

From 8220cea1c31d48379f3de2bcfdd66218a0b69e43 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Mon, 5 Oct 2020 12:59:08 -0600
Subject: [PATCH 05/39] Replace usage of EmptyCache with PythonEnvInfoCache.

---
 src/client/pythonEnvironments/base/cache.ts   | 36 -------------
 .../base/locators/composite/cachingLocator.ts | 21 +++-----
 src/client/pythonEnvironments/index.ts        |  6 +--
 .../composite/cachingLocator.unit.test.ts     | 50 +++----------------
 4 files changed, 15 insertions(+), 98 deletions(-)
 delete mode 100644 src/client/pythonEnvironments/base/cache.ts

diff --git a/src/client/pythonEnvironments/base/cache.ts b/src/client/pythonEnvironments/base/cache.ts
deleted file mode 100644
index fb40d64f4c5c..000000000000
--- a/src/client/pythonEnvironments/base/cache.ts
+++ /dev/null
@@ -1,36 +0,0 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.
-// Licensed under the MIT License.
-
-import { PythonEnvInfo } from './info';
-import { IPythonEnvsCache } from './locators/composite/cachingLocator';
-
-/**
- * A rudimentary empty cache.
- */
-export class EmptyCache implements IPythonEnvsCache {
-    // tslint:disable-next-line: no-single-line-block-comment
-    /* eslint-disable class-methods-use-this */
-
-    public async initialize(): Promise<void> {
-        // Do nothing!
-    }
-
-    public getAllEnvs(): PythonEnvInfo[] | undefined {
-        return undefined;
-    }
-
-    public matchEnv(/* env: Partial<PythonEnvInfo> */): PythonEnvInfo[] {
-        return [];
-    }
-
-    public setAllEnvs(/* envs: PythonEnvInfo[] */): void {
-        // Do nothing!
-    }
-
-    public async flush(): Promise<void> {
-        // Do nothing!
-    }
-
-    // tslint:disable-next-line: no-single-line-block-comment
-    /* eslint-enable class-methods-use-this */
-}
diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index b8ca55f049e1..56851ed18138 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -4,6 +4,7 @@
 import '../../../../common/extensions';
 import { createDeferred } from '../../../../common/utils/async';
 import { logWarning } from '../../../../logging';
+import { IEnvsCache } from '../../envsCache';
 import { PythonEnvInfo } from '../../info';
 import { getMinimalPartialInfo } from '../../info/env';
 import {
@@ -15,19 +16,6 @@ import { getEnvs, getQueryFilter } from '../../locatorUtils';
 import { PythonEnvsWatcher } from '../../watcher';
 import { pickBestEnv } from './reducingLocator';
 
-// Note that we only export IEnvsCache for the sake of code navigation.
-
-/**
- * The cache-related functionality used by CachingLocator.
- */
-export interface IPythonEnvsCache {
-    initialize(): Promise<void>;
-    matchEnv(env: Partial<PythonEnvInfo>): PythonEnvInfo[];
-    getAllEnvs(): PythonEnvInfo[] | undefined;
-    setAllEnvs(envs: PythonEnvInfo[]): void;
-    flush(): Promise<void>;
-}
-
 /**
  * A locator that stores the known environments in the given cache.
  */
@@ -35,7 +23,7 @@ export class CachingLocator extends PythonEnvsWatcher implements ILocator {
     private readonly initializing = createDeferred<void>();
 
     constructor(
-        private readonly cache: IPythonEnvsCache,
+        private readonly cache: IEnvsCache,
         private readonly locator: ILocator,
     ) {
         super();
@@ -79,7 +67,10 @@ export class CachingLocator extends PythonEnvsWatcher implements ILocator {
         if (query === undefined) {
             return undefined;
         }
-        const candidates = this.cache.matchEnv(query);
+        const candidates = this.cache.filterEnvs(query);
+        if (candidates === undefined) {
+            return undefined;
+        }
         if (candidates.length > 0) {
             return pickBestEnv(candidates);
         }
diff --git a/src/client/pythonEnvironments/index.ts b/src/client/pythonEnvironments/index.ts
index 8769b2638885..b67427bac4c6 100644
--- a/src/client/pythonEnvironments/index.ts
+++ b/src/client/pythonEnvironments/index.ts
@@ -3,7 +3,6 @@
 
 import * as vscode from 'vscode';
 import { IServiceContainer, IServiceManager } from '../ioc/types';
-import { EmptyCache } from './base/cache';
 import { PythonEnvInfoCache } from './base/envsCache';
 import { PythonEnvInfo } from './base/info';
 import { ILocator, IPythonEnvsIterator, PythonLocatorQuery } from './base/locator';
@@ -66,16 +65,15 @@ export function createAPI(): [PythonEnvironments, () => void] {
         },
     );
     // XXX For now we use a noop cache.
-    const cache = new EmptyCache();
-    const cachingLocator = new CachingLocator(cache, locators);
+    const cachingLocator = new CachingLocator(envsCache, locators);
 
     return [
         new PythonEnvironments(cachingLocator),
         () => {
             activateLocators();
+            envsCache.initialize().ignoreErrors();
             cachingLocator.initialize().ignoreErrors();
             // Any other activation needed for the API will go here later.
-            envsCache.initialize().ignoreErrors();
         },
     ];
 }
diff --git a/src/test/pythonEnvironments/base/locators/composite/cachingLocator.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/cachingLocator.unit.test.ts
index 8f5dd8aee974..e4051b390dca 100644
--- a/src/test/pythonEnvironments/base/locators/composite/cachingLocator.unit.test.ts
+++ b/src/test/pythonEnvironments/base/locators/composite/cachingLocator.unit.test.ts
@@ -5,8 +5,8 @@ import * as assert from 'assert';
 import * as path from 'path';
 import { Uri } from 'vscode';
 import { createDeferred } from '../../../../../client/common/utils/async';
+import { PythonEnvInfoCache } from '../../../../../client/pythonEnvironments/base/envsCache';
 import { PythonEnvInfo, PythonEnvKind } from '../../../../../client/pythonEnvironments/base/info';
-import { areSameEnv } from '../../../../../client/pythonEnvironments/base/info/env';
 import { CachingLocator } from '../../../../../client/pythonEnvironments/base/locators/composite/cachingLocator';
 import { getEnvs } from '../../../../../client/pythonEnvironments/base/locatorUtils';
 import { PythonEnvsChangedEvent } from '../../../../../client/pythonEnvironments/base/watcher';
@@ -26,49 +26,13 @@ const env5 = createLocatedEnv('/x/y/z/env4', '3.8.1', PythonEnvKind.Venv);
 env5.searchLocation = Uri.file(path.normalize('/x/y/z'));
 const envs = [env1, env2, env3, env4, env5];
 
-class FakeCache {
-    public initialized = false;
-
-    private envs: PythonEnvInfo[] | undefined;
-
+class FakeCache extends PythonEnvInfoCache {
     constructor(
-        private readonly load: () => Promise<PythonEnvInfo[] | undefined>,
-        private readonly store: (e: PythonEnvInfo[]) => Promise<void>,
-    ) {}
-
-    public async initialize(): Promise<void> {
-        if (this.initialized) {
-            return;
-        }
-        this.envs = await this.load();
-        this.initialized = true;
-    }
-
-    public matchEnv(env: Partial<PythonEnvInfo>): PythonEnvInfo[] {
-        const cached = this.envs;
-        if (cached === undefined) {
-            return [];
-        }
-        return cached.filter((e) => areSameEnv(e, env));
-    }
-
-    public getAllEnvs(): PythonEnvInfo[] | undefined {
-        const cached = this.envs;
-        if (cached === undefined) {
-            return undefined;
-        }
-        return [...cached];
-    }
-
-    public setAllEnvs(setOfEnvs: PythonEnvInfo[]): void {
-        this.envs = [...setOfEnvs];
-    }
-
-    public async flush(): Promise<void> {
-        const cached = this.envs;
-        if (cached !== undefined) {
-            await this.store(cached);
-        }
+        load: () => Promise<PythonEnvInfo[] | undefined>,
+        store: (e: PythonEnvInfo[]) => Promise<void>,
+        isComplete: (e: PythonEnvInfo) => boolean = () => true,
+    ) {
+        super(isComplete, () => ({ load, store }));
     }
 }
 

From 7350eb2d273365b3c2b43106383b9aabdd928eb2 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Tue, 29 Sep 2020 17:07:25 -0600
Subject: [PATCH 06/39] Make CachingLocator.initialize() idempotent.

---
 .../base/locators/composite/cachingLocator.ts              | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 56851ed18138..93a1bfd2130f 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -22,6 +22,8 @@ import { pickBestEnv } from './reducingLocator';
 export class CachingLocator extends PythonEnvsWatcher implements ILocator {
     private readonly initializing = createDeferred<void>();
 
+    private initialized = false;
+
     constructor(
         private readonly cache: IEnvsCache,
         private readonly locator: ILocator,
@@ -43,6 +45,11 @@ export class CachingLocator extends PythonEnvsWatcher implements ILocator {
      * to be asynchronous.
      */
     public async initialize(): Promise<void> {
+        if (this.initialized) {
+            return;
+        }
+        this.initialized = true;
+
         await this.cache.initialize();
         const envs = this.cache.getAllEnvs();
         if (envs !== undefined) {

From c328bd6fb02cde4e895d389888b619d8aded9559 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Tue, 29 Sep 2020 17:09:30 -0600
Subject: [PATCH 07/39] Move the onChanged hook to initialize().

---
 .../base/locators/composite/cachingLocator.ts       | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 93a1bfd2130f..d78855f10b2c 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -29,11 +29,6 @@ export class CachingLocator extends PythonEnvsWatcher implements ILocator {
         private readonly locator: ILocator,
     ) {
         super();
-        locator.onChanged((event) => {
-            this.refresh()
-                .then(() => this.fire(event))
-                .ignoreErrors();
-        });
     }
 
     /**
@@ -51,6 +46,7 @@ export class CachingLocator extends PythonEnvsWatcher implements ILocator {
         this.initialized = true;
 
         await this.cache.initialize();
+
         const envs = this.cache.getAllEnvs();
         if (envs !== undefined) {
             this.initializing.resolve();
@@ -61,6 +57,13 @@ export class CachingLocator extends PythonEnvsWatcher implements ILocator {
             await this.refresh();
             this.initializing.resolve();
         }
+
+        this.locator.onChanged((event) => {
+            // We could be a little smarter about when we refresh.
+            this.refresh()
+                .then(() => this.fire(event)) // XXX Drop this line?
+                .ignoreErrors();
+        });
     }
 
     public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator {

From 0a442e31d3f02ec3e6a5ed69320001ac4b055209 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Tue, 29 Sep 2020 17:17:40 -0600
Subject: [PATCH 08/39] Pass the change event through to refresh().

---
 .../base/locators/composite/cachingLocator.ts | 23 ++++++++++++-------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index d78855f10b2c..9384a74d37f2 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -13,7 +13,7 @@ import {
     PythonLocatorQuery,
 } from '../../locator';
 import { getEnvs, getQueryFilter } from '../../locatorUtils';
-import { PythonEnvsWatcher } from '../../watcher';
+import { PythonEnvsChangedEvent, PythonEnvsWatcher } from '../../watcher';
 import { pickBestEnv } from './reducingLocator';
 
 /**
@@ -60,9 +60,7 @@ export class CachingLocator extends PythonEnvsWatcher implements ILocator {
 
         this.locator.onChanged((event) => {
             // We could be a little smarter about when we refresh.
-            this.refresh()
-                .then(() => this.fire(event)) // XXX Drop this line?
-                .ignoreErrors();
+            this.refresh({ event }).ignoreErrors();
         });
     }
 
@@ -127,16 +125,25 @@ export class CachingLocator extends PythonEnvsWatcher implements ILocator {
         return false;
     }
 
-    private async refresh(): Promise<void> {
+    private async refresh(
+        opts: {
+            event?: PythonEnvsChangedEvent;
+        } = {},
+    ): Promise<void> {
         const iterator = this.locator.iterEnvs();
         const envs = await getEnvs(iterator);
-        await this.update(envs);
+        await this.update(envs, opts);
     }
 
-    private async update(envs: PythonEnvInfo[]): Promise<void> {
+    private async update(
+        envs: PythonEnvInfo[],
+        opts: {
+            event?: PythonEnvsChangedEvent;
+        } = {},
+    ): Promise<void> {
         // If necessary, we could skip if there are no changes.
         this.cache.setAllEnvs(envs);
         await this.cache.flush();
-        this.fire({}); // Emit an "onCHanged" event.
+        this.fire(opts.event || {}); // Emit an "onCHanged" event.
     }
 }

From 8642594f82232c023b642706e751d949806d3da4 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Tue, 29 Sep 2020 17:21:22 -0600
Subject: [PATCH 09/39] Do not inherit from PythonEnvsWatcher.

---
 .../base/locators/composite/cachingLocator.ts         | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 9384a74d37f2..0b05c27f3557 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+import { Event } from 'vscode';
 import '../../../../common/extensions';
 import { createDeferred } from '../../../../common/utils/async';
 import { logWarning } from '../../../../logging';
@@ -19,7 +20,11 @@ import { pickBestEnv } from './reducingLocator';
 /**
  * A locator that stores the known environments in the given cache.
  */
-export class CachingLocator extends PythonEnvsWatcher implements ILocator {
+export class CachingLocator implements ILocator {
+    public readonly onChanged: Event<PythonEnvsChangedEvent>;
+
+    private readonly watcher = new PythonEnvsWatcher();
+
     private readonly initializing = createDeferred<void>();
 
     private initialized = false;
@@ -28,7 +33,7 @@ export class CachingLocator extends PythonEnvsWatcher implements ILocator {
         private readonly cache: IEnvsCache,
         private readonly locator: ILocator,
     ) {
-        super();
+        this.onChanged = this.watcher.onChanged;
     }
 
     /**
@@ -144,6 +149,6 @@ export class CachingLocator extends PythonEnvsWatcher implements ILocator {
         // If necessary, we could skip if there are no changes.
         this.cache.setAllEnvs(envs);
         await this.cache.flush();
-        this.fire(opts.event || {}); // Emit an "onCHanged" event.
+        this.watcher.fire(opts.event || {}); // Emit an "onCHanged" event.
     }
 }

From e778d7365807e2b90567597263d2ad44452984ab Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Tue, 29 Sep 2020 17:22:41 -0600
Subject: [PATCH 10/39] Add CachingLocator.dispose().

---
 .../base/locators/composite/cachingLocator.ts               | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 0b05c27f3557..58f2d8f2a93d 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -29,6 +29,8 @@ export class CachingLocator implements ILocator {
 
     private initialized = false;
 
+    private done = createDeferred<void>();
+
     constructor(
         private readonly cache: IEnvsCache,
         private readonly locator: ILocator,
@@ -69,6 +71,10 @@ export class CachingLocator implements ILocator {
         });
     }
 
+    public async dispose(): Promise<void> {
+        this.done.resolve();
+    }
+
     public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator {
         return this.iterFromCache(query);
     }

From 0c6925abb3730b41ebc5d3e25aa61fcfea07b2a0 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Tue, 29 Sep 2020 17:28:03 -0600
Subject: [PATCH 11/39] Factor out initialRefresh().

---
 .../base/locators/composite/cachingLocator.ts | 49 ++++++++++---------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 58f2d8f2a93d..74d79bd72ee1 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -29,7 +29,7 @@ export class CachingLocator implements ILocator {
 
     private initialized = false;
 
-    private done = createDeferred<void>();
+    private readonly done = createDeferred<void>();
 
     constructor(
         private readonly cache: IEnvsCache,
@@ -53,20 +53,8 @@ export class CachingLocator implements ILocator {
         this.initialized = true;
 
         await this.cache.initialize();
-
-        const envs = this.cache.getAllEnvs();
-        if (envs !== undefined) {
-            this.initializing.resolve();
-            await this.refresh();
-        } else {
-            // There is nothing in the cache, so we must wait for the
-            // initial refresh to finish before allowing iteration.
-            await this.refresh();
-            this.initializing.resolve();
-        }
-
+        await this.initialRefresh();
         this.locator.onChanged((event) => {
-            // We could be a little smarter about when we refresh.
             this.refresh({ event }).ignoreErrors();
         });
     }
@@ -126,16 +114,6 @@ export class CachingLocator implements ILocator {
         }
     }
 
-    // eslint-disable-next-line class-methods-use-this,@typescript-eslint/no-unused-vars
-    private async needsRefresh(_envs: PythonEnvInfo[]): Promise<boolean> {
-        // XXX
-        // For now we never refresh.  Options:
-        // * every X minutes (via `initialize()`
-        // * if at least X minutes have elapsed
-        // * if some "stale" check on any known env fails
-        return false;
-    }
-
     private async refresh(
         opts: {
             event?: PythonEnvsChangedEvent;
@@ -157,4 +135,27 @@ export class CachingLocator implements ILocator {
         await this.cache.flush();
         this.watcher.fire(opts.event || {}); // Emit an "onCHanged" event.
     }
+
+    private async initialRefresh(): Promise<void> {
+        const envs = this.cache.getAllEnvs();
+        if (envs !== undefined) {
+            this.initializing.resolve();
+            await this.refresh();
+        } else {
+            // There is nothing in the cache, so we must wait for the
+            // initial refresh to finish before allowing iteration.
+            await this.refresh();
+            this.initializing.resolve();
+        }
+    }
+
+    // eslint-disable-next-line class-methods-use-this,@typescript-eslint/no-unused-vars
+    private async needsRefresh(_envs: PythonEnvInfo[]): Promise<boolean> {
+        // XXX
+        // For now we never refresh.  Options:
+        // * every X minutes (via `initialize()`
+        // * if at least X minutes have elapsed
+        // * if some "stale" check on any known env fails
+        return false;
+    }
 }

From a9730358d82cf501fed3874c8ab37abbeb1e1c5d Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 30 Sep 2020 09:18:40 -0600
Subject: [PATCH 12/39] Add BackgroundLooper.

---
 .../base/locators/composite/cachingLocator.ts | 109 ++++++++++++++++++
 1 file changed, 109 insertions(+)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 74d79bd72ee1..32ed612a7c77 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -1,6 +1,9 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+// tslint:disable-next-line: no-single-line-block-comment
+/* eslint-disable max-classes-per-file */
+
 import { Event } from 'vscode';
 import '../../../../common/extensions';
 import { createDeferred } from '../../../../common/utils/async';
@@ -159,3 +162,109 @@ export class CachingLocator implements ILocator {
         return false;
     }
 }
+
+type RequestID = number;
+type NotifyFunc = () => void;
+
+export class BackgroundLooper {
+    private started = false;
+
+    private stopped = false;
+
+    private readonly done = createDeferred<void>();
+
+    private readonly loopRunning = createDeferred<void>();
+
+    private waitUntilReady= createDeferred<void>();
+
+    private readonly queue: RequestID[] = [];
+
+    private readonly requests: Record<RequestID, [Promise<void>, NotifyFunc]> = {};
+
+    private lastID: number | undefined;
+
+    constructor(
+        private readonly run: (id: RequestID) => Promise<void>,
+    ) {}
+
+    public start(): void {
+        if (this.stopped) {
+            throw Error('already stopped');
+        }
+        if (this.started) {
+            return;
+        }
+        this.started = true;
+
+        this.runLoop().ignoreErrors();
+    }
+
+    public stop(): void {
+        if (this.stopped) {
+            return;
+        }
+        if (!this.started) {
+            throw Error('not started yet');
+        }
+        this.stopped = true;
+
+        this.done.resolve();
+    }
+
+    public async wait(): Promise<void> {
+        // XXX Fail if not started yet?
+        await this.loopRunning;
+    }
+
+    public getID(opts: { changed?: boolean } = {}): RequestID {
+        const changed = opts.changed === undefined ? true : opts.changed;
+        const lastID = this.lastID === undefined ? -1 : this.lastID;
+        let id = lastID + 1;
+        if (!changed && this.lastID !== undefined && this.queue.length > 0) {
+            id = lastID;
+        }
+        this.lastID = id;
+        return id;
+    }
+
+    public addRequest(id: RequestID): Promise<void> {
+        const req = this.requests[id];
+        if (req !== undefined) {
+            // eslint-disable-next-line comma-dangle,comma-spacing
+            const [promise,] = req;
+            return promise;
+        }
+
+        const running = createDeferred<void>();
+        this.requests[id] = [running.promise, () => running.resolve()];
+        this.queue.push(id);
+        this.waitUntilReady.resolve();
+        return running.promise;
+    }
+
+    private async runLoop(): Promise<void> {
+        await Promise.race([
+            this.waitUntilReady.promise,
+            this.done,
+        ]);
+        this.waitUntilReady = createDeferred<void>();
+        while (!this.done.completed) {
+            while (this.queue.length > 0) {
+                const id = this.queue[0];
+                this.queue.shift();
+                const [, notify] = this.requests[id];
+                // eslint-disable-next-line no-await-in-loop
+                await this.run(id);
+                // XXX retries?
+                delete this.requests[id];
+                notify();
+            }
+            // eslint-disable-next-line no-await-in-loop
+            await Promise.race([
+                this.waitUntilReady,
+                this.done,
+            ]);
+        }
+        this.loopRunning.resolve();
+    }
+}

From b87ec6b5a72cda814005eaea76d45267c46d0dd5 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 30 Sep 2020 08:45:30 -0600
Subject: [PATCH 13/39] Only run a single refresh operation at a time.

---
 .../base/locators/composite/cachingLocator.ts | 41 +++++++++++++------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 32ed612a7c77..dfa3d69c6da2 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -32,13 +32,21 @@ export class CachingLocator implements ILocator {
 
     private initialized = false;
 
-    private readonly done = createDeferred<void>();
+    private looper: BackgroundLooper;
+
+    private requests: Record<RequestID, PythonEnvsChangedEvent | undefined> = {};
 
     constructor(
         private readonly cache: IEnvsCache,
         private readonly locator: ILocator,
     ) {
         this.onChanged = this.watcher.onChanged;
+        // eslint-disable-next-line @typescript-eslint/no-use-before-define
+        this.looper = new BackgroundLooper(async (id) => {
+            const event = this.requests[id];
+            await this.doRefresh(event);
+            delete this.requests[id];
+        });
     }
 
     /**
@@ -53,17 +61,18 @@ export class CachingLocator implements ILocator {
         if (this.initialized) {
             return;
         }
-        this.initialized = true;
 
         await this.cache.initialize();
+        this.looper.start();
         await this.initialRefresh();
         this.locator.onChanged((event) => {
-            this.refresh({ event }).ignoreErrors();
+            this.refresh(event).ignoreErrors();
         });
     }
 
     public async dispose(): Promise<void> {
-        this.done.resolve();
+        this.looper.stop();
+        await this.looper.wait();
     }
 
     public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator {
@@ -118,25 +127,31 @@ export class CachingLocator implements ILocator {
     }
 
     private async refresh(
-        opts: {
-            event?: PythonEnvsChangedEvent;
-        } = {},
+        event?: PythonEnvsChangedEvent,
+    ): Promise<void> {
+        const id = this.looper.getID({ changed: !!event });
+        if (this.requests[id] === undefined) {
+            this.requests[id] = event;
+        }
+        return this.looper.addRequest(id);
+    }
+
+    private async doRefresh(
+        event: PythonEnvsChangedEvent | undefined,
     ): Promise<void> {
         const iterator = this.locator.iterEnvs();
         const envs = await getEnvs(iterator);
-        await this.update(envs, opts);
+        await this.update(envs, event);
     }
 
     private async update(
         envs: PythonEnvInfo[],
-        opts: {
-            event?: PythonEnvsChangedEvent;
-        } = {},
+        event?: PythonEnvsChangedEvent,
     ): Promise<void> {
         // If necessary, we could skip if there are no changes.
         this.cache.setAllEnvs(envs);
         await this.cache.flush();
-        this.watcher.fire(opts.event || {}); // Emit an "onCHanged" event.
+        this.watcher.fire(event || {}); // Emit an "onCHanged" event.
     }
 
     private async initialRefresh(): Promise<void> {
@@ -166,7 +181,7 @@ export class CachingLocator implements ILocator {
 type RequestID = number;
 type NotifyFunc = () => void;
 
-export class BackgroundLooper {
+class BackgroundLooper {
     private started = false;
 
     private stopped = false;

From bf39d81dc5b040bc23d869f73fbe51c901e60c3d Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 30 Sep 2020 09:08:13 -0600
Subject: [PATCH 14/39] Simplify BackgroundLooper.

---
 .../base/locators/composite/cachingLocator.ts | 81 +++++++++----------
 1 file changed, 37 insertions(+), 44 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index dfa3d69c6da2..9cc885aead5b 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -32,21 +32,14 @@ export class CachingLocator implements ILocator {
 
     private initialized = false;
 
-    private looper: BackgroundLooper;
-
-    private requests: Record<RequestID, PythonEnvsChangedEvent | undefined> = {};
+    // eslint-disable-next-line @typescript-eslint/no-use-before-define
+    private looper = new BackgroundLooper();
 
     constructor(
         private readonly cache: IEnvsCache,
         private readonly locator: ILocator,
     ) {
         this.onChanged = this.watcher.onChanged;
-        // eslint-disable-next-line @typescript-eslint/no-use-before-define
-        this.looper = new BackgroundLooper(async (id) => {
-            const event = this.requests[id];
-            await this.doRefresh(event);
-            delete this.requests[id];
-        });
     }
 
     /**
@@ -70,9 +63,9 @@ export class CachingLocator implements ILocator {
         });
     }
 
-    public async dispose(): Promise<void> {
-        this.looper.stop();
-        await this.looper.wait();
+    public dispose(): void {
+        const waitUntilStopped = this.looper.stop();
+        waitUntilStopped.ignoreErrors();
     }
 
     public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator {
@@ -130,18 +123,11 @@ export class CachingLocator implements ILocator {
         event?: PythonEnvsChangedEvent,
     ): Promise<void> {
         const id = this.looper.getID({ changed: !!event });
-        if (this.requests[id] === undefined) {
-            this.requests[id] = event;
-        }
-        return this.looper.addRequest(id);
-    }
-
-    private async doRefresh(
-        event: PythonEnvsChangedEvent | undefined,
-    ): Promise<void> {
-        const iterator = this.locator.iterEnvs();
-        const envs = await getEnvs(iterator);
-        await this.update(envs, event);
+        return this.looper.addRequest(id, async () => {
+            const iterator = this.locator.iterEnvs();
+            const envs = await getEnvs(iterator);
+            await this.update(envs, event);
+        });
     }
 
     private async update(
@@ -179,6 +165,7 @@ export class CachingLocator implements ILocator {
 }
 
 type RequestID = number;
+type RunFunc = () => Promise<void>;
 type NotifyFunc = () => void;
 
 class BackgroundLooper {
@@ -194,14 +181,10 @@ class BackgroundLooper {
 
     private readonly queue: RequestID[] = [];
 
-    private readonly requests: Record<RequestID, [Promise<void>, NotifyFunc]> = {};
+    private readonly requests: Record<RequestID, [RunFunc, Promise<void>, NotifyFunc]> = {};
 
     private lastID: number | undefined;
 
-    constructor(
-        private readonly run: (id: RequestID) => Promise<void>,
-    ) {}
-
     public start(): void {
         if (this.stopped) {
             throw Error('already stopped');
@@ -214,9 +197,9 @@ class BackgroundLooper {
         this.runLoop().ignoreErrors();
     }
 
-    public stop(): void {
+    public stop(): Promise<void> {
         if (this.stopped) {
-            return;
+            return this.loopRunning.promise;
         }
         if (!this.started) {
             throw Error('not started yet');
@@ -224,11 +207,14 @@ class BackgroundLooper {
         this.stopped = true;
 
         this.done.resolve();
-    }
 
-    public async wait(): Promise<void> {
-        // XXX Fail if not started yet?
-        await this.loopRunning;
+        // It is conceivable that a separate "waitUntilStopped"
+        // operation would be useful.  If it turned out to be desireable
+        // then at the point we could add such a method separately.
+        // It would do nothing more than `await this.loopRunning`.
+        // Currently there is no need for a separate method since
+        // returning the promise here is sufficient.
+        return this.loopRunning.promise;
     }
 
     public getID(opts: { changed?: boolean } = {}): RequestID {
@@ -242,34 +228,41 @@ class BackgroundLooper {
         return id;
     }
 
-    public addRequest(id: RequestID): Promise<void> {
+    public addRequest(
+        id: RequestID,
+        run: RunFunc,
+    ): Promise<void> {
         const req = this.requests[id];
         if (req !== undefined) {
             // eslint-disable-next-line comma-dangle,comma-spacing
-            const [promise,] = req;
+            const [, promise,] = req;
             return promise;
         }
 
         const running = createDeferred<void>();
-        this.requests[id] = [running.promise, () => running.resolve()];
+        this.requests[id] = [run, running.promise, () => running.resolve()];
         this.queue.push(id);
+        // `waitUntilReady` will get replaced with a new deferred in
+        // the loop once the existing one gets used.
         this.waitUntilReady.resolve();
         return running.promise;
     }
 
     private async runLoop(): Promise<void> {
-        await Promise.race([
-            this.waitUntilReady.promise,
-            this.done,
+        const winner = await Promise.race([
+            this.done.promise.then(() => 0),
+            this.waitUntilReady.promise.then(() => 1),
         ]);
-        this.waitUntilReady = createDeferred<void>();
+        if (winner === 1) {
+            this.waitUntilReady = createDeferred<void>();
+        }
         while (!this.done.completed) {
             while (this.queue.length > 0) {
                 const id = this.queue[0];
                 this.queue.shift();
-                const [, notify] = this.requests[id];
+                const [run, , notify] = this.requests[id];
                 // eslint-disable-next-line no-await-in-loop
-                await this.run(id);
+                await run();
                 // XXX retries?
                 delete this.requests[id];
                 notify();

From 5ad921de1e4a81938f3e3478563908f88ea07d26 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 30 Sep 2020 10:30:46 -0600
Subject: [PATCH 15/39] Add support for retries.

---
 .../base/locators/composite/cachingLocator.ts | 78 +++++++++++++++++--
 1 file changed, 72 insertions(+), 6 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 9cc885aead5b..bad869fea568 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -6,7 +6,7 @@
 
 import { Event } from 'vscode';
 import '../../../../common/extensions';
-import { createDeferred } from '../../../../common/utils/async';
+import { createDeferred, sleep } from '../../../../common/utils/async';
 import { logWarning } from '../../../../logging';
 import { IEnvsCache } from '../../envsCache';
 import { PythonEnvInfo } from '../../info';
@@ -168,7 +168,37 @@ type RequestID = number;
 type RunFunc = () => Promise<void>;
 type NotifyFunc = () => void;
 
+type RetryOptions = {
+    maxRetries: number;
+    intervalms: number;
+};
+
+// Set defaults and otherwise adjust values.
+function normalizeRetryOptions(
+    opts: Partial<RetryOptions> | undefined,
+    defaults: RetryOptions = { maxRetries: 3, intervalms: 100 },
+): RetryOptions | undefined {
+    if (opts === undefined) {
+        return undefined;
+    }
+    const normalized = { ...opts };
+    if (normalized.maxRetries === undefined) {
+        normalized.maxRetries = defaults.maxRetries;
+    } else if (normalized.maxRetries < 0) {
+        // This is effectively infinity.
+        normalized.maxRetries = Number.MAX_SAFE_INTEGER;
+    }
+    if (normalized.intervalms === undefined) {
+        normalized.intervalms = defaults.intervalms;
+    }
+    return normalized as RetryOptions;
+}
+
 class BackgroundLooper {
+    private readonly opts: {
+        retry?: RetryOptions;
+    };
+
     private started = false;
 
     private stopped = false;
@@ -185,6 +215,16 @@ class BackgroundLooper {
 
     private lastID: number | undefined;
 
+    constructor(
+        opts: {
+            retry?: Partial<RetryOptions>;
+        } = {},
+    ) {
+        this.opts = {
+            retry: normalizeRetryOptions(opts.retry),
+        };
+    }
+
     public start(): void {
         if (this.stopped) {
             throw Error('already stopped');
@@ -221,6 +261,7 @@ class BackgroundLooper {
         const changed = opts.changed === undefined ? true : opts.changed;
         const lastID = this.lastID === undefined ? -1 : this.lastID;
         let id = lastID + 1;
+        // XXX `this.queue.length` may not be stable enough...
         if (!changed && this.lastID !== undefined && this.queue.length > 0) {
             id = lastID;
         }
@@ -260,12 +301,8 @@ class BackgroundLooper {
             while (this.queue.length > 0) {
                 const id = this.queue[0];
                 this.queue.shift();
-                const [run, , notify] = this.requests[id];
                 // eslint-disable-next-line no-await-in-loop
-                await run();
-                // XXX retries?
-                delete this.requests[id];
-                notify();
+                await this.runRequest(id);
             }
             // eslint-disable-next-line no-await-in-loop
             await Promise.race([
@@ -275,4 +312,33 @@ class BackgroundLooper {
         }
         this.loopRunning.resolve();
     }
+
+    private async runRequest(id: RequestID): Promise<void> {
+        const [run, , notify] = this.requests[id];
+
+        let retriesLeft = this.opts.retry !== undefined
+            ? this.opts.retry.maxRetries
+            : 0;
+        let retrying = false;
+        do {
+            try {
+                // eslint-disable-next-line no-await-in-loop
+                await run();
+            } catch (err) {
+                if (retriesLeft < 1) {
+                    throw err; // re-trhow
+                }
+                retriesLeft -= 1;
+                logWarning(`failed while handling request (${err})`);
+                logWarning(`retrying (${retriesLeft} attempts left)`);
+                // We cannot get here if "opts.retry" is not defined.
+                // eslint-disable-next-line no-await-in-loop
+                await sleep(this.opts.retry!.intervalms);
+                retrying = true;
+            }
+        } while (!retrying);
+
+        delete this.requests[id];
+        notify();
+    }
 }

From 4a22fece9c1812839e5d0f20bb7afd01b9e489a6 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 30 Sep 2020 11:38:51 -0600
Subject: [PATCH 16/39] Eliminate the queue stability issues with getID().

---
 .../base/locators/composite/cachingLocator.ts | 80 ++++++++++++-------
 1 file changed, 50 insertions(+), 30 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index bad869fea568..f983483d9880 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -119,15 +119,26 @@ export class CachingLocator implements ILocator {
         }
     }
 
-    private async refresh(
+    private refresh(
         event?: PythonEnvsChangedEvent,
     ): Promise<void> {
-        const id = this.looper.getID({ changed: !!event });
-        return this.looper.addRequest(id, async () => {
-            const iterator = this.locator.iterEnvs();
-            const envs = await getEnvs(iterator);
-            await this.update(envs, event);
-        });
+        if (!event) {
+            // Re-use the last req in the queue if possible.
+            const last = this.looper.getLastRequest();
+            if (last !== undefined) {
+                const [, promise] = last;
+                return promise;
+            }
+            // The queue is empty so add a new request.
+        }
+        const [, waitUntilDone] = this.looper.addRequest(
+            async () => {
+                const iterator = this.locator.iterEnvs();
+                const envs = await getEnvs(iterator);
+                await this.update(envs, event);
+            },
+        );
+        return waitUntilDone;
     }
 
     private async update(
@@ -257,36 +268,33 @@ class BackgroundLooper {
         return this.loopRunning.promise;
     }
 
-    public getID(opts: { changed?: boolean } = {}): RequestID {
-        const changed = opts.changed === undefined ? true : opts.changed;
-        const lastID = this.lastID === undefined ? -1 : this.lastID;
-        let id = lastID + 1;
-        // XXX `this.queue.length` may not be stable enough...
-        if (!changed && this.lastID !== undefined && this.queue.length > 0) {
-            id = lastID;
+    public getLastRequest(): [RequestID, Promise<void>] | undefined {
+        if (this.lastID === undefined) {
+            return undefined;
         }
-        this.lastID = id;
-        return id;
-    }
-
-    public addRequest(
-        id: RequestID,
-        run: RunFunc,
-    ): Promise<void> {
-        const req = this.requests[id];
-        if (req !== undefined) {
-            // eslint-disable-next-line comma-dangle,comma-spacing
-            const [, promise,] = req;
-            return promise;
+        const req = this.requests[this.lastID];
+        if (req === undefined) {
+            // The queue must be empty.
+            return undefined;
         }
+        // eslint-disable-next-line comma-dangle,comma-spacing
+        const [, promise,] = req;
+        return [this.lastID, promise];
+    }
 
+    public addRequest(run: RunFunc): [RequestID, Promise<void>] {
+        const reqid = this.getNextID();
+        // This is the only method that adds requests to the queue
+        // and `getNextID()` keeps us from having collisions here.
+        // So we are guaranteed that there are no matching requests
+        // in the queue.
         const running = createDeferred<void>();
-        this.requests[id] = [run, running.promise, () => running.resolve()];
-        this.queue.push(id);
+        this.requests[reqid] = [run, running.promise, () => running.resolve()];
+        this.queue.push(reqid);
         // `waitUntilReady` will get replaced with a new deferred in
         // the loop once the existing one gets used.
         this.waitUntilReady.resolve();
-        return running.promise;
+        return [reqid, running.promise];
     }
 
     private async runLoop(): Promise<void> {
@@ -341,4 +349,16 @@ class BackgroundLooper {
         delete this.requests[id];
         notify();
     }
+
+    private getNextID(): RequestID {
+        // For nowe there is no way to queue up a request with
+        // an ID that did not originate here.  So we don't need
+        // to worry about collisions.
+        if (this.lastID === undefined) {
+            this.lastID = 1;
+        } else {
+            this.lastID += 1;
+        }
+        return this.lastID;
+    }
 }

From 8a561e03913a58badcd89b5e40762c2f99092ac4 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 30 Sep 2020 12:27:20 -0600
Subject: [PATCH 17/39] Fix the logic of the run loop.

---
 .../base/locators/composite/cachingLocator.ts     | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index f983483d9880..319344e112e5 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -298,14 +298,16 @@ class BackgroundLooper {
     }
 
     private async runLoop(): Promise<void> {
-        const winner = await Promise.race([
+        const getWinner = () => Promise.race([
             this.done.promise.then(() => 0),
             this.waitUntilReady.promise.then(() => 1),
         ]);
-        if (winner === 1) {
-            this.waitUntilReady = createDeferred<void>();
-        }
+        let winner = await getWinner();
         while (!this.done.completed) {
+            if (winner === 1) {
+                this.waitUntilReady = createDeferred<void>();
+            }
+
             while (this.queue.length > 0) {
                 const id = this.queue[0];
                 this.queue.shift();
@@ -313,10 +315,7 @@ class BackgroundLooper {
                 await this.runRequest(id);
             }
             // eslint-disable-next-line no-await-in-loop
-            await Promise.race([
-                this.waitUntilReady,
-                this.done,
-            ]);
+            winner = await getWinner();
         }
         this.loopRunning.resolve();
     }

From 6038238ec302b989a0305e43bd353b90acfb491d Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 30 Sep 2020 13:24:26 -0600
Subject: [PATCH 18/39] Periodically refresh the cache.

---
 .../base/locators/composite/cachingLocator.ts | 193 +++++++++++++++---
 1 file changed, 160 insertions(+), 33 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 319344e112e5..47de0d038f8c 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -20,26 +20,66 @@ import { getEnvs, getQueryFilter } from '../../locatorUtils';
 import { PythonEnvsChangedEvent, PythonEnvsWatcher } from '../../watcher';
 import { pickBestEnv } from './reducingLocator';
 
+type CachingLocatorOptions = {
+    refreshMinutes: number,
+    refreshRetryMinutes: number,
+};
+
+// Set defaults and otherwise adjust values.
+function normalizeCachingLocatorOptions(
+    opts: Partial<CachingLocatorOptions>,
+    defaults: CachingLocatorOptions = {
+        refreshMinutes: 24 * 60, // 1 day
+        refreshRetryMinutes: 10,
+    },
+): CachingLocatorOptions {
+    const normalized = { ...opts };
+    if (normalized.refreshMinutes === undefined) {
+        normalized.refreshMinutes = defaults.refreshMinutes;
+    }
+    if (normalized.refreshRetryMinutes === undefined) {
+        normalized.refreshRetryMinutes = defaults.refreshRetryMinutes;
+    }
+    return normalized as CachingLocatorOptions;
+}
+
 /**
  * A locator that stores the known environments in the given cache.
  */
 export class CachingLocator implements ILocator {
     public readonly onChanged: Event<PythonEnvsChangedEvent>;
 
+    private readonly opts: CachingLocatorOptions;
+
     private readonly watcher = new PythonEnvsWatcher();
 
     private readonly initializing = createDeferred<void>();
 
     private initialized = false;
 
-    // eslint-disable-next-line @typescript-eslint/no-use-before-define
-    private looper = new BackgroundLooper();
+    private looper: BackgroundLooper;
 
     constructor(
         private readonly cache: IEnvsCache,
         private readonly locator: ILocator,
+        opts: {
+            refreshMinutes?: number,
+            refreshRetryMinutes?: number,
+            checkStale?: boolean,
+        } = {},
     ) {
         this.onChanged = this.watcher.onChanged;
+        this.opts = normalizeCachingLocatorOptions(opts);
+        // eslint-disable-next-line @typescript-eslint/no-use-before-define
+        this.looper = new BackgroundLooper({
+            runDefault: () => this.doRefresh(),
+            retry: {
+                intervalms: this.opts.refreshRetryMinutes * 60 * 1000,
+            },
+            periodic: {
+                intervalms: this.opts.refreshMinutes * 60 * 1000,
+            },
+        });
     }
 
     /**
@@ -59,6 +99,7 @@ export class CachingLocator implements ILocator {
         this.looper.start();
         await this.initialRefresh();
         this.locator.onChanged((event) => {
+            // XXX only do it if no earlier events are pending...
             this.refresh(event).ignoreErrors();
         });
     }
@@ -132,15 +173,19 @@ export class CachingLocator implements ILocator {
             // The queue is empty so add a new request.
         }
         const [, waitUntilDone] = this.looper.addRequest(
-            async () => {
-                const iterator = this.locator.iterEnvs();
-                const envs = await getEnvs(iterator);
-                await this.update(envs, event);
-            },
+            () => this.doRefresh(event),
         );
         return waitUntilDone;
     }
 
+    private async doRefresh(
+        event?: PythonEnvsChangedEvent,
+    ): Promise<void> {
+        const iterator = this.locator.iterEnvs();
+        const envs = await getEnvs(iterator);
+        await this.update(envs, event);
+    }
+
     private async update(
         envs: PythonEnvInfo[],
         event?: PythonEnvsChangedEvent,
@@ -187,7 +232,10 @@ type RetryOptions = {
 // Set defaults and otherwise adjust values.
 function normalizeRetryOptions(
     opts: Partial<RetryOptions> | undefined,
-    defaults: RetryOptions = { maxRetries: 3, intervalms: 100 },
+    defaults: RetryOptions = {
+        maxRetries: 3,
+        intervalms: 100,
+    },
 ): RetryOptions | undefined {
     if (opts === undefined) {
         return undefined;
@@ -205,9 +253,39 @@ function normalizeRetryOptions(
     return normalized as RetryOptions;
 }
 
+type PeriodicOptions = {
+    intervalms: number;
+    initialTimestamp: number;
+};
+
+function normalizePeriodicOptions(
+    opts: Partial<PeriodicOptions> | undefined,
+    defaults: PeriodicOptions = {
+        intervalms: -1,
+        initialTimestamp: -1,
+    },
+): PeriodicOptions | undefined {
+    if (opts === undefined) {
+        return undefined;
+    }
+    const normalized = { ...opts };
+    if (normalized.intervalms === undefined) {
+        // "never run"
+        normalized.intervalms = defaults.intervalms;
+    }
+    if (normalized.initialTimestamp === undefined && normalized.intervalms > -1) {
+        normalized.initialTimestamp = Date.now() + normalized.intervalms;
+    } else {
+        normalized.initialTimestamp = defaults.initialTimestamp;
+    }
+    return normalized as PeriodicOptions;
+}
+
 class BackgroundLooper {
     private readonly opts: {
+        runDefault: RunFunc;
         retry?: RetryOptions;
+        periodic?: PeriodicOptions;
     };
 
     private started = false;
@@ -226,14 +304,25 @@ class BackgroundLooper {
 
     private lastID: number | undefined;
 
+    private nextPeriod = -1;
+
     constructor(
         opts: {
+            runDefault?: RunFunc;
             retry?: Partial<RetryOptions>;
+            periodic?: Partial<PeriodicOptions>;
         } = {},
     ) {
         this.opts = {
+            runDefault: opts.runDefault !== undefined
+                ? opts.runDefault
+                : async () => { throw Error('no default operation provided'); },
             retry: normalizeRetryOptions(opts.retry),
+            periodic: normalizePeriodicOptions(opts.periodic),
         };
+        if (this.opts.periodic !== undefined) {
+            this.nextPeriod = this.opts.periodic.initialTimestamp;
+        }
     }
 
     public start(): void {
@@ -282,37 +371,59 @@ class BackgroundLooper {
         return [this.lastID, promise];
     }
 
-    public addRequest(run: RunFunc): [RequestID, Promise<void>] {
+    public addRequest(run?: RunFunc): [RequestID, Promise<void>] {
         const reqid = this.getNextID();
         // This is the only method that adds requests to the queue
         // and `getNextID()` keeps us from having collisions here.
         // So we are guaranteed that there are no matching requests
         // in the queue.
         const running = createDeferred<void>();
-        this.requests[reqid] = [run, running.promise, () => running.resolve()];
+        this.requests[reqid] = [
+            run !== undefined ? run : this.opts.runDefault,
+            running.promise,
+            () => running.resolve(),
+        ];
         this.queue.push(reqid);
-        // `waitUntilReady` will get replaced with a new deferred in
-        // the loop once the existing one gets used.
-        this.waitUntilReady.resolve();
+        if (this.queue.length === 1) {
+            // `waitUntilReady` will get replaced with a new deferred
+            // in the loop once the existing one gets used.
+            // We let the queue clear out before triggering the loop
+            // again.
+            this.waitUntilReady.resolve();
+        }
         return [reqid, running.promise];
     }
 
     private async runLoop(): Promise<void> {
-        const getWinner = () => Promise.race([
-            this.done.promise.then(() => 0),
-            this.waitUntilReady.promise.then(() => 1),
-        ]);
+        const getWinner = () => {
+            const promises = [
+                this.done.promise.then(() => 0),
+                this.waitUntilReady.promise.then(() => 1),
+            ];
+            if (this.opts.periodic !== undefined && this.nextPeriod > -1) {
+                const msLeft = Math.max(0, this.nextPeriod - Date.now());
+                promises.push(
+                    sleep(msLeft).then(() => 2),
+                );
+            }
+            return Promise.race(promises);
+        };
+
         let winner = await getWinner();
         while (!this.done.completed) {
             if (winner === 1) {
                 this.waitUntilReady = createDeferred<void>();
-            }
-
-            while (this.queue.length > 0) {
-                const id = this.queue[0];
-                this.queue.shift();
                 // eslint-disable-next-line no-await-in-loop
-                await this.runRequest(id);
+                await this.flush();
+            } else if (winner === 2) {
+                // We reset the period before queueing to avoid any races.
+                this.nextPeriod = Date.now() + this.opts.periodic!.intervalms;
+                // Rather than running the request directly, we add
+                // it to the queue.  This avoids races.
+                this.addRequest(this.opts.runDefault);
+            } else {
+                // This should not be reachable.
+                throw Error(`unsupported winner ${winner}`);
             }
             // eslint-disable-next-line no-await-in-loop
             winner = await getWinner();
@@ -320,12 +431,32 @@ class BackgroundLooper {
         this.loopRunning.resolve();
     }
 
-    private async runRequest(id: RequestID): Promise<void> {
-        const [run, , notify] = this.requests[id];
+    private async flush(): Promise<void> {
+        // Run every request in the queue.
+        while (this.queue.length > 0) {
+            const reqid = this.queue[0];
+            // We pop the request off the queue early because ....?
+            this.queue.shift();
+            const [run, , notify] = this.requests[reqid];
+
+            // eslint-disable-next-line no-await-in-loop
+            await this.runRequest(run);
+
+            // We leave the request until right before `notify()`
+            // for the sake of any calls to `getLastRequest()`.
+            delete this.requests[reqid];
+            notify();
+        }
+    }
 
-        let retriesLeft = this.opts.retry !== undefined
-            ? this.opts.retry.maxRetries
-            : 0;
+    private async runRequest(run: RunFunc): Promise<void> {
+        if (this.opts.retry === undefined) {
+            // eslint-disable-next-line no-await-in-loop
+            await run();
+            return;
+        }
+        let retriesLeft = this.opts.retry.maxRetries;
+        const retryIntervalms = this.opts.retry.intervalms;
         let retrying = false;
         do {
             try {
@@ -338,15 +469,11 @@ class BackgroundLooper {
                 retriesLeft -= 1;
                 logWarning(`failed while handling request (${err})`);
                 logWarning(`retrying (${retriesLeft} attempts left)`);
-                // We cannot get here if "opts.retry" is not defined.
                 // eslint-disable-next-line no-await-in-loop
-                await sleep(this.opts.retry!.intervalms);
+                await sleep(retryIntervalms);
                 retrying = true;
             }
         } while (!retrying);
-
-        delete this.requests[id];
-        notify();
     }
 
     private getNextID(): RequestID {

From 72becf89839a53d499619cd66579dffac8b19a86 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 30 Sep 2020 13:44:10 -0600
Subject: [PATCH 19/39] Rely on onChanged to know if the cache is stale.

---
 .../base/locators/composite/cachingLocator.ts  | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 47de0d038f8c..174008274ffc 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -65,7 +65,6 @@ export class CachingLocator implements ILocator {
         opts: {
             refreshMinutes?: number,
             refreshRetryMinutes?: number,
-            checkStale?: boolean,
         } = {},
     ) {
         this.onChanged = this.watcher.onChanged;
@@ -148,10 +147,9 @@ export class CachingLocator implements ILocator {
             logWarning('envs cache unexpectedly not initialized');
             return;
         }
-        if (await this.needsRefresh(envs)) {
-            // Refresh in the background.
-            this.refresh().ignoreErrors();
-        }
+        // We trust `this.locator.onChanged` to be reliable.
+        // So there is no need to check if anything is stale
+        // at this point.
         if (query !== undefined) {
             const filter = getQueryFilter(query);
             yield* envs.filter(filter);
@@ -208,16 +206,6 @@ export class CachingLocator implements ILocator {
             this.initializing.resolve();
         }
     }
-
-    // eslint-disable-next-line class-methods-use-this,@typescript-eslint/no-unused-vars
-    private async needsRefresh(_envs: PythonEnvInfo[]): Promise<boolean> {
-        // XXX
-        // For now we never refresh.  Options:
-        // * every X minutes (via `initialize()`
-        // * if at least X minutes have elapsed
-        // * if some "stale" check on any known env fails
-        return false;
-    }
 }
 
 type RequestID = number;

From 764482a3315b9ab50e1c0cf1df50b6af00e60c24 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 30 Sep 2020 14:26:54 -0600
Subject: [PATCH 20/39] Add BackgroundLooper.getNextRequest().

---
 .../base/locators/composite/cachingLocator.ts | 87 ++++++++++++-------
 1 file changed, 56 insertions(+), 31 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 174008274ffc..ce2d1f0485f7 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -97,10 +97,7 @@ export class CachingLocator implements ILocator {
         await this.cache.initialize();
         this.looper.start();
         await this.initialRefresh();
-        this.locator.onChanged((event) => {
-            // XXX only do it if no earlier events are pending...
-            this.refresh(event).ignoreErrors();
-        });
+        this.locator.onChanged((event) => this.handleChange(event));
     }
 
     public dispose(): void {
@@ -158,21 +155,15 @@ export class CachingLocator implements ILocator {
         }
     }
 
-    private refresh(
-        event?: PythonEnvsChangedEvent,
-    ): Promise<void> {
-        if (!event) {
-            // Re-use the last req in the queue if possible.
-            const last = this.looper.getLastRequest();
-            if (last !== undefined) {
-                const [, promise] = last;
-                return promise;
-            }
-            // The queue is empty so add a new request.
+    private refresh(): Promise<void> {
+        // Re-use the last req in the queue if possible.
+        const last = this.looper.getLastRequest();
+        if (last !== undefined) {
+            const [, promise] = last;
+            return promise;
         }
-        const [, waitUntilDone] = this.looper.addRequest(
-            () => this.doRefresh(event),
-        );
+        // The queue is empty so add a new request.
+        const [, waitUntilDone] = this.looper.addRequest();
         return waitUntilDone;
     }
 
@@ -194,6 +185,15 @@ export class CachingLocator implements ILocator {
         this.watcher.fire(event || {}); // Emit an "onCHanged" event.
     }
 
+    private handleChange(event: PythonEnvsChangedEvent): void {
+        const req = this.looper.getNextRequest();
+        if (req === undefined) {
+            // There isn't already a pending request.
+            this.looper.addRequest(() => this.doRefresh(event));
+        }
+        // Otherwise let the pending request take care of it.
+    }
+
     private async initialRefresh(): Promise<void> {
         const envs = this.cache.getAllEnvs();
         if (envs !== undefined) {
@@ -284,8 +284,11 @@ class BackgroundLooper {
 
     private readonly loopRunning = createDeferred<void>();
 
-    private waitUntilReady= createDeferred<void>();
+    private waitUntilReady = createDeferred<void>();
 
+    private running: RequestID | undefined;
+
+    // For now we don't worry about a max queue size.
     private readonly queue: RequestID[] = [];
 
     private readonly requests: Record<RequestID, [RunFunc, Promise<void>, NotifyFunc]> = {};
@@ -346,32 +349,48 @@ class BackgroundLooper {
     }
 
     public getLastRequest(): [RequestID, Promise<void>] | undefined {
-        if (this.lastID === undefined) {
+        let reqID: RequestID;
+        if (this.queue.length > 0) {
+            reqID = this.queue[this.queue.length - 1];
+        } else if (this.running !== undefined) {
+            reqID = this.running;
+        } else {
             return undefined;
         }
-        const req = this.requests[this.lastID];
-        if (req === undefined) {
+        // The req cannot be undefined since every queued ID has a request.
+        // eslint-disable-next-line comma-dangle,comma-spacing
+        const [, promise,] = this.requests[reqID];
+        if (reqID === undefined) {
             // The queue must be empty.
             return undefined;
         }
+        return [reqID, promise];
+    }
+
+    public getNextRequest(): [RequestID, Promise<void>] | undefined {
+        if (this.queue.length === 0) {
+            return undefined;
+        }
+        const reqID = this.queue[0];
+        // The req cannot be undefined since every queued ID has a request.
         // eslint-disable-next-line comma-dangle,comma-spacing
-        const [, promise,] = req;
-        return [this.lastID, promise];
+        const [, promise,] = this.requests[reqID]!;
+        return [reqID, promise];
     }
 
     public addRequest(run?: RunFunc): [RequestID, Promise<void>] {
-        const reqid = this.getNextID();
+        const reqID = this.getNextID();
         // This is the only method that adds requests to the queue
         // and `getNextID()` keeps us from having collisions here.
         // So we are guaranteed that there are no matching requests
         // in the queue.
         const running = createDeferred<void>();
-        this.requests[reqid] = [
+        this.requests[reqID] = [
             run !== undefined ? run : this.opts.runDefault,
             running.promise,
             () => running.resolve(),
         ];
-        this.queue.push(reqid);
+        this.queue.push(reqID);
         if (this.queue.length === 1) {
             // `waitUntilReady` will get replaced with a new deferred
             // in the loop once the existing one gets used.
@@ -379,7 +398,7 @@ class BackgroundLooper {
             // again.
             this.waitUntilReady.resolve();
         }
-        return [reqid, running.promise];
+        return [reqID, running.promise];
     }
 
     private async runLoop(): Promise<void> {
@@ -420,21 +439,27 @@ class BackgroundLooper {
     }
 
     private async flush(): Promise<void> {
+        if (this.running !== undefined) {
+            // We must be flushing the queue already.
+            return;
+        }
         // Run every request in the queue.
         while (this.queue.length > 0) {
-            const reqid = this.queue[0];
+            const reqID = this.queue[0];
+            this.running = reqID;
             // We pop the request off the queue early because ....?
             this.queue.shift();
-            const [run, , notify] = this.requests[reqid];
+            const [run, , notify] = this.requests[reqID];
 
             // eslint-disable-next-line no-await-in-loop
             await this.runRequest(run);
 
             // We leave the request until right before `notify()`
             // for the sake of any calls to `getLastRequest()`.
-            delete this.requests[reqid];
+            delete this.requests[reqID];
             notify();
         }
+        this.running = undefined;
     }
 
     private async runRequest(run: RunFunc): Promise<void> {

From 1a71c49899f5a7c5d3e09bd2a6949c68f05d3713 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 30 Sep 2020 14:55:16 -0600
Subject: [PATCH 21/39] Factor out CachingLocator.iterFromDownstream().

---
 .../base/locators/composite/cachingLocator.ts | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index ce2d1f0485f7..89976577db48 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -106,6 +106,11 @@ export class CachingLocator implements ILocator {
     }
 
     public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator {
+        // We assume that `getAllEnvs()` is cheap enough that calling
+        // it again in `iterFromCache()` is not a problem.
+        if (this.cache.getAllEnvs() === undefined) {
+            return this.iterFromDownstream(query);
+        }
         return this.iterFromCache(query);
     }
 
@@ -135,10 +140,21 @@ export class CachingLocator implements ILocator {
         return resolved;
     }
 
-    private async* iterFromCache(query?: PythonLocatorQuery): IPythonEnvsIterator {
-        // XXX For now we wait for the initial refresh to finish...
+    private async* iterFromDownstream(query?: PythonLocatorQuery): IPythonEnvsIterator {
+        // For now we wait for the initial refresh to finish.  If that
+        // turns out to be a problem then we can do something more
+        // clever here.
         await this.initializing.promise;
+        const iterator = this.iterFromCache(query);
+        let res = await iterator.next();
+        while (!res.done) {
+            yield res.value;
+            // eslint-disable-next-line no-await-in-loop
+            res = await iterator.next();
+        }
+    }
 
+    private async* iterFromCache(query?: PythonLocatorQuery): IPythonEnvsIterator {
         const envs = this.cache.getAllEnvs();
         if (envs === undefined) {
             logWarning('envs cache unexpectedly not initialized');

From 99af470a30096cb963ef905fe30f7ede9210252b Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 30 Sep 2020 16:39:33 -0600
Subject: [PATCH 22/39] Fix the tslint rules.

---
 tslint.json | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tslint.json b/tslint.json
index 0b83af565aa2..bce2aad44748 100644
--- a/tslint.json
+++ b/tslint.json
@@ -52,6 +52,7 @@
         "no-unnecessary-type-assertion": false,
         "no-submodule-imports": false,
         "no-redundant-jsdoc": false,
-        "binary-expression-operand-order": false
+        "binary-expression-operand-order": false,
+        "no-use-before-declare": false
     }
 }

From e31ee150f5e7ef3c0de43bfc29a38cd8ed3bfd85 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Mon, 5 Oct 2020 13:18:41 -0600
Subject: [PATCH 23/39] Fix callbacks in SimpleLocator.

---
 src/test/pythonEnvironments/base/common.ts | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/test/pythonEnvironments/base/common.ts b/src/test/pythonEnvironments/base/common.ts
index fe3ddc1d1ff8..1e78ed20f59d 100644
--- a/src/test/pythonEnvironments/base/common.ts
+++ b/src/test/pythonEnvironments/base/common.ts
@@ -73,13 +73,13 @@ export class SimpleLocator extends Locator {
         const callbacks = this.callbacks;
         let envs = this.envs;
         const iterator: IPythonEnvsIterator = async function*() {
-            if (callbacks?.onQuery !== undefined) {
+            if (callbacks.onQuery !== undefined) {
                 envs = await callbacks.onQuery(query, envs);
             }
-            if (callbacks?.before !== undefined) {
+            if (callbacks.before !== undefined) {
                 await callbacks.before;
             }
-            if (callbacks?.beforeEach !== undefined) {
+            if (callbacks.beforeEach !== undefined) {
                 // The results will likely come in a different order.
                 const mapped = mapToIterator(envs, async (env) => {
                     await callbacks.beforeEach!(env);
@@ -87,31 +87,31 @@ export class SimpleLocator extends Locator {
                 });
                 for await (const env of iterable(mapped)) {
                     yield env;
-                    if (callbacks?.afterEach !== undefined) {
+                    if (callbacks.afterEach !== undefined) {
                         await callbacks.afterEach(env);
                     }
                 }
             } else {
                 for (const env of envs) {
                     yield env;
-                    if (callbacks?.afterEach !== undefined) {
+                    if (callbacks.afterEach !== undefined) {
                         await callbacks.afterEach(env);
                     }
                 }
             }
-            if (callbacks?.after!== undefined) {
+            if (callbacks.after!== undefined) {
                 await callbacks.after;
             }
             deferred.resolve();
         }();
-        iterator.onUpdated = this.callbacks?.onUpdated;
+        iterator.onUpdated = this.callbacks.onUpdated;
         return iterator;
     }
     public async resolveEnv(env: string | PythonEnvInfo): Promise<PythonEnvInfo | undefined> {
         const envInfo: PythonEnvInfo = typeof env === 'string' ? createLocatedEnv('', '', undefined, env) : env;
-        if (this.callbacks?.resolve === undefined) {
+        if (this.callbacks.resolve === undefined) {
             return envInfo;
-        } else if (this.callbacks?.resolve === null) {
+        } else if (this.callbacks.resolve === null) {
             return undefined;
         } else {
             return this.callbacks.resolve(envInfo);

From d73de3577bef2e77a5ad350de86d7b7e89c5659b Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Mon, 5 Oct 2020 13:20:45 -0600
Subject: [PATCH 24/39] Fix a typo.

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

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 89976577db48..818a3744d900 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -506,7 +506,7 @@ class BackgroundLooper {
     }
 
     private getNextID(): RequestID {
-        // For nowe there is no way to queue up a request with
+        // For now there is no way to queue up a request with
         // an ID that did not originate here.  So we don't need
         // to worry about collisions.
         if (this.lastID === undefined) {

From c26b56c6f226405ad0b6792076a733cd3a3cb266 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Mon, 5 Oct 2020 13:31:51 -0600
Subject: [PATCH 25/39] Use a syntactic shortcut.

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

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 818a3744d900..9aebde8747cc 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -402,7 +402,7 @@ class BackgroundLooper {
         // in the queue.
         const running = createDeferred<void>();
         this.requests[reqID] = [
-            run !== undefined ? run : this.opts.runDefault,
+            run ?? this.opts.runDefault,
             running.promise,
             () => running.resolve(),
         ];

From e756686c18243275ed1d95142d69a3fccfd00481 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Mon, 5 Oct 2020 13:50:35 -0600
Subject: [PATCH 26/39] Add doc comments on internal methods.

---
 .../base/locators/composite/cachingLocator.ts | 28 ++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 9aebde8747cc..10051f114fc6 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -140,6 +140,11 @@ export class CachingLocator implements ILocator {
         return resolved;
     }
 
+    /**
+     * A generator that yields the envs provided by the downstream locator.
+     *
+     * Contrast this with `iterFromCache()` that yields only from the cache.
+     */
     private async* iterFromDownstream(query?: PythonLocatorQuery): IPythonEnvsIterator {
         // For now we wait for the initial refresh to finish.  If that
         // turns out to be a problem then we can do something more
@@ -154,6 +159,12 @@ export class CachingLocator implements ILocator {
         }
     }
 
+    /**
+     * A generator that yields the envs found in the cache.
+     *
+     * Contrast this with `iterFromDownstream()` which relies on
+     * the downstream locator.
+     */
     private async* iterFromCache(query?: PythonLocatorQuery): IPythonEnvsIterator {
         const envs = this.cache.getAllEnvs();
         if (envs === undefined) {
@@ -171,6 +182,12 @@ export class CachingLocator implements ILocator {
         }
     }
 
+    /**
+     * Trigger a refresh of the cache from the downstream locator.
+     *
+     * Note that if a refresh has already been requested or is currently
+     * running, this is a noop.
+     */
     private refresh(): Promise<void> {
         // Re-use the last req in the queue if possible.
         const last = this.looper.getLastRequest();
@@ -183,6 +200,11 @@ export class CachingLocator implements ILocator {
         return waitUntilDone;
     }
 
+    /**
+     * Immediately perform a refresh of the cache from the downstream locator.
+     *
+     * It does not matter if another refresh is already
+     */
     private async doRefresh(
         event?: PythonEnvsChangedEvent,
     ): Promise<void> {
@@ -191,6 +213,9 @@ export class CachingLocator implements ILocator {
         await this.update(envs, event);
     }
 
+    /**
+     * Set the cache to the given envs, flush, and emit an onChanged event.
+     */
     private async update(
         envs: PythonEnvInfo[],
         event?: PythonEnvsChangedEvent,
@@ -204,7 +229,8 @@ export class CachingLocator implements ILocator {
     private handleChange(event: PythonEnvsChangedEvent): void {
         const req = this.looper.getNextRequest();
         if (req === undefined) {
-            // There isn't already a pending request.
+            // There isn't already a pending request (due to an
+            // onChanged event), so we add one.
             this.looper.addRequest(() => this.doRefresh(event));
         }
         // Otherwise let the pending request take care of it.

From 316dd912445833b5dcb9268d86c7c7a22e6cacaf Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Mon, 5 Oct 2020 14:21:36 -0600
Subject: [PATCH 27/39] Make the different refresh scenarios a bit easier to
 follow.

---
 .../base/locators/composite/cachingLocator.ts | 83 +++++++++++--------
 1 file changed, 47 insertions(+), 36 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 10051f114fc6..27bcd847f36f 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -71,7 +71,7 @@ export class CachingLocator implements ILocator {
         this.opts = normalizeCachingLocatorOptions(opts);
         // eslint-disable-next-line @typescript-eslint/no-use-before-define
         this.looper = new BackgroundLooper({
-            runDefault: () => this.doRefresh(),
+            runDefault: () => this.refresh(),
             retry: {
                 intervalms: this.opts.refreshRetryMinutes * 60 * 1000,
             },
@@ -96,8 +96,20 @@ export class CachingLocator implements ILocator {
 
         await this.cache.initialize();
         this.looper.start();
-        await this.initialRefresh();
-        this.locator.onChanged((event) => this.handleChange(event));
+
+        this.locator.onChanged((event) => this.ensureCurrentRefresh(event));
+
+        // Do the initial refresh.
+        const envs = this.cache.getAllEnvs();
+        if (envs !== undefined) {
+            this.initializing.resolve();
+            await this.ensureRecentRefresh();
+        } else {
+            // There is nothing in the cache, so we must wait for the
+            // initial refresh to finish before allowing iteration.
+            await this.ensureRecentRefresh();
+            this.initializing.resolve();
+        }
     }
 
     public dispose(): void {
@@ -134,7 +146,7 @@ export class CachingLocator implements ILocator {
             const envs = this.cache.getAllEnvs();
             if (envs !== undefined) {
                 envs.push(resolved);
-                await this.update(envs);
+                await this.updateCache(envs);
             }
         }
         return resolved;
@@ -183,12 +195,13 @@ export class CachingLocator implements ILocator {
     }
 
     /**
-     * Trigger a refresh of the cache from the downstream locator.
+     * Maybe trigger a refresh of the cache from the downstream locator.
      *
-     * Note that if a refresh has already been requested or is currently
-     * running, this is a noop.
+     * If a refresh isn't already running then we request a refresh and
+     * wait for it to finish.  Otherwise we do not make a new request,
+     * but instead only wait for the last requested refresh to complete.
      */
-    private refresh(): Promise<void> {
+    private ensureRecentRefresh(): Promise<void> {
         // Re-use the last req in the queue if possible.
         const last = this.looper.getLastRequest();
         if (last !== undefined) {
@@ -196,27 +209,48 @@ export class CachingLocator implements ILocator {
             return promise;
         }
         // The queue is empty so add a new request.
-        const [, waitUntilDone] = this.looper.addRequest();
+        const [, waitUntilDone] = this.looper.addRequest(() => this.refresh());
         return waitUntilDone;
     }
 
+    /**
+     * Maybe trigger a refresh of the cache from the downstream locator.
+     *
+     * Make sure that a completely new refresh will be started soon and
+     * wait for it to finish.  If a refresh isn't already running then
+     * we start one and wait for it to finish.  If one is already
+     * running then we make sure a new one is requested to start after
+     * that and wait for it to finish.  That means if one is already
+     * waiting in the queue then we wait for that one instead of making
+     * a new request.
+     */
+    private ensureCurrentRefresh(event?: PythonEnvsChangedEvent): void {
+        const req = this.looper.getNextRequest();
+        if (req === undefined) {
+            // There isn't already a pending request (due to an
+            // onChanged event), so we add one.
+            this.looper.addRequest(() => this.refresh(event));
+        }
+        // Otherwise let the pending request take care of it.
+    }
+
     /**
      * Immediately perform a refresh of the cache from the downstream locator.
      *
-     * It does not matter if another refresh is already
+     * It does not matter if another refresh is already running.
      */
-    private async doRefresh(
+    private async refresh(
         event?: PythonEnvsChangedEvent,
     ): Promise<void> {
         const iterator = this.locator.iterEnvs();
         const envs = await getEnvs(iterator);
-        await this.update(envs, event);
+        await this.updateCache(envs, event);
     }
 
     /**
      * Set the cache to the given envs, flush, and emit an onChanged event.
      */
-    private async update(
+    private async updateCache(
         envs: PythonEnvInfo[],
         event?: PythonEnvsChangedEvent,
     ): Promise<void> {
@@ -225,29 +259,6 @@ export class CachingLocator implements ILocator {
         await this.cache.flush();
         this.watcher.fire(event || {}); // Emit an "onCHanged" event.
     }
-
-    private handleChange(event: PythonEnvsChangedEvent): void {
-        const req = this.looper.getNextRequest();
-        if (req === undefined) {
-            // There isn't already a pending request (due to an
-            // onChanged event), so we add one.
-            this.looper.addRequest(() => this.doRefresh(event));
-        }
-        // Otherwise let the pending request take care of it.
-    }
-
-    private async initialRefresh(): Promise<void> {
-        const envs = this.cache.getAllEnvs();
-        if (envs !== undefined) {
-            this.initializing.resolve();
-            await this.refresh();
-        } else {
-            // There is nothing in the cache, so we must wait for the
-            // initial refresh to finish before allowing iteration.
-            await this.refresh();
-            this.initializing.resolve();
-        }
-    }
 }
 
 type RequestID = number;

From c1ee05b42c6690a0600bacc87533d8cb6b8c231f Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Mon, 5 Oct 2020 14:57:47 -0600
Subject: [PATCH 28/39] Add doc comments for BackgroundLooper.

---
 .../base/locators/composite/cachingLocator.ts | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 27bcd847f36f..374cad5e1bab 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -322,6 +322,12 @@ function normalizePeriodicOptions(
     return normalized as PeriodicOptions;
 }
 
+/**
+ * This helps avoid running duplicate expensive operations.
+ *
+ * The key aspect is that alraedy running or queue requests can be
+ * re-used instead of creating a duplicate request.
+ */
 class BackgroundLooper {
     private readonly opts: {
         runDefault: RunFunc;
@@ -369,6 +375,11 @@ class BackgroundLooper {
         }
     }
 
+    /**
+     * Start the request execution loop.
+     *
+     * Currently it does not support being re-started.
+     */
     public start(): void {
         if (this.stopped) {
             throw Error('already stopped');
@@ -381,6 +392,11 @@ class BackgroundLooper {
         this.runLoop().ignoreErrors();
     }
 
+    /**
+     * Stop the loop (assuming it was already started.)
+     *
+     * @returns - a promise that resolves once the loop has stopped.
+     */
     public stop(): Promise<void> {
         if (this.stopped) {
             return this.loopRunning.promise;
@@ -401,6 +417,15 @@ class BackgroundLooper {
         return this.loopRunning.promise;
     }
 
+    /**
+     * Return the most recent active request, if any.
+     *
+     * If there are no pending requests then this is the currently
+     * running one (if one is running).
+     *
+     * @returns - the ID of the request and its completion promise;
+     *            if there are no active requests then you get `undefined`
+     */
     public getLastRequest(): [RequestID, Promise<void>] | undefined {
         let reqID: RequestID;
         if (this.queue.length > 0) {
@@ -420,6 +445,15 @@ class BackgroundLooper {
         return [reqID, promise];
     }
 
+    /**
+     * Return the request that is waiting to run next, if any.
+     *
+     * The request is the next one that will be run.  This implies that
+     * there is one already running.
+     *
+     * @returns - the ID of the request and its completion promise;
+     *            if there are no pending requests then you get `undefined`
+     */
     public getNextRequest(): [RequestID, Promise<void>] | undefined {
         if (this.queue.length === 0) {
             return undefined;
@@ -431,6 +465,15 @@ class BackgroundLooper {
         return [reqID, promise];
     }
 
+    /**
+     * Request that a function be run.
+     *
+     * If one is already running then the new request is added to the
+     * end of the queue.  Otherwise it is run immediately.
+     *
+     * @returns - the ID of the new request and its completion promise;
+     *            the promise resolves once the request has completed
+     */
     public addRequest(run?: RunFunc): [RequestID, Promise<void>] {
         const reqID = this.getNextID();
         // This is the only method that adds requests to the queue
@@ -454,6 +497,9 @@ class BackgroundLooper {
         return [reqID, running.promise];
     }
 
+    /**
+     * This is the actual loop where the queue is managed and waiting happens.
+     */
     private async runLoop(): Promise<void> {
         const getWinner = () => {
             const promises = [
@@ -491,6 +537,12 @@ class BackgroundLooper {
         this.loopRunning.resolve();
     }
 
+    /**
+     * Run all pending requests, in queue order.
+     *
+     * Each request's completion promise resolves once that request
+     * finishes.
+     */
     private async flush(): Promise<void> {
         if (this.running !== undefined) {
             // We must be flushing the queue already.
@@ -515,6 +567,9 @@ class BackgroundLooper {
         this.running = undefined;
     }
 
+    /**
+     * Run a single request.
+     */
     private async runRequest(run: RunFunc): Promise<void> {
         if (this.opts.retry === undefined) {
             // eslint-disable-next-line no-await-in-loop
@@ -542,6 +597,9 @@ class BackgroundLooper {
         } while (!retrying);
     }
 
+    /**
+     * Provide the request ID to use next.
+     */
     private getNextID(): RequestID {
         // For now there is no way to queue up a request with
         // an ID that did not originate here.  So we don't need

From d66687441e0d468f130cae7fdb29715e0f4c8043 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Mon, 5 Oct 2020 15:25:58 -0600
Subject: [PATCH 29/39] Clarify a comment about popping the next request off
 the queue.

---
 .../base/locators/composite/cachingLocator.ts                  | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 374cad5e1bab..95bc1e2e6e33 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -552,7 +552,8 @@ class BackgroundLooper {
         while (this.queue.length > 0) {
             const reqID = this.queue[0];
             this.running = reqID;
-            // We pop the request off the queue early because ....?
+            // We pop the request off the queue here so it doesn't show
+            // up as both running and pending.
             this.queue.shift();
             const [run, , notify] = this.requests[reqID];
 

From b091801b69fe9249eb82e72ab3231ac1d2071e3e Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 30 Sep 2020 15:04:30 -0600
Subject: [PATCH 30/39] Drop the retry/periodic code.

---
 .../base/locators/composite/cachingLocator.ts | 150 +-----------------
 1 file changed, 2 insertions(+), 148 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 95bc1e2e6e33..64d787117e4a 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -6,7 +6,7 @@
 
 import { Event } from 'vscode';
 import '../../../../common/extensions';
-import { createDeferred, sleep } from '../../../../common/utils/async';
+import { createDeferred } from '../../../../common/utils/async';
 import { logWarning } from '../../../../logging';
 import { IEnvsCache } from '../../envsCache';
 import { PythonEnvInfo } from '../../info';
@@ -20,37 +20,12 @@ import { getEnvs, getQueryFilter } from '../../locatorUtils';
 import { PythonEnvsChangedEvent, PythonEnvsWatcher } from '../../watcher';
 import { pickBestEnv } from './reducingLocator';
 
-type CachingLocatorOptions = {
-    refreshMinutes: number,
-    refreshRetryMinutes: number,
-};
-
-// Set defaults and otherwise adjust values.
-function normalizeCachingLocatorOptions(
-    opts: Partial<CachingLocatorOptions>,
-    defaults: CachingLocatorOptions = {
-        refreshMinutes: 24 * 60, // 1 day
-        refreshRetryMinutes: 10,
-    },
-): CachingLocatorOptions {
-    const normalized = { ...opts };
-    if (normalized.refreshMinutes === undefined) {
-        normalized.refreshMinutes = defaults.refreshMinutes;
-    }
-    if (normalized.refreshRetryMinutes === undefined) {
-        normalized.refreshRetryMinutes = defaults.refreshRetryMinutes;
-    }
-    return normalized as CachingLocatorOptions;
-}
-
 /**
  * A locator that stores the known environments in the given cache.
  */
 export class CachingLocator implements ILocator {
     public readonly onChanged: Event<PythonEnvsChangedEvent>;
 
-    private readonly opts: CachingLocatorOptions;
-
     private readonly watcher = new PythonEnvsWatcher();
 
     private readonly initializing = createDeferred<void>();
@@ -62,22 +37,11 @@ export class CachingLocator implements ILocator {
     constructor(
         private readonly cache: IEnvsCache,
         private readonly locator: ILocator,
-        opts: {
-            refreshMinutes?: number,
-            refreshRetryMinutes?: number,
-        } = {},
     ) {
         this.onChanged = this.watcher.onChanged;
-        this.opts = normalizeCachingLocatorOptions(opts);
         // eslint-disable-next-line @typescript-eslint/no-use-before-define
         this.looper = new BackgroundLooper({
             runDefault: () => this.refresh(),
-            retry: {
-                intervalms: this.opts.refreshRetryMinutes * 60 * 1000,
-            },
-            periodic: {
-                intervalms: this.opts.refreshMinutes * 60 * 1000,
-            },
         });
     }
 
@@ -265,63 +229,6 @@ type RequestID = number;
 type RunFunc = () => Promise<void>;
 type NotifyFunc = () => void;
 
-type RetryOptions = {
-    maxRetries: number;
-    intervalms: number;
-};
-
-// Set defaults and otherwise adjust values.
-function normalizeRetryOptions(
-    opts: Partial<RetryOptions> | undefined,
-    defaults: RetryOptions = {
-        maxRetries: 3,
-        intervalms: 100,
-    },
-): RetryOptions | undefined {
-    if (opts === undefined) {
-        return undefined;
-    }
-    const normalized = { ...opts };
-    if (normalized.maxRetries === undefined) {
-        normalized.maxRetries = defaults.maxRetries;
-    } else if (normalized.maxRetries < 0) {
-        // This is effectively infinity.
-        normalized.maxRetries = Number.MAX_SAFE_INTEGER;
-    }
-    if (normalized.intervalms === undefined) {
-        normalized.intervalms = defaults.intervalms;
-    }
-    return normalized as RetryOptions;
-}
-
-type PeriodicOptions = {
-    intervalms: number;
-    initialTimestamp: number;
-};
-
-function normalizePeriodicOptions(
-    opts: Partial<PeriodicOptions> | undefined,
-    defaults: PeriodicOptions = {
-        intervalms: -1,
-        initialTimestamp: -1,
-    },
-): PeriodicOptions | undefined {
-    if (opts === undefined) {
-        return undefined;
-    }
-    const normalized = { ...opts };
-    if (normalized.intervalms === undefined) {
-        // "never run"
-        normalized.intervalms = defaults.intervalms;
-    }
-    if (normalized.initialTimestamp === undefined && normalized.intervalms > -1) {
-        normalized.initialTimestamp = Date.now() + normalized.intervalms;
-    } else {
-        normalized.initialTimestamp = defaults.initialTimestamp;
-    }
-    return normalized as PeriodicOptions;
-}
-
 /**
  * This helps avoid running duplicate expensive operations.
  *
@@ -331,8 +238,6 @@ function normalizePeriodicOptions(
 class BackgroundLooper {
     private readonly opts: {
         runDefault: RunFunc;
-        retry?: RetryOptions;
-        periodic?: PeriodicOptions;
     };
 
     private started = false;
@@ -354,25 +259,16 @@ class BackgroundLooper {
 
     private lastID: number | undefined;
 
-    private nextPeriod = -1;
-
     constructor(
         opts: {
             runDefault?: RunFunc;
-            retry?: Partial<RetryOptions>;
-            periodic?: Partial<PeriodicOptions>;
         } = {},
     ) {
         this.opts = {
             runDefault: opts.runDefault !== undefined
                 ? opts.runDefault
                 : async () => { throw Error('no default operation provided'); },
-            retry: normalizeRetryOptions(opts.retry),
-            periodic: normalizePeriodicOptions(opts.periodic),
         };
-        if (this.opts.periodic !== undefined) {
-            this.nextPeriod = this.opts.periodic.initialTimestamp;
-        }
     }
 
     /**
@@ -506,12 +402,6 @@ class BackgroundLooper {
                 this.done.promise.then(() => 0),
                 this.waitUntilReady.promise.then(() => 1),
             ];
-            if (this.opts.periodic !== undefined && this.nextPeriod > -1) {
-                const msLeft = Math.max(0, this.nextPeriod - Date.now());
-                promises.push(
-                    sleep(msLeft).then(() => 2),
-                );
-            }
             return Promise.race(promises);
         };
 
@@ -521,12 +411,6 @@ class BackgroundLooper {
                 this.waitUntilReady = createDeferred<void>();
                 // eslint-disable-next-line no-await-in-loop
                 await this.flush();
-            } else if (winner === 2) {
-                // We reset the period before queueing to avoid any races.
-                this.nextPeriod = Date.now() + this.opts.periodic!.intervalms;
-                // Rather than running the request directly, we add
-                // it to the queue.  This avoids races.
-                this.addRequest(this.opts.runDefault);
             } else {
                 // This should not be reachable.
                 throw Error(`unsupported winner ${winner}`);
@@ -558,7 +442,7 @@ class BackgroundLooper {
             const [run, , notify] = this.requests[reqID];
 
             // eslint-disable-next-line no-await-in-loop
-            await this.runRequest(run);
+            await run();
 
             // We leave the request until right before `notify()`
             // for the sake of any calls to `getLastRequest()`.
@@ -568,36 +452,6 @@ class BackgroundLooper {
         this.running = undefined;
     }
 
-    /**
-     * Run a single request.
-     */
-    private async runRequest(run: RunFunc): Promise<void> {
-        if (this.opts.retry === undefined) {
-            // eslint-disable-next-line no-await-in-loop
-            await run();
-            return;
-        }
-        let retriesLeft = this.opts.retry.maxRetries;
-        const retryIntervalms = this.opts.retry.intervalms;
-        let retrying = false;
-        do {
-            try {
-                // eslint-disable-next-line no-await-in-loop
-                await run();
-            } catch (err) {
-                if (retriesLeft < 1) {
-                    throw err; // re-trhow
-                }
-                retriesLeft -= 1;
-                logWarning(`failed while handling request (${err})`);
-                logWarning(`retrying (${retriesLeft} attempts left)`);
-                // eslint-disable-next-line no-await-in-loop
-                await sleep(retryIntervalms);
-                retrying = true;
-            }
-        } while (!retrying);
-    }
-
     /**
      * Provide the request ID to use next.
      */

From 0c90c6f0c5086e6f5236f3c6586df0f9d07cbc59 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 7 Oct 2020 11:26:00 -0600
Subject: [PATCH 31/39] Fix typos.

---
 src/client/pythonEnvironments/base/envsCache.ts             | 2 +-
 .../base/locators/composite/cachingLocator.ts               | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/client/pythonEnvironments/base/envsCache.ts b/src/client/pythonEnvironments/base/envsCache.ts
index 4c763dc7b76d..1eef95f4204e 100644
--- a/src/client/pythonEnvironments/base/envsCache.ts
+++ b/src/client/pythonEnvironments/base/envsCache.ts
@@ -29,7 +29,7 @@ export interface IEnvsCache {
     setAllEnvs(envs: PythonEnvInfo[]): void;
 
     /**
-     * If the cache has been initialized, return environmnent info objects that match a query object.
+     * If the cache has been initialized, return environment info objects that match a query object.
      * If none of the environments in the cache match the query data, return an empty array.
      * If the in-memory cache has not been initialized prior to calling `filterEnvs`, return `undefined`.
      *
diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 64d787117e4a..272c435cdd12 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -221,7 +221,7 @@ export class CachingLocator implements ILocator {
         // If necessary, we could skip if there are no changes.
         this.cache.setAllEnvs(envs);
         await this.cache.flush();
-        this.watcher.fire(event || {}); // Emit an "onCHanged" event.
+        this.watcher.fire(event || {}); // Emit an "onChanged" event.
     }
 }
 
@@ -232,7 +232,7 @@ type NotifyFunc = () => void;
 /**
  * This helps avoid running duplicate expensive operations.
  *
- * The key aspect is that alraedy running or queue requests can be
+ * The key aspect is that already running or queue requests can be
  * re-used instead of creating a duplicate request.
  */
 class BackgroundLooper {
@@ -305,7 +305,7 @@ class BackgroundLooper {
         this.done.resolve();
 
         // It is conceivable that a separate "waitUntilStopped"
-        // operation would be useful.  If it turned out to be desireable
+        // operation would be useful.  If it turned out to be desirable
         // then at the point we could add such a method separately.
         // It would do nothing more than `await this.loopRunning`.
         // Currently there is no need for a separate method since

From 6bc3b006141095cc662014b3b1a20dbf4e929b42 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 7 Oct 2020 11:38:23 -0600
Subject: [PATCH 32/39] Drop unnecessary eslint directives.

---
 .../base/locators/composite/cachingLocator.ts                 | 4 ----
 src/client/pythonEnvironments/index.ts                        | 2 --
 2 files changed, 6 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 272c435cdd12..7a68faa35d85 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -130,7 +130,6 @@ export class CachingLocator implements ILocator {
         let res = await iterator.next();
         while (!res.done) {
             yield res.value;
-            // eslint-disable-next-line no-await-in-loop
             res = await iterator.next();
         }
     }
@@ -409,13 +408,11 @@ class BackgroundLooper {
         while (!this.done.completed) {
             if (winner === 1) {
                 this.waitUntilReady = createDeferred<void>();
-                // eslint-disable-next-line no-await-in-loop
                 await this.flush();
             } else {
                 // This should not be reachable.
                 throw Error(`unsupported winner ${winner}`);
             }
-            // eslint-disable-next-line no-await-in-loop
             winner = await getWinner();
         }
         this.loopRunning.resolve();
@@ -441,7 +438,6 @@ class BackgroundLooper {
             this.queue.shift();
             const [run, , notify] = this.requests[reqID];
 
-            // eslint-disable-next-line no-await-in-loop
             await run();
 
             // We leave the request until right before `notify()`
diff --git a/src/client/pythonEnvironments/index.ts b/src/client/pythonEnvironments/index.ts
index b67427bac4c6..9464482a6355 100644
--- a/src/client/pythonEnvironments/index.ts
+++ b/src/client/pythonEnvironments/index.ts
@@ -103,11 +103,9 @@ function getWorkspaceFolders() {
     const rootAdded = new vscode.EventEmitter<vscode.Uri>();
     const rootRemoved = new vscode.EventEmitter<vscode.Uri>();
     vscode.workspace.onDidChangeWorkspaceFolders((event) => {
-        // eslint-disable-next-line no-restricted-syntax
         for (const root of event.removed) {
             rootRemoved.fire(root.uri);
         }
-        // eslint-disable-next-line no-restricted-syntax
         for (const root of event.added) {
             rootAdded.fire(root.uri);
         }

From 89b6e5ee9fd00c7145e60afa34bb77391e8e3db4 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 7 Oct 2020 12:00:43 -0600
Subject: [PATCH 33/39] Move BackgroundRequestLooper to its own file.

---
 src/client/common/utils/backgroundLoop.ts     | 248 +++++++++++++++++
 .../base/locators/composite/cachingLocator.ts | 249 +-----------------
 tslint.json                                   |   3 +-
 3 files changed, 252 insertions(+), 248 deletions(-)
 create mode 100644 src/client/common/utils/backgroundLoop.ts

diff --git a/src/client/common/utils/backgroundLoop.ts b/src/client/common/utils/backgroundLoop.ts
new file mode 100644
index 000000000000..7086b4222a8f
--- /dev/null
+++ b/src/client/common/utils/backgroundLoop.ts
@@ -0,0 +1,248 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import { createDeferred } from './async';
+
+type RequestID = number;
+type RunFunc = () => Promise<void>;
+type NotifyFunc = () => void;
+
+/**
+ * This helps avoid running duplicate expensive operations.
+ *
+ * The key aspect is that already running or queue requests can be
+ * re-used instead of creating a duplicate request.
+ */
+export class BackgroundRequestLooper {
+    private readonly opts: {
+        runDefault: RunFunc;
+    };
+
+    private started = false;
+
+    private stopped = false;
+
+    private readonly done = createDeferred<void>();
+
+    private readonly loopRunning = createDeferred<void>();
+
+    private waitUntilReady = createDeferred<void>();
+
+    private running: RequestID | undefined;
+
+    // For now we don't worry about a max queue size.
+    private readonly queue: RequestID[] = [];
+
+    private readonly requests: Record<RequestID, [RunFunc, Promise<void>, NotifyFunc]> = {};
+
+    private lastID: number | undefined;
+
+    constructor(
+        opts: {
+            runDefault?: RunFunc;
+        } = {}
+    ) {
+        this.opts = {
+            runDefault:
+                opts.runDefault !== undefined
+                    ? opts.runDefault
+                    : async () => {
+                          throw Error('no default operation provided');
+                      }
+        };
+    }
+
+    /**
+     * Start the request execution loop.
+     *
+     * Currently it does not support being re-started.
+     */
+    public start(): void {
+        if (this.stopped) {
+            throw Error('already stopped');
+        }
+        if (this.started) {
+            return;
+        }
+        this.started = true;
+
+        this.runLoop().ignoreErrors();
+    }
+
+    /**
+     * Stop the loop (assuming it was already started.)
+     *
+     * @returns - a promise that resolves once the loop has stopped.
+     */
+    public stop(): Promise<void> {
+        if (this.stopped) {
+            return this.loopRunning.promise;
+        }
+        if (!this.started) {
+            throw Error('not started yet');
+        }
+        this.stopped = true;
+
+        this.done.resolve();
+
+        // It is conceivable that a separate "waitUntilStopped"
+        // operation would be useful.  If it turned out to be desirable
+        // then at the point we could add such a method separately.
+        // It would do nothing more than `await this.loopRunning`.
+        // Currently there is no need for a separate method since
+        // returning the promise here is sufficient.
+        return this.loopRunning.promise;
+    }
+
+    /**
+     * Return the most recent active request, if any.
+     *
+     * If there are no pending requests then this is the currently
+     * running one (if one is running).
+     *
+     * @returns - the ID of the request and its completion promise;
+     *            if there are no active requests then you get `undefined`
+     */
+    public getLastRequest(): [RequestID, Promise<void>] | undefined {
+        let reqID: RequestID;
+        if (this.queue.length > 0) {
+            reqID = this.queue[this.queue.length - 1];
+        } else if (this.running !== undefined) {
+            reqID = this.running;
+        } else {
+            return undefined;
+        }
+        // The req cannot be undefined since every queued ID has a request.
+        const [, promise] = this.requests[reqID];
+        if (reqID === undefined) {
+            // The queue must be empty.
+            return undefined;
+        }
+        return [reqID, promise];
+    }
+
+    /**
+     * Return the request that is waiting to run next, if any.
+     *
+     * The request is the next one that will be run.  This implies that
+     * there is one already running.
+     *
+     * @returns - the ID of the request and its completion promise;
+     *            if there are no pending requests then you get `undefined`
+     */
+    public getNextRequest(): [RequestID, Promise<void>] | undefined {
+        if (this.queue.length === 0) {
+            return undefined;
+        }
+        const reqID = this.queue[0];
+        // The req cannot be undefined since every queued ID has a request.
+        const [, promise] = this.requests[reqID]!;
+        return [reqID, promise];
+    }
+
+    /**
+     * Request that a function be run.
+     *
+     * If one is already running then the new request is added to the
+     * end of the queue.  Otherwise it is run immediately.
+     *
+     * @returns - the ID of the new request and its completion promise;
+     *            the promise resolves once the request has completed
+     */
+    public addRequest(run?: RunFunc): [RequestID, Promise<void>] {
+        const reqID = this.getNextID();
+        // This is the only method that adds requests to the queue
+        // and `getNextID()` keeps us from having collisions here.
+        // So we are guaranteed that there are no matching requests
+        // in the queue.
+        const running = createDeferred<void>();
+        this.requests[reqID] = [
+            // [RunFunc, "done" promise, NotifyFunc]
+            run ?? this.opts.runDefault,
+            running.promise,
+            () => running.resolve()
+        ];
+        this.queue.push(reqID);
+        if (this.queue.length === 1) {
+            // `waitUntilReady` will get replaced with a new deferred
+            // in the loop once the existing one gets used.
+            // We let the queue clear out before triggering the loop
+            // again.
+            this.waitUntilReady.resolve();
+        }
+        return [reqID, running.promise];
+    }
+
+    /**
+     * This is the actual loop where the queue is managed and waiting happens.
+     */
+    private async runLoop(): Promise<void> {
+        const getWinner = () => {
+            const promises = [
+                // These are the competing operations.
+                // Note that the losers keep running in the background.
+                this.done.promise.then(() => 0),
+                this.waitUntilReady.promise.then(() => 1)
+            ];
+            return Promise.race(promises);
+        };
+
+        let winner = await getWinner();
+        while (!this.done.completed) {
+            if (winner === 1) {
+                this.waitUntilReady = createDeferred<void>();
+                await this.flush();
+            } else {
+                // This should not be reachable.
+                throw Error(`unsupported winner ${winner}`);
+            }
+            winner = await getWinner();
+        }
+        this.loopRunning.resolve();
+    }
+
+    /**
+     * Run all pending requests, in queue order.
+     *
+     * Each request's completion promise resolves once that request
+     * finishes.
+     */
+    private async flush(): Promise<void> {
+        if (this.running !== undefined) {
+            // We must be flushing the queue already.
+            return;
+        }
+        // Run every request in the queue.
+        while (this.queue.length > 0) {
+            const reqID = this.queue[0];
+            this.running = reqID;
+            // We pop the request off the queue here so it doesn't show
+            // up as both running and pending.
+            this.queue.shift();
+            const [run, , notify] = this.requests[reqID];
+
+            await run();
+
+            // We leave the request until right before `notify()`
+            // for the sake of any calls to `getLastRequest()`.
+            delete this.requests[reqID];
+            notify();
+        }
+        this.running = undefined;
+    }
+
+    /**
+     * Provide the request ID to use next.
+     */
+    private getNextID(): RequestID {
+        // For now there is no way to queue up a request with
+        // an ID that did not originate here.  So we don't need
+        // to worry about collisions.
+        if (this.lastID === undefined) {
+            this.lastID = 1;
+        } else {
+            this.lastID += 1;
+        }
+        return this.lastID;
+    }
+}
diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 7a68faa35d85..266751b39aa4 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -1,12 +1,10 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-// tslint:disable-next-line: no-single-line-block-comment
-/* eslint-disable max-classes-per-file */
-
 import { Event } from 'vscode';
 import '../../../../common/extensions';
 import { createDeferred } from '../../../../common/utils/async';
+import { BackgroundRequestLooper } from '../../../../common/utils/backgroundLoop';
 import { logWarning } from '../../../../logging';
 import { IEnvsCache } from '../../envsCache';
 import { PythonEnvInfo } from '../../info';
@@ -32,15 +30,14 @@ export class CachingLocator implements ILocator {
 
     private initialized = false;
 
-    private looper: BackgroundLooper;
+    private looper: BackgroundRequestLooper;
 
     constructor(
         private readonly cache: IEnvsCache,
         private readonly locator: ILocator,
     ) {
         this.onChanged = this.watcher.onChanged;
-        // eslint-disable-next-line @typescript-eslint/no-use-before-define
-        this.looper = new BackgroundLooper({
+        this.looper = new BackgroundRequestLooper({
             runDefault: () => this.refresh(),
         });
     }
@@ -223,243 +220,3 @@ export class CachingLocator implements ILocator {
         this.watcher.fire(event || {}); // Emit an "onChanged" event.
     }
 }
-
-type RequestID = number;
-type RunFunc = () => Promise<void>;
-type NotifyFunc = () => void;
-
-/**
- * This helps avoid running duplicate expensive operations.
- *
- * The key aspect is that already running or queue requests can be
- * re-used instead of creating a duplicate request.
- */
-class BackgroundLooper {
-    private readonly opts: {
-        runDefault: RunFunc;
-    };
-
-    private started = false;
-
-    private stopped = false;
-
-    private readonly done = createDeferred<void>();
-
-    private readonly loopRunning = createDeferred<void>();
-
-    private waitUntilReady = createDeferred<void>();
-
-    private running: RequestID | undefined;
-
-    // For now we don't worry about a max queue size.
-    private readonly queue: RequestID[] = [];
-
-    private readonly requests: Record<RequestID, [RunFunc, Promise<void>, NotifyFunc]> = {};
-
-    private lastID: number | undefined;
-
-    constructor(
-        opts: {
-            runDefault?: RunFunc;
-        } = {},
-    ) {
-        this.opts = {
-            runDefault: opts.runDefault !== undefined
-                ? opts.runDefault
-                : async () => { throw Error('no default operation provided'); },
-        };
-    }
-
-    /**
-     * Start the request execution loop.
-     *
-     * Currently it does not support being re-started.
-     */
-    public start(): void {
-        if (this.stopped) {
-            throw Error('already stopped');
-        }
-        if (this.started) {
-            return;
-        }
-        this.started = true;
-
-        this.runLoop().ignoreErrors();
-    }
-
-    /**
-     * Stop the loop (assuming it was already started.)
-     *
-     * @returns - a promise that resolves once the loop has stopped.
-     */
-    public stop(): Promise<void> {
-        if (this.stopped) {
-            return this.loopRunning.promise;
-        }
-        if (!this.started) {
-            throw Error('not started yet');
-        }
-        this.stopped = true;
-
-        this.done.resolve();
-
-        // It is conceivable that a separate "waitUntilStopped"
-        // operation would be useful.  If it turned out to be desirable
-        // then at the point we could add such a method separately.
-        // It would do nothing more than `await this.loopRunning`.
-        // Currently there is no need for a separate method since
-        // returning the promise here is sufficient.
-        return this.loopRunning.promise;
-    }
-
-    /**
-     * Return the most recent active request, if any.
-     *
-     * If there are no pending requests then this is the currently
-     * running one (if one is running).
-     *
-     * @returns - the ID of the request and its completion promise;
-     *            if there are no active requests then you get `undefined`
-     */
-    public getLastRequest(): [RequestID, Promise<void>] | undefined {
-        let reqID: RequestID;
-        if (this.queue.length > 0) {
-            reqID = this.queue[this.queue.length - 1];
-        } else if (this.running !== undefined) {
-            reqID = this.running;
-        } else {
-            return undefined;
-        }
-        // The req cannot be undefined since every queued ID has a request.
-        // eslint-disable-next-line comma-dangle,comma-spacing
-        const [, promise,] = this.requests[reqID];
-        if (reqID === undefined) {
-            // The queue must be empty.
-            return undefined;
-        }
-        return [reqID, promise];
-    }
-
-    /**
-     * Return the request that is waiting to run next, if any.
-     *
-     * The request is the next one that will be run.  This implies that
-     * there is one already running.
-     *
-     * @returns - the ID of the request and its completion promise;
-     *            if there are no pending requests then you get `undefined`
-     */
-    public getNextRequest(): [RequestID, Promise<void>] | undefined {
-        if (this.queue.length === 0) {
-            return undefined;
-        }
-        const reqID = this.queue[0];
-        // The req cannot be undefined since every queued ID has a request.
-        // eslint-disable-next-line comma-dangle,comma-spacing
-        const [, promise,] = this.requests[reqID]!;
-        return [reqID, promise];
-    }
-
-    /**
-     * Request that a function be run.
-     *
-     * If one is already running then the new request is added to the
-     * end of the queue.  Otherwise it is run immediately.
-     *
-     * @returns - the ID of the new request and its completion promise;
-     *            the promise resolves once the request has completed
-     */
-    public addRequest(run?: RunFunc): [RequestID, Promise<void>] {
-        const reqID = this.getNextID();
-        // This is the only method that adds requests to the queue
-        // and `getNextID()` keeps us from having collisions here.
-        // So we are guaranteed that there are no matching requests
-        // in the queue.
-        const running = createDeferred<void>();
-        this.requests[reqID] = [
-            run ?? this.opts.runDefault,
-            running.promise,
-            () => running.resolve(),
-        ];
-        this.queue.push(reqID);
-        if (this.queue.length === 1) {
-            // `waitUntilReady` will get replaced with a new deferred
-            // in the loop once the existing one gets used.
-            // We let the queue clear out before triggering the loop
-            // again.
-            this.waitUntilReady.resolve();
-        }
-        return [reqID, running.promise];
-    }
-
-    /**
-     * This is the actual loop where the queue is managed and waiting happens.
-     */
-    private async runLoop(): Promise<void> {
-        const getWinner = () => {
-            const promises = [
-                this.done.promise.then(() => 0),
-                this.waitUntilReady.promise.then(() => 1),
-            ];
-            return Promise.race(promises);
-        };
-
-        let winner = await getWinner();
-        while (!this.done.completed) {
-            if (winner === 1) {
-                this.waitUntilReady = createDeferred<void>();
-                await this.flush();
-            } else {
-                // This should not be reachable.
-                throw Error(`unsupported winner ${winner}`);
-            }
-            winner = await getWinner();
-        }
-        this.loopRunning.resolve();
-    }
-
-    /**
-     * Run all pending requests, in queue order.
-     *
-     * Each request's completion promise resolves once that request
-     * finishes.
-     */
-    private async flush(): Promise<void> {
-        if (this.running !== undefined) {
-            // We must be flushing the queue already.
-            return;
-        }
-        // Run every request in the queue.
-        while (this.queue.length > 0) {
-            const reqID = this.queue[0];
-            this.running = reqID;
-            // We pop the request off the queue here so it doesn't show
-            // up as both running and pending.
-            this.queue.shift();
-            const [run, , notify] = this.requests[reqID];
-
-            await run();
-
-            // We leave the request until right before `notify()`
-            // for the sake of any calls to `getLastRequest()`.
-            delete this.requests[reqID];
-            notify();
-        }
-        this.running = undefined;
-    }
-
-    /**
-     * Provide the request ID to use next.
-     */
-    private getNextID(): RequestID {
-        // For now there is no way to queue up a request with
-        // an ID that did not originate here.  So we don't need
-        // to worry about collisions.
-        if (this.lastID === undefined) {
-            this.lastID = 1;
-        } else {
-            this.lastID += 1;
-        }
-        return this.lastID;
-    }
-}
diff --git a/tslint.json b/tslint.json
index bce2aad44748..0b83af565aa2 100644
--- a/tslint.json
+++ b/tslint.json
@@ -52,7 +52,6 @@
         "no-unnecessary-type-assertion": false,
         "no-submodule-imports": false,
         "no-redundant-jsdoc": false,
-        "binary-expression-operand-order": false,
-        "no-use-before-declare": false
+        "binary-expression-operand-order": false
     }
 }

From 5fd0141504ce2e7ca6932825b7fe177553fa7dce Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 7 Oct 2020 12:03:02 -0600
Subject: [PATCH 34/39] Drop a dead comment.

---
 src/client/pythonEnvironments/index.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/client/pythonEnvironments/index.ts b/src/client/pythonEnvironments/index.ts
index 9464482a6355..600c9aafd6c0 100644
--- a/src/client/pythonEnvironments/index.ts
+++ b/src/client/pythonEnvironments/index.ts
@@ -64,7 +64,6 @@ export function createAPI(): [PythonEnvironments, () => void] {
             };
         },
     );
-    // XXX For now we use a noop cache.
     const cachingLocator = new CachingLocator(envsCache, locators);
 
     return [

From 68a618fadb55200dcaf367dab54d48d5513897fd Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 7 Oct 2020 12:09:02 -0600
Subject: [PATCH 35/39] downstream -> wrapped

---
 .../base/locators/composite/cachingLocator.ts     | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 266751b39aa4..0eff1fa82bae 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -82,7 +82,7 @@ export class CachingLocator implements ILocator {
         // We assume that `getAllEnvs()` is cheap enough that calling
         // it again in `iterFromCache()` is not a problem.
         if (this.cache.getAllEnvs() === undefined) {
-            return this.iterFromDownstream(query);
+            return this.iterFromWrappedLocator(query);
         }
         return this.iterFromCache(query);
     }
@@ -114,11 +114,11 @@ export class CachingLocator implements ILocator {
     }
 
     /**
-     * A generator that yields the envs provided by the downstream locator.
+     * A generator that yields the envs provided by the wrapped locator.
      *
      * Contrast this with `iterFromCache()` that yields only from the cache.
      */
-    private async* iterFromDownstream(query?: PythonLocatorQuery): IPythonEnvsIterator {
+    private async* iterFromWrappedLocator(query?: PythonLocatorQuery): IPythonEnvsIterator {
         // For now we wait for the initial refresh to finish.  If that
         // turns out to be a problem then we can do something more
         // clever here.
@@ -134,8 +134,7 @@ export class CachingLocator implements ILocator {
     /**
      * A generator that yields the envs found in the cache.
      *
-     * Contrast this with `iterFromDownstream()` which relies on
-     * the downstream locator.
+     * Contrast this with `iterFromWrappedLocator()`.
      */
     private async* iterFromCache(query?: PythonLocatorQuery): IPythonEnvsIterator {
         const envs = this.cache.getAllEnvs();
@@ -155,7 +154,7 @@ export class CachingLocator implements ILocator {
     }
 
     /**
-     * Maybe trigger a refresh of the cache from the downstream locator.
+     * Maybe trigger a refresh of the cache from the wrapped locator.
      *
      * If a refresh isn't already running then we request a refresh and
      * wait for it to finish.  Otherwise we do not make a new request,
@@ -174,7 +173,7 @@ export class CachingLocator implements ILocator {
     }
 
     /**
-     * Maybe trigger a refresh of the cache from the downstream locator.
+     * Maybe trigger a refresh of the cache from the wrapped locator.
      *
      * Make sure that a completely new refresh will be started soon and
      * wait for it to finish.  If a refresh isn't already running then
@@ -195,7 +194,7 @@ export class CachingLocator implements ILocator {
     }
 
     /**
-     * Immediately perform a refresh of the cache from the downstream locator.
+     * Immediately perform a refresh of the cache from the wrapped locator.
      *
      * It does not matter if another refresh is already running.
      */

From adacb7cef48d669a59fcf9814005befa6e9492e8 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 7 Oct 2020 12:45:31 -0600
Subject: [PATCH 36/39] refresh() -> addRefreshRequest()

---
 src/client/common/utils/backgroundLoop.ts     | 13 +++++----
 .../base/locators/composite/cachingLocator.ts | 27 ++++++++++++-------
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/client/common/utils/backgroundLoop.ts b/src/client/common/utils/backgroundLoop.ts
index 7086b4222a8f..5ecd3a375713 100644
--- a/src/client/common/utils/backgroundLoop.ts
+++ b/src/client/common/utils/backgroundLoop.ts
@@ -39,16 +39,15 @@ export class BackgroundRequestLooper {
 
     constructor(
         opts: {
-            runDefault?: RunFunc;
+            runDefault?: RunFunc | null;
         } = {}
     ) {
         this.opts = {
-            runDefault:
-                opts.runDefault !== undefined
-                    ? opts.runDefault
-                    : async () => {
-                          throw Error('no default operation provided');
-                      }
+            runDefault: opts.runDefault
+                ? opts.runDefault
+                : async () => {
+                      throw Error('no default operation provided');
+                  }
         };
     }
 
diff --git a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
index 0eff1fa82bae..511dc1b8d4c2 100644
--- a/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
+++ b/src/client/pythonEnvironments/base/locators/composite/cachingLocator.ts
@@ -38,7 +38,7 @@ export class CachingLocator implements ILocator {
     ) {
         this.onChanged = this.watcher.onChanged;
         this.looper = new BackgroundRequestLooper({
-            runDefault: () => this.refresh(),
+            runDefault: null,
         });
     }
 
@@ -168,8 +168,7 @@ export class CachingLocator implements ILocator {
             return promise;
         }
         // The queue is empty so add a new request.
-        const [, waitUntilDone] = this.looper.addRequest(() => this.refresh());
-        return waitUntilDone;
+        return this.addRefreshRequest();
     }
 
     /**
@@ -188,22 +187,30 @@ export class CachingLocator implements ILocator {
         if (req === undefined) {
             // There isn't already a pending request (due to an
             // onChanged event), so we add one.
-            this.looper.addRequest(() => this.refresh(event));
+            this.addRefreshRequest(event)
+                .ignoreErrors();
         }
         // Otherwise let the pending request take care of it.
     }
 
     /**
-     * Immediately perform a refresh of the cache from the wrapped locator.
+     * Queue up a new request to refresh the cache from the wrapped locator.
      *
-     * It does not matter if another refresh is already running.
+     * Once the request is added, that refresh will run no matter what
+     * at some future point (possibly immediately).  It does not matter
+     * if another refresh is already running.  You probably want to use
+     * `ensureRecentRefresh()` or * `ensureCurrentRefresh()` instead,
+     * to avoid unnecessary refreshes.
      */
-    private async refresh(
+    private addRefreshRequest(
         event?: PythonEnvsChangedEvent,
     ): Promise<void> {
-        const iterator = this.locator.iterEnvs();
-        const envs = await getEnvs(iterator);
-        await this.updateCache(envs, event);
+        const [, waitUntilDone] = this.looper.addRequest(async () => {
+            const iterator = this.locator.iterEnvs();
+            const envs = await getEnvs(iterator);
+            await this.updateCache(envs, event);
+        });
+        return waitUntilDone;
     }
 
     /**

From e8914f6d9968653d519269511f60e960534cf2d0 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 7 Oct 2020 12:53:58 -0600
Subject: [PATCH 37/39] Clarify a potentially confusing situation in
 getGlobalPersistentStore().

---
 src/test/pythonEnvironments/base/envsCache.unit.test.ts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/test/pythonEnvironments/base/envsCache.unit.test.ts b/src/test/pythonEnvironments/base/envsCache.unit.test.ts
index 3ec81ff77e27..b0dcb252629d 100644
--- a/src/test/pythonEnvironments/base/envsCache.unit.test.ts
+++ b/src/test/pythonEnvironments/base/envsCache.unit.test.ts
@@ -36,6 +36,11 @@ suite('Environment Info cache', () => {
     });
 
     function getGlobalPersistentStore() {
+        // It may look like we are making this call directly, but note
+        // that in `setup()` we have already stubbed the function out.
+        // We take this approach so the tests more closely match how
+        // `PythonEnvInfoCache` will actually be used in the VS Code
+        // extension.
         const store = externalDependencies.getGlobalPersistentStore<PythonEnvInfo[]>('PYTHON_ENV_INFO_CACHE');
         return {
             load: async () => store.get(),

From 134915bb336d9ee11f477b19ae236807c48b64d7 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed, 7 Oct 2020 17:29:59 -0600
Subject: [PATCH 38/39] Use a more concise syntax.

---
 src/client/common/utils/backgroundLoop.ts | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/client/common/utils/backgroundLoop.ts b/src/client/common/utils/backgroundLoop.ts
index 5ecd3a375713..bd7cc5f407f8 100644
--- a/src/client/common/utils/backgroundLoop.ts
+++ b/src/client/common/utils/backgroundLoop.ts
@@ -43,11 +43,10 @@ export class BackgroundRequestLooper {
         } = {}
     ) {
         this.opts = {
-            runDefault: opts.runDefault
-                ? opts.runDefault
-                : async () => {
-                      throw Error('no default operation provided');
-                  }
+            runDefault:
+                opts.runDefault ?? (async () => {
+                    throw Error('no default operation provided');
+                })
         };
     }
 

From fa63318a62292704ee3dc7c82ff23272c318a7e2 Mon Sep 17 00:00:00 2001
From: Eric Snow <ericsnowcurrently@gmail.com>
Date: Thu, 8 Oct 2020 13:06:03 -0600
Subject: [PATCH 39/39] lint

---
 src/client/common/utils/backgroundLoop.ts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/client/common/utils/backgroundLoop.ts b/src/client/common/utils/backgroundLoop.ts
index bd7cc5f407f8..f7cc78c51b73 100644
--- a/src/client/common/utils/backgroundLoop.ts
+++ b/src/client/common/utils/backgroundLoop.ts
@@ -44,7 +44,8 @@ export class BackgroundRequestLooper {
     ) {
         this.opts = {
             runDefault:
-                opts.runDefault ?? (async () => {
+                opts.runDefault ??
+                (async () => {
                     throw Error('no default operation provided');
                 })
         };