disposableFiles: fix edge cases again (#1163)

* disposableFiles: fix edge cases again

PROBLEM:
some tests implicitly depend on the global `ExtensionDisposableFiles.INSTANCE`,
other tests clear it, but it is not correctly re-initialized.

SOLUTION:
use `clearInstance()`, and rearrange its logic to avoid a race
condition. Hoist `TestExtensionDisposableFiles` into
`FakeExtensionContext` so it can use `clearInstance()`.

ANALYSIS:

Previous attempt to fix this in f143bad was incomplete, and only
"worked" by accident. Problem resurfaces in `feature/debugconfig`
because it has more tests that depend on `ExtensionDisposableFiles.INSTANCE`.

Rearrange `clearInstance()` so that
    ExtensionDisposableFiles.INSTANCE = undefined
is the first step. Because `del.sync()` does file IO, node may switch to
another context while it waits on the file IO.

Fixes this error (among others):

    1) localLambdaRunner
      "before all" hook in "localLambdaRunner":
    Error: ExtensionDisposableFiles already initialized
     at Function.<anonymous> (/Volumes/workplace/aws-toolkit-vscode/src/shared/utilities/disposableFiles.ts:78:19)
     at Generator.next (<anonymous>)
     at /Volumes/workplace/aws-toolkit-vscode/dist/src/shared/utilities/disposableFiles.js:12:71
     at new Promise (<anonymous>)
     at /Volumes/workplace/aws-toolkit-vscode/dist/src/shared/utilities/disposableFiles.js:8:12
     at Function.initialize (/Volumes/workplace/aws-toolkit-vscode/dist/src/shared/utilities/disposableFiles.js:79:16)
     at Function.<anonymous> (/Volumes/workplace/aws-toolkit-vscode/src/test/fakeExtensionContext.ts:72:44)
     at Generator.next (<anonymous>)
     at /Volumes/workplace/aws-toolkit-vscode/dist/src/test/fakeExtensionContext.js:12:71
     at new Promise (<anonymous>)
     at /Volumes/workplace/aws-toolkit-vscode/dist/src/test/fakeExtensionContext.js:8:12
     at Function.getNew (/Volumes/workplace/aws-toolkit-vscode/dist/src/test/fakeExtensionContext.js:54:16)
     at /Volumes/workplace/aws-toolkit-vscode/src/test/shared/codelens/localLambdaRunner.test.ts:22:36
     at Generator.next (<anonymous>)
     at /Volumes/workplace/aws-toolkit-vscode/dist/src/test/shared/codelens/localLambdaRunner.test.js:12:71
     at new Promise (<anonymous>)
     at /Volumes/workplace/aws-toolkit-vscode/dist/src/test/shared/codelens/localLambdaRunner.test.js:8:12
     at Context.<anonymous> (/Volumes/workplace/aws-toolkit-vscode/src/test/shared/codelens/localLambdaRunner.test.ts:21:23)
     at processImmediate (internal/timers.js:439:21)
     at process.topLevelDomainCallback (domain.js:131:23)

* fixup! disposableFiles: fix edge cases again

Author: justinmk3

src/shared/utilities/disposableFiles.ts

@@ -26,6 +26,10 @@ export class DisposableFiles implements vscode.Disposable {
         return this
     }
 
