symfony · derrabus · Nov 12, 2020 · Nov 11, 2020
diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@@ -413,32 +413,66 @@ private function analyzeReferences()
         $this->singleUsePrivateIds = array_diff_key($this->singleUsePrivateIds, $this->circularReferences);
     }

-    private function collectCircularReferences(string $sourceId, array $edges, array &$checkedNodes, array $path = [], bool $byConstructor = true): void
+    private function collectCircularReferences(string $sourceId, array $edges, array &$checkedNodes, array &$loops = [], array $path = [], bool $byConstructor = true): void
     {
         $path[$sourceId] = $byConstructor;
         $checkedNodes[$sourceId] = true;
         foreach ($edges as $edge) {
             $node = $edge->getDestNode();
             $id = $node->getId();
-
             if (!($definition = $node->getValue()) instanceof Definition || $sourceId === $id || ($edge->isLazy() && ($this->proxyDumper ?? $this->getProxyDumper())->isProxyCandidate($definition)) || $edge->isWeak()) {
                 continue;
             }
 
             if (isset($path[$id])) {
                 $loop = null;
                 $loopByConstructor = $edge->isReferencedByConstructor();
+                $pathInLoop = [$id, []];
                 foreach ($path as $k => $pathByConstructor) {
                     if (null !== $loop) {
                         $loop[] = $k;
+                        $pathInLoop[1][$k] = $pathByConstructor;
+                        $loops[$k][] = &$pathInLoop;
                         $loopByConstructor = $loopByConstructor && $pathByConstructor;
                     } elseif ($k === $id) {
                         $loop = [];
                     }
                 }
                 $this->addCircularReferences($id, $loop, $loopByConstructor);
             } elseif (!isset($checkedNodes[$id])) {
-                $this->collectCircularReferences($id, $node->getOutEdges(), $checkedNodes, $path, $edge->isReferencedByConstructor());
+                $this->collectCircularReferences($id, $node->getOutEdges(), $checkedNodes, $loops, $path, $edge->isReferencedByConstructor());
+            } elseif (isset($loops[$id])) {
+                // we already had detected loops for this edge
+                // let's check if we have a common ancestor in one of the detected loops
+                foreach ($loops[$id] as [$first, $loopPath]) {
+                    if (!isset($path[$first])) {
+                        continue;
+                    }
+                    // We have a common ancestor, let's fill the current path
+                    $fillPath = null;
+                    foreach ($loopPath as $k => $pathByConstructor) {
+                        if (null !== $fillPath) {
+                            $fillPath[$k] = $pathByConstructor;
+                        } elseif ($k === $id) {
+                            $fillPath = $path;
+                            $fillPath[$k] = $pathByConstructor;
+                        }
+                    }
+
+                    // we can now build the loop
+                    $loop = null;
+                    $loopByConstructor = $edge->isReferencedByConstructor();
+                    foreach ($fillPath as $k => $pathByConstructor) {
+                        if (null !== $loop) {
+                            $loop[] = $k;
+                            $loopByConstructor = $loopByConstructor && $pathByConstructor;
+                        } elseif ($k === $first) {
+                            $loop = [];
+                        }
+                    }
+                    $this->addCircularReferences($first, $loop, true);
+                    break;
+                }
             }
         }
         unset($path[$sourceId]);

diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
@@ -1374,6 +1374,9 @@ public function testAlmostCircular($visibility)
         $container = include __DIR__.'/Fixtures/containers/container_almost_circular.php';
         $container->compile();
 
+        $pA = $container->get('pA');
+        $this->assertEquals(new \stdClass(), $pA);
+
         $logger = $container->get('monolog.logger');
         $this->assertEquals(new \stdClass(), $logger->handler);
 

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
@@ -1054,6 +1054,9 @@ public function testAlmostCircular($visibility)
 
         $container = new $container();
 
+        $pA = $container->get('pA');
+        $this->assertEquals(new \stdClass(), $pA);
+
         $logger = $container->get('monolog.logger');
         $this->assertEquals(new \stdClass(), $logger->handler);
 

diff --git a/...ony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php b/...ony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php
@@ -9,6 +9,21 @@
 $public = 'public' === $visibility;
 $container = new ContainerBuilder();
 
+// multiple path detection
+
+$container->register('pA', 'stdClass')->setPublic(true)
+    ->addArgument(new Reference('pB'))
+    ->addArgument(new Reference('pC'));
+
+$container->register('pB', 'stdClass')->setPublic($public)
+    ->setProperty('d', new Reference('pD'));
+$container->register('pC', 'stdClass')->setPublic($public)
+    ->setLazy(true)
+    ->setProperty('d', new Reference('pD'));
+
+$container->register('pD', 'stdClass')->setPublic($public)
+    ->addArgument(new Reference('pA'));
+
 // monolog-like + handler that require monolog
 
 $container->register('monolog.logger', 'stdClass')->setPublic(true)

diff --git a/...ony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_private.php b/...ony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_private.php
@@ -40,6 +40,7 @@ public function __construct()
             'manager2' => 'getManager2Service',
             'manager3' => 'getManager3Service',
             'monolog.logger' => 'getMonolog_LoggerService',
+            'pA' => 'getPAService',
             'root' => 'getRootService',
             'subscriber' => 'getSubscriberService',
         ];
