Fix bug when all extensions are deactivated and reactived (#1036)

paranext · Aug 1, 2024 · 3210013 · 3210013
1 parent b6d1191
commit 3210013
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 19 deletions.
diff --git a/extensions/src/platform-scripture/src/checks/extension-host-check-runner.service.ts b/extensions/src/platform-scripture/src/checks/extension-host-check-runner.service.ts
@@ -164,7 +164,7 @@ class CheckRunnerEngine
   async disableCheck(checkId: string, projectId?: string): Promise<void> {
     const lock = this.mutexesPerCheck.get(checkId);
     await lock.runExclusive(async () => {
-      const checkDisposalList = new UnsubscriberAsyncList();
+      const checksToDispose: Check[] = [];
 
       this.enabledChecksByProjectId.forEach((enabledChecks, enabledProjectId) => {
         const relevantCheckIndex = enabledChecks.findIndex(
@@ -173,14 +173,18 @@ class CheckRunnerEngine
         );
         if (relevantCheckIndex < 0) return;
 
-        // Remove the check from the list of enabled checks
+        // Remove the check from the list of enabled checks and prepare to dispose of it
         const relevantCheck = enabledChecks.splice(relevantCheckIndex, 1)[0];
-
-        // Prepare to tear down the check
-        checkDisposalList.add(relevantCheck.check.dispose);
+        checksToDispose.push(relevantCheck.check);
       });
 
-      await checkDisposalList.runAllUnsubscribers();
+      await Promise.all(
+        checksToDispose.map(async (check) => {
+          // The check object might be gone already if an extension was unloaded that contained it
+          if (check) await check.dispose();
+        }),
+      );
+
       this.notifyUpdate(['AvailableChecks', 'CheckResults']);
     });
   }

diff --git a/src/extension-host/services/extension.service.ts b/src/extension-host/services/extension.service.ts
@@ -979,19 +979,18 @@ async function deactivateExtension(extension: ExtensionInfo): Promise<boolean |
  * @param extensions - Extension info for the extensions we want to deactivate.
  * @returns An array of the deactivation results - `true`, `false`, or `undefined`.
  */
-function deactivateExtensions(extensions: ExtensionInfo[]): Promise<(boolean | undefined)[]> {
-  return Promise.all(
-    extensions.map(async (extension) => {
-      try {
-        const isDeactivated = await deactivateExtension(extension);
-        if (!isDeactivated) logger.error(`Extension '${extension.name}' failed to deactivate.`);
-        return isDeactivated;
-      } catch (e) {
-        logger.error(`Extension '${extension.name}' threw while deactivating! ${e}`);
-        return false;
-      }
-    }),
-  );
+async function deactivateExtensions(extensions: ExtensionInfo[]): Promise<void> {
+  // We want to deactivate extensions sequentially in opposite order of which they were activated
+  // eslint-disable-next-line no-restricted-syntax
+  for (const extension of [...extensions].reverse()) {
+    try {
+      // eslint-disable-next-line no-await-in-loop
+      const isDeactivated = await deactivateExtension(extension);
+      if (!isDeactivated) logger.error(`Extension '${extension.name}' failed to deactivate.`);
+    } catch (e) {
+      logger.error(`Extension '${extension.name}' threw while deactivating! ${e}`);
+    }
+  }
 }
 
 async function resyncContributions(