Merge pull request #30159 from backstage/freben/module-init-order

fix module initialization order bug

.changeset/rotten-dragons-guess.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-app-api': patch
+---
+
+Fixed a bug where occasionally the initialization order of multiple modules consuming a single extension point could happen in the wrong order.

packages/backend-app-api/src/lib/DependencyGraph.test.ts

@@ -514,5 +514,21 @@ describe('DependencyGraph', () => {
         }).parallelTopologicalTraversal(async id => id),
       ).rejects.toThrow('Circular dependency detected');
     });
+
+    it('awaits all producers', async () => {
+      await expect(
+        DependencyGraph.fromMap({
+          1: { provides: ['a'] },
+          2: { provides: ['a'] },
+          3: { consumes: ['a'] },
+        }).parallelTopologicalTraversal(async id => {
+          // Delaying 2 should not make 3 run, wait for 1 too first
+          if (id === '2') {
+            await new Promise(resolve => setTimeout(resolve, 100));
+          }
+          return id;
+        }),
+      ).resolves.toEqual(['1', '2', '3']);
+    });
   });
 });

packages/backend-app-api/src/lib/DependencyGraph.ts

@@ -188,12 +188,25 @@ export class DependencyGraph<T> {
     fn: (value: T) => Promise<TResult>,
   ): Promise<TResult[]> {
     const allProvided = this.#allProvided;
-    const producedSoFar = new Set<string>();
     const waiting = new Set(this.#nodes.values());
     const visited = new Set<Node<T>>();
     const results = new Array<TResult>();
     let inFlight = 0; // Keep track of how many callbacks are in flight, so that we know if we got stuck
 
+    // This keeps track of a counter of how many providers there are still left
+    // to be visited for each dependency. This needs to be a counter instead of
+    // a flag for the special case where there are several providers of a given
+    // value, even though there may be only one consumer of it.
+    const producedRemaining = new Map<string, number>();
+    for (const node of this.#nodes) {
+      for (const provided of node.provides) {
+        producedRemaining.set(
+          provided,
+          (producedRemaining.get(provided) ?? 0) + 1,
+        );
+      }
+    }
+
     // Find all nodes that have no dependencies that have not already been produced by visited nodes
     async function processMoreNodes() {
       if (waiting.size === 0) {
@@ -203,7 +216,10 @@ export class DependencyGraph<T> {
       for (const node of waiting) {
         let ready = true;
         for (const consumed of node.consumes) {
-          if (allProvided.has(consumed) && !producedSoFar.has(consumed)) {
+          if (
+            allProvided.has(consumed) &&
+            producedRemaining.get(consumed) !== 0
+          ) {
             ready = false;
             continue;
           }
@@ -234,7 +250,17 @@ export class DependencyGraph<T> {
       const result = await fn(node.value);
       results.push(result);
 
-      node.provides.forEach(produced => producedSoFar.add(produced));
+      node.provides.forEach(produced => {
+        const remaining = producedRemaining.get(produced);
+        if (!remaining) {
+          // This should be impossible, if the code that generates the map is correct
+          throw new Error(
+            `Internal error: Node provided superfluous dependency '${produced}'`,
+          );
+        }
+        producedRemaining.set(produced, remaining - 1);
+      });
+
       inFlight -= 1;
       await processMoreNodes();
     }

packages/backend-app-api/src/wiring/BackendInitializer.test.ts

@@ -1134,4 +1134,68 @@ describe('BackendInitializer', () => {
     backend.add(instanceMetadataPlugin);
     await backend.start();
   });
+
+  it('should properly wait for all modules that consume an extension point to really finish, before starting the module that provides that extension point', async () => {
+    expect.assertions(3);
+    const backend = new BackendInitializer(baseFactories);
+    const ext = createExtensionPoint<{ hello: (message: string) => void }>({
+      id: 'a',
+    });
+    const plugin = createBackendPlugin({
+      pluginId: 'test',
+      register(reg) {
+        reg.registerInit({
+          deps: {},
+          async init() {},
+        });
+      },
+    });
+    const producerModule = createBackendModule({
+      pluginId: 'test',
+      moduleId: 'producer',
+      register(reg) {
+        const hello = jest.fn();
+        reg.registerExtensionPoint(ext, { hello });
+        reg.registerInit({
+          deps: {},
+          async init() {
+            // we must not have been initialized before both of the consuming modules have been initialized
+            expect(hello).toHaveBeenCalledTimes(2);
+            expect(hello).toHaveBeenNthCalledWith(1, 'fast');
+            expect(hello).toHaveBeenNthCalledWith(2, 'slow');
+          },
+        });
+      },
+    });
+    const fastConsumerModule = createBackendModule({
+      pluginId: 'test',
+      moduleId: 'fast-consumer',
+      register(reg) {
+        reg.registerInit({
+          deps: { x: ext },
+          async init({ x }) {
+            x.hello('fast');
+          },
+        });
+      },
+    });
+    const slowConsumerModule = createBackendModule({
+      pluginId: 'test',
+      moduleId: 'slow-consumer',
+      register(reg) {
+        reg.registerInit({
+          deps: { x: ext },
+          async init({ x }) {
+            await new Promise(resolve => setTimeout(resolve, 100));
+            x.hello('slow');
+          },
+        });
+      },
+    });
+    await backend.add(plugin);
+    await backend.add(producerModule);
+    await backend.add(fastConsumerModule);
+    await backend.add(slowConsumerModule);
+    await backend.start();
+  });
 });