From bc00bd161a356ef8d62c414c6aba90801459be14 Mon Sep 17 00:00:00 2001
From: icezohu <princewith@gmail.com>
Date: Tue, 6 Jun 2023 10:47:50 +0800
Subject: [PATCH] fix(multip-app-manager): init multiple Application instances
 while starting up a sub app (#1986)

* fix: init multiple Application instances while
starting up sub app

by Jacob

* fix: import error

by Jacob

* feat: use async-mutex for locking process

by Jacob

* revert: unused update

by Jacob

* test: should get same obj ref
when asynchronously access with same sub app name

by Jacob

* feat: add async-mutex dependency
to multi-app-manager package.json

by Jacob
---
 .../plugins/multi-app-manager/package.json    |  3 +-
 .../server/__tests__/multiple-apps.test.ts    | 34 +++++++++++++
 .../multi-app-manager/src/server/server.ts    | 51 ++++++++++---------
 3 files changed, 64 insertions(+), 24 deletions(-)

diff --git a/packages/plugins/multi-app-manager/package.json b/packages/plugins/multi-app-manager/package.json
index ca85e2105..9836ac688 100644
--- a/packages/plugins/multi-app-manager/package.json
+++ b/packages/plugins/multi-app-manager/package.json
@@ -9,7 +9,8 @@
   "main": "./lib/index.js",
   "types": "./lib/index.d.ts",
   "dependencies": {
-    "@nocobase/server": "0.9.4-alpha.2"
+    "@nocobase/server": "0.9.4-alpha.2",
+    "async-mutex": "^0.3.2"
   },
   "gitHead": "ce588eefb0bfc50f7d5bbee575e0b5e843bf6644"
 }
diff --git a/packages/plugins/multi-app-manager/src/server/__tests__/multiple-apps.test.ts b/packages/plugins/multi-app-manager/src/server/__tests__/multiple-apps.test.ts
index 350dcdb1c..0668dd2ca 100644
--- a/packages/plugins/multi-app-manager/src/server/__tests__/multiple-apps.test.ts
+++ b/packages/plugins/multi-app-manager/src/server/__tests__/multiple-apps.test.ts
@@ -172,4 +172,38 @@ describe('multiple apps create', () => {
     await app.start();
     expect(app.appManager.applications.get(subAppName)).toBeDefined();
   });
+
+  it('should get same obj ref when asynchronously access with same sub app name', async () => {
+    const subAppName = `t_${uid()}`;
+
+    const subApp = await app.db.getRepository('applications').create({
+      values: {
+        name: subAppName,
+        options: {},
+      },
+    });
+
+    await app.appManager.removeApplication(subAppName);
+    expect(app.appManager.applications.get(subAppName)).toBeUndefined();
+
+    const instances = [];
+
+    app.on('afterSubAppAdded', (subApp) => {
+      instances.push(subApp);
+    });
+
+    const promises = [];
+    for (let i = 0; i < 3; i++) {
+      promises.push(
+        (async () => {
+          await app.appManager.getApplication(subAppName);
+        })(),
+      );
+    }
+    await Promise.all(promises);
+
+    expect(instances.length).toBe(1);
+    expect(instances[0]).toBeDefined();
+    expect(instances[0].name == subAppName).toBeTruthy();
+  });
 });
diff --git a/packages/plugins/multi-app-manager/src/server/server.ts b/packages/plugins/multi-app-manager/src/server/server.ts
index 75e4ee66d..35d3b62ce 100644
--- a/packages/plugins/multi-app-manager/src/server/server.ts
+++ b/packages/plugins/multi-app-manager/src/server/server.ts
@@ -4,6 +4,7 @@ import lodash from 'lodash';
 import * as path from 'path';
 import { resolve } from 'path';
 import { ApplicationModel } from './models/application';
+import { Mutex } from 'async-mutex';
 
 export type AppDbCreator = (app: Application, transaction?: Transactionable) => Promise<void>;
 export type AppOptionsFactory = (appName: string, mainApp: Application) => any;
@@ -69,6 +70,8 @@ export class PluginMultiAppManager extends Plugin {
   appDbCreator: AppDbCreator = defaultDbCreator;
   appOptionsFactory: AppOptionsFactory = defaultAppOptionsFactory;
 
+  private beforeGetApplicationMutex = new Mutex();
+
   setAppOptionsFactory(factory: AppOptionsFactory) {
     this.appOptionsFactory = factory;
   }
@@ -150,35 +153,37 @@ export class PluginMultiAppManager extends Plugin {
     this.app.on(
       'beforeGetApplication',
       async ({ appManager, name, options }: { appManager: AppManager; name: string; options: any }) => {
-        if (appManager.applications.has(name)) {
-          return;
-        }
+        await this.beforeGetApplicationMutex.runExclusive(async () => {
+          if (appManager.applications.has(name)) {
+            return;
+          }
 
-        const applicationRecord = (await this.app.db.getRepository('applications').findOne({
-          filter: {
-            name,
-          },
-        })) as ApplicationModel | null;
+          const applicationRecord = (await this.app.db.getRepository('applications').findOne({
+            filter: {
+              name,
+            },
+          })) as ApplicationModel | null;
 
-        const instanceOptions = applicationRecord.get('options');
+          const instanceOptions = applicationRecord.get('options');
 
-        // skip standalone deployment application
-        if (instanceOptions?.standaloneDeployment && appManager.runningMode !== 'single') {
-          return;
-        }
+          // skip standalone deployment application
+          if (instanceOptions?.standaloneDeployment && appManager.runningMode !== 'single') {
+            return;
+          }
 
-        if (!applicationRecord) {
-          return;
-        }
+          if (!applicationRecord) {
+            return;
+          }
 
-        const subApp = await applicationRecord.registerToMainApp(this.app, {
-          appOptionsFactory: this.appOptionsFactory,
+          const subApp = await applicationRecord.registerToMainApp(this.app, {
+            appOptionsFactory: this.appOptionsFactory,
+          });
+
+          // must skip load on upgrade
+          if (!options?.upgrading) {
+            await subApp.load();
+          }
         });
-
-        // must skip load on upgrade
-        if (!options?.upgrading) {
-          await subApp.load();
-        }
       },
     );