Merge pull request #19268 from marleypowell/marley/19264-circular-dependency-fix

fix: 🐛 implemented a circular dependency check in the ServiceRegistry

.changeset/tricky-melons-fold.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-app-api': patch
+---
+
+Backend startup will now fail if any circular service dependencies are detected.

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

@@ -27,7 +27,7 @@ describe('DependencyGraph', () => {
   });
 
   describe('detectCircularDependency', () => {
-    it('should return undefined with not deps', () => {
+    it('should return undefined with no deps', () => {
       expect(
         DependencyGraph.fromMap({
           1: {},
@@ -38,7 +38,7 @@ describe('DependencyGraph', () => {
       ).toBeUndefined();
     });
 
-    it('should return undefined with no circular deps', async () => {
+    it('should return undefined with no circular deps', () => {
       expect(
         DependencyGraph.fromMap({
           1: { provides: ['a'] },
@@ -49,7 +49,7 @@ describe('DependencyGraph', () => {
       ).toBeUndefined();
     });
 
-    it('should detect an immediate circular dep', async () => {
+    it('should detect an immediate circular dependency', () => {
       expect(
         DependencyGraph.fromMap({
           1: { provides: ['a'], consumes: ['a'] },
@@ -57,7 +57,7 @@ describe('DependencyGraph', () => {
       ).toEqual(['1', '1']);
     });
 
-    it('should detect a small circular dep', async () => {
+    it('should detect a small circular dependency', () => {
       expect(
         DependencyGraph.fromMap({
           1: { provides: ['a'], consumes: ['b'] },
@@ -66,7 +66,7 @@ describe('DependencyGraph', () => {
       ).toEqual(['1', '2', '1']);
     });
 
-    it('should detect a larger distant circular dep', async () => {
+    it('should detect a larger distance circular dependency', () => {
       expect(
         DependencyGraph.fromMap({
           1: { provides: ['a'] },
@@ -74,7 +74,280 @@ describe('DependencyGraph', () => {
           3: { provides: ['c'], consumes: ['b'] },
           4: { provides: ['d', 'e'], consumes: ['c', 'a'] },
         }).detectCircularDependency(),
-      ).toEqual(['2', '3', '4', '2']);
+      ).toEqual(['2', '4', '3', '2']);
+    });
+
+    it('should detect a circular dependency starting from the first node', () => {
+      expect(
+        DependencyGraph.fromMap({
+          1: { provides: ['a'], consumes: ['b'] },
+          2: { provides: ['b'], consumes: ['c'] },
+          3: { provides: ['c'], consumes: ['d'] },
+          4: { provides: ['d'], consumes: ['a'] },
+        }).detectCircularDependency(),
+      ).toEqual(['1', '2', '3', '4', '1']);
+    });
+
+    it('should detect all independent circular dependency cycles', () => {
+      expect(
+        Array.from(
+          DependencyGraph.fromMap({
+            // Cycle 1
+            1: { provides: ['a'], consumes: ['b'] },
+            2: { provides: ['b'], consumes: ['c'] },
+            3: { provides: ['c'], consumes: ['a'] },
+
+            // Cycle 2
+            4: { provides: ['d'], consumes: ['e'] },
+            5: { provides: ['e'], consumes: ['f'] },
+            6: { provides: ['f'], consumes: ['d'] },
+
+            // Cycle 3
+            7: { provides: ['g'], consumes: ['h'] },
+            8: { provides: ['h'], consumes: ['i'] },
+            9: { provides: ['i'], consumes: ['g'] },
+          }).detectCircularDependencies(),
+        ),
+      ).toEqual([
+        ['1', '2', '3', '1'],
+        ['4', '5', '6', '4'],
+        ['7', '8', '9', '7'],
+      ]);
+    });
+
+    it('should only detect unique circular dependency cycles', () => {
+      expect(
+        Array.from(
+          DependencyGraph.fromMap({
+            1: { provides: ['a'], consumes: ['b'] },
+            2: { provides: ['b'], consumes: ['c', 'a'] },
+            3: { provides: ['c'], consumes: ['d', 'a'] },
+            4: { provides: ['d'], consumes: ['e', 'a'] },
+            5: { provides: ['e'], consumes: ['f', 'a'] },
+            6: { provides: ['f'], consumes: ['h', 'a'] },
+            7: { provides: ['h'], consumes: ['a'] },
+          }).detectCircularDependencies(),
+        ),
+      ).toEqual([
+        ['1', '2', '1'],
+        ['1', '2', '3', '1'],
+        ['1', '2', '3', '4', '1'],
+        ['1', '2', '3', '4', '5', '1'],
+        ['1', '2', '3', '4', '5', '6', '1'],
+        ['1', '2', '3', '4', '5', '6', '7', '1'],
+      ]);
+    });
+
+    it('should detect circular dependency cycles in order when fromIterable', () => {
+      expect(
+        Array.from(
+          DependencyGraph.fromIterable([
+            { value: 'a', provides: ['a'], consumes: ['b'] },
+            { value: 'b', provides: ['b'], consumes: ['c', 'a'] },
+            { value: 'c', provides: ['c'], consumes: ['d', 'a'] },
+            { value: '4', provides: ['d'], consumes: ['e', 'a'] },
+            { value: '5', provides: ['e'], consumes: ['f', 'a'] },
+            { value: '6', provides: ['f'], consumes: ['h', 'a'] },
+          ]).detectCircularDependencies(),
+        ),
+      ).toEqual([
+        ['a', 'b', 'a'],
+        ['a', 'b', 'c', 'a'],
+        ['a', 'b', 'c', '4', 'a'],
+        ['a', 'b', 'c', '4', '5', 'a'],
+        ['a', 'b', 'c', '4', '5', '6', 'a'],
+      ]);
+    });
+
+    it('should detect circular dependency cycles in order by key when fromMap', () => {
+      expect(
+        Array.from(
+          DependencyGraph.fromMap({
+            1: { provides: ['a'], consumes: ['b'] },
+            2: { provides: ['b'], consumes: ['c', 'a'] },
+            3: { provides: ['c'], consumes: ['d', 'a'] },
+            4: { provides: ['d'], consumes: ['e', 'a'] },
+            5: { provides: ['e'], consumes: ['f', 'a'] },
+            6: { provides: ['f'], consumes: ['h', 'a'] },
+          }).detectCircularDependencies(),
+        ),
+      ).toEqual([
+        ['1', '2', '1'],
+        ['1', '2', '3', '1'],
+        ['1', '2', '3', '4', '1'],
+        ['1', '2', '3', '4', '5', '1'],
+        ['1', '2', '3', '4', '5', '6', '1'],
+      ]);
+    });
+
+    it('should detect circular dependency cycles in order by key when fromMap 2', () => {
+      expect(
+        Array.from(
+          DependencyGraph.fromMap({
+            a: { provides: ['a'], consumes: ['b'] },
+            b: { provides: ['b'], consumes: ['c', 'a'] },
+            c: { provides: ['c'], consumes: ['d', 'a'] },
+            4: { provides: ['d'], consumes: ['e', 'a'] },
+            5: { provides: ['e'], consumes: ['f', 'a'] },
+            6: { provides: ['f'], consumes: ['h', 'a'] },
+          }).detectCircularDependencies(),
+        ),
+      ).toEqual([
+        ['4', 'a', 'b', 'c', '4'],
+        ['5', 'a', 'b', 'c', '4', '5'],
+        ['6', 'a', 'b', 'c', '4', '5', '6'],
+        ['a', 'b', 'a'],
+        ['a', 'b', 'c', 'a'],
+      ]);
+    });
+
+    it('should detect circular dependency cycles in order by key when fromMap 3', () => {
+      expect(
+        Array.from(
+          DependencyGraph.fromMap({
+            a: { provides: ['a'], consumes: ['b'] },
+            b: { provides: ['b'], consumes: ['c', 'a'] },
+            c: { provides: ['c'], consumes: ['d', 'a'] },
+            d: { provides: ['d'], consumes: ['e', 'a'] },
+            e: { provides: ['e'], consumes: ['f', 'a'] },
+            f: { provides: ['f'], consumes: ['h', 'a'] },
+          }).detectCircularDependencies(),
+        ),
+      ).toEqual([
+        ['a', 'b', 'a'],
+        ['a', 'b', 'c', 'a'],
+        ['a', 'b', 'c', 'd', 'a'],
+        ['a', 'b', 'c', 'd', 'e', 'a'],
+        ['a', 'b', 'c', 'd', 'e', 'f', 'a'],
+      ]);
+    });
+
+    it('should detect circular dependency cycles with duplicate keys when fromIterable', () => {
+      expect(
+        Array.from(
+          DependencyGraph.fromIterable([
+            { value: 'a', provides: ['a'], consumes: ['b'] },
+            { value: 'a', provides: ['b'], consumes: ['c', 'a'] },
+            { value: 'a', provides: ['c'], consumes: ['d', 'a'] },
+            { value: 'a', provides: ['d'], consumes: ['e', 'a'] },
+            { value: 'a', provides: ['e'], consumes: ['f', 'a'] },
+            { value: 'a', provides: ['f'], consumes: ['h', 'a'] },
+          ]).detectCircularDependencies(),
+        ),
+      ).toEqual([
+        ['a', 'a', 'a'],
+        ['a', 'a', 'a', 'a'],
+        ['a', 'a', 'a', 'a', 'a'],
+        ['a', 'a', 'a', 'a', 'a', 'a'],
+        ['a', 'a', 'a', 'a', 'a', 'a', 'a'],
+      ]);
+    });
+
+    it('should detect circular dependency cycles with object values when fromIterable', () => {
+      expect(
+        Array.from(
+          DependencyGraph.fromIterable([
+            { value: { key: 1 }, provides: ['a'], consumes: ['b'] },
+            { value: { key: 2 }, provides: ['b'], consumes: ['c', 'a'] },
+            { value: { key: 3 }, provides: ['c'], consumes: ['d', 'a'] },
+            { value: { key: 4 }, provides: ['d'], consumes: ['e', 'a'] },
+            { value: { key: 5 }, provides: ['e'], consumes: ['f', 'a'] },
+            { value: { key: 6 }, provides: ['f'], consumes: ['h', 'a'] },
+          ]).detectCircularDependencies(),
+        ),
+      ).toEqual([
+        [{ key: 1 }, { key: 2 }, { key: 1 }],
+        [{ key: 1 }, { key: 2 }, { key: 3 }, { key: 1 }],
+        [{ key: 1 }, { key: 2 }, { key: 3 }, { key: 4 }, { key: 1 }],
+        [
+          { key: 1 },
+          { key: 2 },
+          { key: 3 },
+          { key: 4 },
+          { key: 5 },
+          { key: 1 },
+        ],
+        [
+          { key: 1 },
+          { key: 2 },
+          { key: 3 },
+          { key: 4 },
+          { key: 5 },
+          { key: 6 },
+          { key: 1 },
+        ],
+      ]);
+    });
+
+    it('should detect circular dependency cycles with array values when fromIterable', () => {
+      expect(
+        Array.from(
+          DependencyGraph.fromIterable([
+            { value: [1], provides: ['a'], consumes: ['b'] },
+            { value: [2], provides: ['b'], consumes: ['c', 'a'] },
+            { value: [3], provides: ['c'], consumes: ['d', 'a'] },
+            { value: [4], provides: ['d'], consumes: ['e', 'a'] },
+            { value: [5], provides: ['e'], consumes: ['f', 'a'] },
+            { value: [6], provides: ['f'], consumes: ['h', 'a'] },
+          ]).detectCircularDependencies(),
+        ),
+      ).toEqual([
+        [[1], [2], [1]],
+        [[1], [2], [3], [1]],
+        [[1], [2], [3], [4], [1]],
+        [[1], [2], [3], [4], [5], [1]],
+        [[1], [2], [3], [4], [5], [6], [1]],
+      ]);
+    });
+
+    it('should detect circular dependency cycles by reference with symbol values when fromIterable', () => {
+      const symbol1 = Symbol(1);
+      const symbol2 = Symbol(2);
+      const symbol3 = Symbol(3);
+      const symbol4 = Symbol(4);
+      const symbol5 = Symbol(5);
+      const symbol6 = Symbol(6);
+
+      expect(
+        Array.from(
+          DependencyGraph.fromIterable([
+            { value: symbol1, provides: ['a'], consumes: ['b'] },
+            { value: symbol2, provides: ['b'], consumes: ['c', 'a'] },
+            { value: symbol3, provides: ['c'], consumes: ['d', 'a'] },
+            { value: symbol4, provides: ['d'], consumes: ['e', 'a'] },
+            { value: symbol5, provides: ['e'], consumes: ['f', 'a'] },
+            { value: symbol6, provides: ['f'], consumes: ['h', 'a'] },
+          ]).detectCircularDependencies(),
+        ),
+      ).toEqual([
+        [symbol1, symbol2, symbol1],
+        [symbol1, symbol2, symbol3, symbol1],
+        [symbol1, symbol2, symbol3, symbol4, symbol1],
+        [symbol1, symbol2, symbol3, symbol4, symbol5, symbol1],
+        [symbol1, symbol2, symbol3, symbol4, symbol5, symbol6, symbol1],
+      ]);
+    });
+
+    it('should ignore circular dependency cycles by reference with symbol values when fromMap', () => {
+      const symbol1 = Symbol('1');
+      const symbol2 = Symbol('2');
+      const symbol3 = Symbol('3');
+      const symbol4 = Symbol('4');
+      const symbol5 = Symbol('5');
+      const symbol6 = Symbol('6');
+
+      expect(
+        Array.from(
+          DependencyGraph.fromMap({
+            [symbol1]: { provides: ['a'], consumes: ['b'] },
+            [symbol2]: { provides: ['b'], consumes: ['c', 'a'] },
+            [symbol3]: { provides: ['c'], consumes: ['d', 'a'] },
+            [symbol4]: { provides: ['d'], consumes: ['e', 'a'] },
+            [symbol5]: { provides: ['e'], consumes: ['f', 'a'] },
+            [symbol6]: { provides: ['f'], consumes: ['h', 'a'] },
+          }).detectCircularDependencies(),
+        ),
+      ).toEqual([]);
     });
   });
 

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

@@ -37,6 +37,37 @@ class Node<T> {
   ) {}
 }
 
+/** @internal */
+class CycleKeySet<T> {
+  static from<T>(nodes: Array<Node<T>>) {
+    return new CycleKeySet<T>(nodes);
+  }
+
+  #nodeIds: Map<T, number>;
+  #cycleKeys: Set<string>;
+
+  private constructor(nodes: Array<Node<T>>) {
+    this.#nodeIds = new Map(nodes.map((n, i) => [n.value, i]));
+    this.#cycleKeys = new Set<string>();
+  }
+
+  tryAdd(path: T[]): boolean {
+    const cycleKey = this.#getCycleKey(path);
+    if (this.#cycleKeys.has(cycleKey)) {
+      return false;
+    }
+    this.#cycleKeys.add(cycleKey);
+    return true;
+  }
+
+  #getCycleKey(path: T[]): string {
+    return path
+      .map(n => this.#nodeIds.get(n)!)
+      .sort()
+      .join(',');
+  }
+}
+
 /**
  * Internal helper to help validate and traverse a dependency graph.
  * @internal
@@ -78,7 +109,9 @@ export class DependencyGraph<T> {
     }
   }
 
-  // Find all nodes that consume dependencies that are not provided by any other node
+  /**
+   * Find all nodes that consume dependencies that are not provided by any other node.
+   */
   findUnsatisfiedDeps(): Array<{ value: T; unsatisfied: string[] }> {
     const unsatisfiedDependencies = [];
     for (const node of this.#nodes.values()) {
@@ -92,9 +125,21 @@ export class DependencyGraph<T> {
     return unsatisfiedDependencies;
   }
 
-  // Detect circular dependencies within the graph, returning the path of nodes that
-  // form a cycle, with the same node as the first and last element of the array.
+  /**
+   * Detect the first circular dependency within the graph, returning the path of nodes that
+   * form a cycle, with the same node as the first and last element of the array.
+   */
   detectCircularDependency(): T[] | undefined {
+    return this.detectCircularDependencies().next().value;
+  }
+
+  /**
+   * Detect circular dependencies within the graph, returning the path of nodes that
+   * form a cycle, with the same node as the first and last element of the array.
+   */
+  *detectCircularDependencies(): Generator<T[], undefined> {
+    const cycleKeys = CycleKeySet.from(this.#nodes);
+
     for (const startNode of this.#nodes) {
       const visited = new Set<Node<T>>();
       const stack = new Array<[node: Node<T>, path: T[]]>([
@@ -108,16 +153,20 @@ export class DependencyGraph<T> {
           continue;
         }
         visited.add(node);
-        for (const produced of node.provides) {
-          const consumerNodes = this.#nodes.filter(other =>
-            other.consumes.has(produced),
+        for (const consumed of node.consumes) {
+          const providerNodes = this.#nodes.filter(other =>
+            other.provides.has(consumed),
           );
-          for (const consumer of consumerNodes) {
-            if (consumer === startNode) {
-              return [...path, startNode.value];
+          for (const provider of providerNodes) {
+            if (provider === startNode) {
+              if (cycleKeys.tryAdd(path)) {
+                yield [...path, startNode.value];
+              }
+
+              break;
             }
-            if (!visited.has(consumer)) {
-              stack.push([consumer, [...path, consumer.value]]);
+            if (!visited.has(provider)) {
+              stack.push([provider, [...path, provider.value]]);
             }
           }
         }

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

@@ -164,7 +164,7 @@ export class BackendInitializer {
   }
 
   async #doStart(): Promise<void> {
-    this.#serviceHolder = new ServiceRegistry([
+    this.#serviceHolder = ServiceRegistry.create([
       ...this.#defaultApiFactories,
       ...this.#providedServiceFactories,
     ]);

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

@@ -90,12 +90,12 @@ const refDefault2b = createServiceRef<{ x: number }>({
 
 describe('ServiceRegistry', () => {
   it('should return undefined if there is no factory defined', async () => {
-    const registry = new ServiceRegistry([]);
+    const registry = ServiceRegistry.create([]);
     expect(registry.get(ref1, 'catalog')).toBe(undefined);
   });
 
   it('should return an implementation for a registered ref', async () => {
-    const registry = new ServiceRegistry([sf1()]);
+    const registry = ServiceRegistry.create([sf1()]);
     await expect(registry.get(ref1, 'catalog')).resolves.toEqual({ x: 1 });
     await expect(registry.get(ref1, 'scaffolder')).resolves.toEqual({ x: 1 });
     expect(await registry.get(ref1, 'catalog')).toBe(
@@ -110,7 +110,7 @@ describe('ServiceRegistry', () => {
   });
 
   it('should handle multiple factories with different serviceRefs', async () => {
-    const registry = new ServiceRegistry([sf1(), sf2()]);
+    const registry = ServiceRegistry.create([sf1(), sf2()]);
 
     await expect(registry.get(ref1, 'catalog')).resolves.toEqual({
       x: 1,
@@ -131,7 +131,7 @@ describe('ServiceRegistry', () => {
         return { x: 2 };
       },
     });
-    const registry = new ServiceRegistry([factory(), sf1()]);
+    const registry = ServiceRegistry.create([factory(), sf1()]);
     await expect(registry.get(ref2, 'catalog')).rejects.toThrow(
       "Failed to instantiate 'root' scoped service '2' because it depends on 'plugin' scoped service '1'.",
     );
@@ -145,7 +145,7 @@ describe('ServiceRegistry', () => {
         return { x: rootDep.x };
       },
     });
-    const registry = new ServiceRegistry([factory(), sf2()]);
+    const registry = ServiceRegistry.create([factory(), sf2()]);
     await expect(registry.get(ref1, 'catalog')).resolves.toEqual({
       x: 2,
     });
@@ -160,7 +160,7 @@ describe('ServiceRegistry', () => {
         return { x: rootDep.x };
       },
     });
-    const registry = new ServiceRegistry([factory(), sf2()]);
+    const registry = ServiceRegistry.create([factory(), sf2()]);
     await expect(registry.get(ref, 'catalog')).resolves.toEqual({
       x: 2,
     });
@@ -175,35 +175,35 @@ describe('ServiceRegistry', () => {
         return { pluginId: meta.getId() };
       },
     });
-    const registry = new ServiceRegistry([factory()]);
+    const registry = ServiceRegistry.create([factory()]);
     await expect(registry.get(ref, 'catalog')).resolves.toEqual({
       pluginId: 'catalog',
     });
   });
 
   it('should use the last factory for each ref', async () => {
-    const registry = new ServiceRegistry([sf2(), sf2b()]);
+    const registry = ServiceRegistry.create([sf2(), sf2b()]);
     await expect(registry.get(ref2, 'catalog')).resolves.toEqual({
       x: 22,
     });
   });
 
   it('should use the defaultFactory from the ref if not provided to the registry', async () => {
-    const registry = new ServiceRegistry([]);
+    const registry = ServiceRegistry.create([]);
     await expect(registry.get(refDefault1, 'catalog')).resolves.toEqual({
       x: 10,
     });
   });
 
   it('should not use the defaultFactory from the ref if provided to the registry', async () => {
-    const registry = new ServiceRegistry([sf1()]);
+    const registry = ServiceRegistry.create([sf1()]);
     await expect(registry.get(refDefault1, 'catalog')).resolves.toEqual({
       x: 1,
     });
   });
 
   it('should handle duplicate defaultFactories by duplicating the implementations', async () => {
-    const registry = new ServiceRegistry([]);
+    const registry = ServiceRegistry.create([]);
     await expect(registry.get(refDefault2a, 'catalog')).resolves.toEqual({
       x: 20,
     });
@@ -234,7 +234,7 @@ describe('ServiceRegistry', () => {
       defaultFactory: factoryLoader,
     });
 
-    const registry = new ServiceRegistry([]);
+    const registry = ServiceRegistry.create([]);
     await Promise.all([
       expect(registry.get(ref, 'catalog')).resolves.toBeUndefined(),
       expect(registry.get(ref, 'catalog')).resolves.toBeUndefined(),
@@ -252,7 +252,7 @@ describe('ServiceRegistry', () => {
       factory,
     });
 
-    const registry = new ServiceRegistry([myFactory()]);
+    const registry = ServiceRegistry.create([myFactory()]);
 
     await Promise.all([
       registry.get(ref1, 'catalog')!,
@@ -274,7 +274,7 @@ describe('ServiceRegistry', () => {
       factory,
     });
 
-    const registry = new ServiceRegistry([myFactory()]);
+    const registry = ServiceRegistry.create([myFactory()]);
 
     await Promise.all([
       registry.get(ref1, 'catalog')!,
@@ -296,7 +296,7 @@ describe('ServiceRegistry', () => {
       },
     });
 
-    const registry = new ServiceRegistry([myFactory()]);
+    const registry = ServiceRegistry.create([myFactory()]);
 
     await expect(registry.get(ref1, 'catalog')).rejects.toThrow(
       "Failed to instantiate service '1' for 'catalog' because the following dependent services are missing: '2'",
@@ -323,13 +323,217 @@ describe('ServiceRegistry', () => {
       },
     });
 
-    const registry = new ServiceRegistry([factoryA(), factoryB()]);
+    const registry = ServiceRegistry.create([factoryA(), factoryB()]);
 
     await expect(registry.get(refA, 'catalog')).rejects.toThrow(
       "Failed to instantiate service 'a' for 'catalog' because the factory function threw an error, Error: Failed to instantiate service 'b' for 'catalog' because the following dependent services are missing: 'c', 'd'",
     );
   });
 
+  describe('checkForCircularDeps', () => {
+    it('should throw if there are shallow circular dependencies', async () => {
+      const refA = createServiceRef<string>({ id: 'a' });
+      const refB = createServiceRef<string>({ id: 'b' });
+
+      const factoryA = createServiceFactory({
+        service: refA,
+        deps: { b: refB },
+        factory: async ({ b }) => b,
+      });
+
+      const factoryB = createServiceFactory({
+        service: refB,
+        deps: { a: refA },
+        factory: async ({ a }) => a,
+      });
+
+      expect(() => ServiceRegistry.create([factoryA(), factoryB()])).toThrow(
+        `Circular dependencies detected:
+  'a' -> 'b' -> 'a'`,
+      );
+    });
+
+    it('should throw if there are multiple circular dependency cycles', async () => {
+      const refA = createServiceRef<string>({ id: 'a' });
+      const refB = createServiceRef<string>({ id: 'b' });
+      const refC = createServiceRef<string>({ id: 'c' });
+      const refD = createServiceRef<string>({ id: 'd' });
+
+      const factoryA = createServiceFactory({
+        service: refA,
+        deps: { b: refB },
+        factory: async ({ b }) => b,
+      });
+
+      const factoryB = createServiceFactory({
+        service: refB,
+        deps: { a: refA },
+        factory: async ({ a }) => a,
+      });
+
+      const factoryC = createServiceFactory({
+        service: refC,
+        deps: { d: refD },
+        factory: async ({ d }) => d,
+      });
+
+      const factoryD = createServiceFactory({
+        service: refD,
+        deps: { c: refC },
+        factory: async ({ c }) => c,
+      });
+
+      expect(() =>
+        ServiceRegistry.create([
+          factoryA(),
+          factoryB(),
+          factoryC(),
+          factoryD(),
+        ]),
+      ).toThrow(
+        `Circular dependencies detected:
+  'a' -> 'b' -> 'a'
+  'c' -> 'd' -> 'c'`,
+      );
+    });
+
+    it('should throw if there are deep circular dependencies', async () => {
+      const refA = createServiceRef<string>({ id: 'a' });
+      const refB = createServiceRef<string>({ id: 'b' });
+      const refC = createServiceRef<string>({ id: 'c' });
+
+      const factoryA = createServiceFactory({
+        service: refA,
+        deps: { b: refB },
+        factory: async ({ b }) => b,
+      });
+
+      const factoryB = createServiceFactory({
+        service: refB,
+        deps: { c: refC },
+        factory: async ({ c }) => c,
+      });
+
+      const factoryC = createServiceFactory({
+        service: refC,
+        deps: { a: refA },
+        factory: async ({ a }) => a,
+      });
+
+      expect(() =>
+        ServiceRegistry.create([factoryA(), factoryB(), factoryC()]),
+      ).toThrow(
+        `Circular dependencies detected:
+  'a' -> 'b' -> 'c' -> 'a'`,
+      );
+    });
+
+    it('should throw if there are deep circular dependencies 2', async () => {
+      const refA = createServiceRef<string>({ id: 'a' });
+      const refB = createServiceRef<string>({ id: 'b' });
+      const refC = createServiceRef<string>({ id: 'c' });
+      const refD = createServiceRef<string>({ id: 'd' });
+
+      const factoryA = createServiceFactory({
+        service: refA,
+        deps: { b: refB },
+        factory: async ({ b }) => b,
+      });
+
+      const factoryB = createServiceFactory({
+        service: refB,
+        deps: { c: refC, d: refD },
+        factory: async ({ c, d }) => c + d,
+      });
+
+      const factoryC = createServiceFactory({
+        service: refC,
+        deps: { a: refA },
+        factory: async ({ a }) => a,
+      });
+
+      const factoryD = createServiceFactory({
+        service: refD,
+        deps: {},
+        factory: async () => 'd',
+      });
+
+      expect(() =>
+        ServiceRegistry.create([
+          factoryA(),
+          factoryB(),
+          factoryC(),
+          factoryD(),
+        ]),
+      ).toThrow(
+        `Circular dependencies detected:
+  'a' -> 'b' -> 'c' -> 'a'`,
+      );
+    });
+
+    it('should throw if there are circular dependencies', async () => {
+      const refA = createServiceRef<string>({ id: 'a' });
+      const refB = createServiceRef<string>({ id: 'b' });
+      const refC = createServiceRef<string>({ id: 'c' });
+
+      const factoryA = createServiceFactory({
+        service: refA,
+        deps: { b: refB, c: refC },
+        factory: async ({ b, c }) => b + c,
+      });
+
+      const factoryB = createServiceFactory({
+        service: refB,
+        deps: {},
+        factory: async () => 'b',
+      });
+
+      const factoryC = createServiceFactory({
+        service: refC,
+        deps: { a: refA },
+        factory: async ({ a }) => a,
+      });
+
+      expect(() =>
+        ServiceRegistry.create([factoryA(), factoryB(), factoryC()]),
+      ).toThrow(
+        `Circular dependencies detected:
+  'a' -> 'c' -> 'a'`,
+      );
+    });
+
+    it('should not infinitely loop if there are circular dependencies where not all nodes are in the cycle', async () => {
+      const refA = createServiceRef<string>({ id: 'a' });
+      const refB = createServiceRef<string>({ id: 'b' });
+      const refC = createServiceRef<string>({ id: 'c' });
+
+      const factoryA = createServiceFactory({
+        service: refA,
+        deps: { b: refB },
+        factory: async ({ b }) => b,
+      });
+
+      const factoryB = createServiceFactory({
+        service: refB,
+        deps: { c: refC },
+        factory: async ({ c }) => c,
+      });
+
+      const factoryC = createServiceFactory({
+        service: refC,
+        deps: { b: refB },
+        factory: async ({ b }) => b,
+      });
+
+      expect(() =>
+        ServiceRegistry.create([factoryA(), factoryB(), factoryC()]),
+      ).toThrow(
+        `Circular dependencies detected:
+  'b' -> 'c' -> 'b'`,
+      );
+    });
+  });
+
   it('should decorate error messages thrown by the top-level factory function', async () => {
     const myFactory = createServiceFactory({
       service: ref1,
@@ -342,7 +546,7 @@ describe('ServiceRegistry', () => {
       },
     });
 
-    const registry = new ServiceRegistry([myFactory()]);
+    const registry = ServiceRegistry.create([myFactory()]);
 
     await expect(registry.get(ref1, 'catalog')).rejects.toThrow(
       "Failed to instantiate service '1' because createRootContext threw an error, Error: top-level error",
@@ -358,7 +562,7 @@ describe('ServiceRegistry', () => {
       },
     });
 
-    const registry = new ServiceRegistry([myFactory()]);
+    const registry = ServiceRegistry.create([myFactory()]);
 
     await expect(registry.get(ref1, 'catalog')).rejects.toThrow(
       "Failed to instantiate service '1' for 'catalog' because the factory function threw an error, Error: error in plugin",
@@ -373,7 +577,7 @@ describe('ServiceRegistry', () => {
       },
     });
 
-    const registry = new ServiceRegistry([]);
+    const registry = ServiceRegistry.create([]);
 
     await expect(registry.get(ref, 'catalog')).rejects.toThrow(
       "Failed to instantiate service '1' because the default factory loader threw an error, Error: default factory error",

packages/backend-app-api/src/wiring/ServiceRegistry.ts

@@ -20,11 +20,12 @@ import {
   coreServices,
   createServiceFactory,
 } from '@backstage/backend-plugin-api';
-import { stringifyError } from '@backstage/errors';
+import { ConflictError, stringifyError } from '@backstage/errors';
 import { EnumerableServiceHolder } from './types';
 // Direct internal import to avoid duplication
 // eslint-disable-next-line @backstage/no-forbidden-package-imports
 import { InternalServiceFactory } from '@backstage/backend-plugin-api/src/services/system/types';
+import { DependencyGraph } from '../lib/DependencyGraph';
 /**
  * Keep in sync with `@backstage/backend-plugin-api/src/services/system/types.ts`
  * @internal
@@ -57,6 +58,12 @@ const pluginMetadataServiceFactory = createServiceFactory(
 );
 
 export class ServiceRegistry implements EnumerableServiceHolder {
+  static create(factories: Array<ServiceFactory>): EnumerableServiceHolder {
+    const registry = new ServiceRegistry(factories);
+    registry.checkForCircularDeps();
+    return registry;
+  }
+
   readonly #providedFactories: Map<string, InternalServiceFactory>;
   readonly #loadedDefaultFactories: Map<
     Function,
@@ -73,13 +80,23 @@ export class ServiceRegistry implements EnumerableServiceHolder {
     InternalServiceFactory,
     Promise<unknown>
   >();
+  readonly #dependencyGraph: DependencyGraph<string>;
 
-  constructor(factories: Array<ServiceFactory>) {
+  private constructor(factories: Array<ServiceFactory>) {
     this.#providedFactories = new Map(
       factories.map(sf => [sf.service.id, toInternalServiceFactory(sf)]),
     );
     this.#loadedDefaultFactories = new Map();
     this.#implementations = new Map();
+    this.#dependencyGraph = DependencyGraph.fromIterable(
+      Array.from(this.#providedFactories).map(
+        ([serviceId, serviceFactory]) => ({
+          value: serviceId,
+          provides: [serviceId],
+          consumes: Object.values(serviceFactory.deps).map(d => d.id),
+        }),
+      ),
+    );
   }
 
   #resolveFactory(
@@ -146,6 +163,20 @@ export class ServiceRegistry implements EnumerableServiceHolder {
     }
   }
 
+  checkForCircularDeps(): void {
+    const circularDependencies = Array.from(
+      this.#dependencyGraph.detectCircularDependencies(),
+    );
+
+    if (circularDependencies.length) {
+      const cycles = circularDependencies
+        .map(c => c.map(id => `'${id}'`).join(' -> '))
+        .join('\n  ');
+
+      throw new ConflictError(`Circular dependencies detected:\n  ${cycles}`);
+    }
+  }
+
   getServiceRefs(): ServiceRef<unknown>[] {
     return Array.from(this.#providedFactories.values()).map(f => f.service);
   }