+    public isDisposed(): boolean {
+        return this._disposed
+    }
+
     public dispose(): void {
         const logger: Logger = getLogger()
         if (!this._disposed) {
@@ -70,7 +74,7 @@ export class ExtensionDisposableFiles extends DisposableFiles {
     }
 
     public static async initialize(extensionContext: vscode.ExtensionContext): Promise<void> {
-        if (!!ExtensionDisposableFiles.INSTANCE) {
+        if (ExtensionDisposableFiles.INSTANCE && !ExtensionDisposableFiles.INSTANCE.isDisposed()) {
             throw new Error('ExtensionDisposableFiles already initialized')
         }
 
@@ -82,7 +86,7 @@ export class ExtensionDisposableFiles extends DisposableFiles {
     }
 
     public static getInstance(): ExtensionDisposableFiles {
-        if (!ExtensionDisposableFiles.INSTANCE) {
+        if (!ExtensionDisposableFiles.INSTANCE || ExtensionDisposableFiles.INSTANCE.isDisposed()) {
             throw new Error('ExtensionDisposableFiles not initialized')
         }
 

src/test/fakeExtensionContext.ts

@@ -3,7 +3,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { ExtensionContext, Memento } from 'vscode'
+import * as vscode from 'vscode'
+import * as del from 'del'
 import { ExtensionDisposableFiles } from '../shared/utilities/disposableFiles'
 
 export interface FakeMementoStorage {
@@ -15,12 +16,12 @@ export interface FakeExtensionState {
     workspaceState?: FakeMementoStorage
 }
 
-export class FakeExtensionContext implements ExtensionContext {
+export class FakeExtensionContext implements vscode.ExtensionContext {
     public subscriptions: {
         dispose(): any
     }[] = []
-    public workspaceState: Memento = new FakeMemento()
-    public globalState: Memento = new FakeMemento()
+    public workspaceState: vscode.Memento = new FakeMemento()
+    public globalState: vscode.Memento = new FakeMemento()
     public storagePath: string | undefined
     public globalStoragePath: string = '.'
     public logPath: string = ''
@@ -54,15 +55,37 @@ export class FakeExtensionContext implements ExtensionContext {
     public static async getNew(): Promise<FakeExtensionContext> {
         const ctx = new FakeExtensionContext()
         try {
-            ExtensionDisposableFiles.getInstance().dispose()
+            TestExtensionDisposableFiles.clearInstance()
         } catch {
-            await ExtensionDisposableFiles.initialize(ctx)
+            // Ignore.
         }
+        await TestExtensionDisposableFiles.initialize(ctx)
         return ctx
     }
 }
 
-class FakeMemento implements Memento {
+export class TestExtensionDisposableFiles extends ExtensionDisposableFiles {
+    public static ORIGINAL_INSTANCE = ExtensionDisposableFiles.INSTANCE
+
+    public static clearInstance() {
+        // XXX: INSTANCE=undefined is done first, to avoid a race:
+        //      1. del.sync() does file IO
+        //      2. the Node scheduler looks for pending handlers to execute while waiting on IO
+        //      3. other async handlers may try to use ExtensionDisposableFiles.getInstance()
+        const instance = ExtensionDisposableFiles.INSTANCE
+        ExtensionDisposableFiles.INSTANCE = undefined
+        if (instance) {
+            del.sync([instance.toolkitTempFolder], { force: true })
+            instance.dispose()
+        }
+    }
+
+    public static resetOriginalInstance() {
+        ExtensionDisposableFiles.INSTANCE = TestExtensionDisposableFiles.ORIGINAL_INSTANCE
+    }
+}
+
+class FakeMemento implements vscode.Memento {
     public constructor(private readonly _storage: FakeMementoStorage = {}) {}
     public get<T>(key: string): T | undefined
     public get<T>(key: string, defaultValue: T): T

src/test/shared/utilities/disposableFiles.test.ts

@@ -11,6 +11,7 @@ import * as vscode from 'vscode'
 import { mkdir } from '../../../shared/filesystem'
 import { fileExists, makeTemporaryToolkitFolder } from '../../../shared/filesystemUtilities'
 import { DisposableFiles, ExtensionDisposableFiles } from '../../../shared/utilities/disposableFiles'
+import { TestExtensionDisposableFiles } from '../../fakeExtensionContext'
 
 describe('DisposableFiles', async () => {
     let tempFolder: string
@@ -87,22 +88,6 @@ describe('DisposableFiles', async () => {
 })
 
 describe('ExtensionDisposableFiles', async () => {
-    class TestExtensionDisposableFiles extends ExtensionDisposableFiles {
-        public static ORIGINAL_INSTANCE = ExtensionDisposableFiles.INSTANCE
-
-        public static clearInstance() {
-            if (ExtensionDisposableFiles.INSTANCE) {
-                del.sync([ExtensionDisposableFiles.INSTANCE.toolkitTempFolder], { force: true })
-            }
-
-            ExtensionDisposableFiles.INSTANCE = undefined
-        }
-
-        public static resetOriginalInstance() {
-            ExtensionDisposableFiles.INSTANCE = TestExtensionDisposableFiles.ORIGINAL_INSTANCE
-        }
-    }
-
     let extensionContext: vscode.ExtensionContext
 
     beforeEach(() => {