@@ -87,6 +88,9 @@ public function getRemovedIds(): array
             'manager4' => true,
             'monolog.logger_2' => true,
             'multiuse1' => true,
+            'pB' => true,
+            'pC' => true,
+            'pD' => true,
             'subscriber2' => true,
         ];
     }
@@ -374,6 +378,28 @@ protected function getMonolog_LoggerService()
         return $instance;
     }
 
+    /**
+     * Gets the public 'pA' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getPAService()
+    {
+        $a = new \stdClass();
+
+        $b = ($this->privates['pC'] ?? $this->getPCService());
+
+        if (isset($this->services['pA'])) {
+            return $this->services['pA'];
+        }
+
+        $this->services['pA'] = $instance = new \stdClass($a, $b);
+
+        $a->d = ($this->privates['pD'] ?? $this->getPDService());
+
+        return $instance;
+    }
+
     /**
      * Gets the public 'root' shared service.
      *
@@ -481,4 +507,34 @@ protected function getManager4Service($lazyLoad = true)
 
         return $instance;
     }
+
+    /**
+     * Gets the private 'pC' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getPCService($lazyLoad = true)
+    {
+        $this->privates['pC'] = $instance = new \stdClass();
+
+        $instance->d = ($this->privates['pD'] ?? $this->getPDService());
+
+        return $instance;
+    }
+
+    /**
+     * Gets the private 'pD' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getPDService()
+    {
+        $a = ($this->services['pA'] ?? $this->getPAService());
+
+        if (isset($this->privates['pD'])) {
+            return $this->privates['pD'];
+        }
+
+        return $this->privates['pD'] = new \stdClass($a);
+    }
 }
diff --git a/...fony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php b/...fony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php
@@ -53,6 +53,10 @@ public function __construct()
             'manager3' => 'getManager3Service',
             'monolog.logger' => 'getMonolog_LoggerService',
             'monolog.logger_2' => 'getMonolog_Logger2Service',
+            'pA' => 'getPAService',
+            'pB' => 'getPBService',
+            'pC' => 'getPCService',
+            'pD' => 'getPDService',
             'root' => 'getRootService',
             'subscriber' => 'getSubscriberService',
         ];
@@ -558,6 +562,71 @@ protected function getMonolog_Logger2Service()
         return $instance;
     }
 
+    /**
+     * Gets the public 'pA' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getPAService()
+    {
+        $a = ($this->services['pB'] ?? $this->getPBService());
+
+        if (isset($this->services['pA'])) {
+            return $this->services['pA'];
+        }
+        $b = ($this->services['pC'] ?? $this->getPCService());
+
+        if (isset($this->services['pA'])) {
+            return $this->services['pA'];
+        }
+
+        return $this->services['pA'] = new \stdClass($a, $b);
+    }
+
+    /**
+     * Gets the public 'pB' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getPBService()
+    {
+        $this->services['pB'] = $instance = new \stdClass();
+
+        $instance->d = ($this->services['pD'] ?? $this->getPDService());
+
+        return $instance;
+    }
+
+    /**
+     * Gets the public 'pC' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getPCService($lazyLoad = true)
+    {
+        $this->services['pC'] = $instance = new \stdClass();
+
+        $instance->d = ($this->services['pD'] ?? $this->getPDService());
+
+        return $instance;
+    }
+
+    /**
+     * Gets the public 'pD' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getPDService()
+    {
+        $a = ($this->services['pA'] ?? $this->getPAService());
+
+        if (isset($this->services['pD'])) {
+            return $this->services['pD'];
+        }
+
+        return $this->services['pD'] = new \stdClass($a);
+    }
+
     /**
      * Gets the public 'root' shared service.
      *