symfony · fabpot · Nov 5, 2017 · Nov 4, 2017
diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php
@@ -122,6 +122,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
     private $autoconfiguredInstanceof = array();
 
     private $removedIds = array();
+    private $alreadyLoading = array();
 
     public function __construct(ParameterBagInterface $parameterBag = null)
     {
@@ -594,12 +595,13 @@ private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_
             throw $e;
         }
 
-        $this->loading[$id] = true;
+        $loading = isset($this->alreadyLoading[$id]) ? 'loading' : 'alreadyLoading';
+        $this->{$loading}[$id] = true;
 
         try {
             $service = $this->createService($definition, $id);
         } finally {
-            unset($this->loading[$id]);
+            unset($this->{$loading}[$id]);
         }
 
         return $service;
@@ -1089,6 +1091,10 @@ private function createService(Definition $definition, $id, $tryProxy = true)
 
         $arguments = $this->resolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments())));
 
+        if (null !== $id && $definition->isShared() && isset($this->services[$id]) && ($tryProxy || !$definition->isLazy())) {
+            return $this->services[$id];
+        }
+
         if (null !== $factory = $definition->getFactory()) {
             if (is_array($factory)) {
                 $factory = array($this->resolveServices($parameterBag->resolveValue($factory[0])), $factory[1]);
@@ -1551,7 +1557,8 @@ private function callMethod($service, $call)
     private function shareService(Definition $definition, $service, $id)
     {
         if (null !== $id && $definition->isShared()) {
-            $this->services[$this->normalizeId($id)] = $service;
+            $this->services[$id] = $service;
+            unset($this->loading[$id], $this->alreadyLoading[$id]);
         }
     }
 

diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@@ -261,13 +261,15 @@ private function addServiceLocalTempVariables($cId, Definition $definition, arra
 
         array_unshift($inlinedDefinitions, $definition);
 
+        $isNonLazyShared = !$this->getProxyDumper()->isProxyCandidate($definition) && $definition->isShared();
         $calls = $behavior = array();
         foreach ($inlinedDefinitions as $iDefinition) {
-            $this->getServiceCallsFromArguments($iDefinition->getArguments(), $calls, $behavior);
-            $this->getServiceCallsFromArguments($iDefinition->getMethodCalls(), $calls, $behavior);
-            $this->getServiceCallsFromArguments($iDefinition->getProperties(), $calls, $behavior);
-            $this->getServiceCallsFromArguments(array($iDefinition->getConfigurator()), $calls, $behavior);
-            $this->getServiceCallsFromArguments(array($iDefinition->getFactory()), $calls, $behavior);
+            $this->getServiceCallsFromArguments($iDefinition->getArguments(), $calls, $behavior, $isNonLazyShared);
+            $isPreInstantiation = $isNonLazyShared && $iDefinition !== $definition && !$this->hasReference($cId, $iDefinition->getMethodCalls(), true) && !$this->hasReference($cId, $iDefinition->getProperties(), true);
+            $this->getServiceCallsFromArguments($iDefinition->getMethodCalls(), $calls, $behavior, $isPreInstantiation);
+            $this->getServiceCallsFromArguments($iDefinition->getProperties(), $calls, $behavior, $isPreInstantiation);
+            $this->getServiceCallsFromArguments(array($iDefinition->getConfigurator()), $calls, $behavior, $isPreInstantiation);
+            $this->getServiceCallsFromArguments(array($iDefinition->getFactory()), $calls, $behavior, $isNonLazyShared);
         }
 
         $code = '';
@@ -289,6 +291,16 @@ private function addServiceLocalTempVariables($cId, Definition $definition, arra
         }
 
         if ('' !== $code) {
+            if ($isNonLazyShared) {
+                $code .= <<<EOTXT
+
+        if (isset(\$this->services['$cId'])) {
+            return \$this->services['$cId'];
+        }
+
+EOTXT;
+            }
+
             $code .= "\n";
         }
 
@@ -495,15 +507,15 @@ private function isTrivialInstance(Definition $definition)
         }
 
         foreach ($definition->getArguments() as $arg) {
-            if (!$arg || ($arg instanceof Reference && 'service_container' !== (string) $arg)) {
+            if (!$arg || ($arg instanceof Reference && 'service_container' === (string) $arg)) {
                 continue;
             }
             if (is_array($arg) && 3 >= count($arg)) {
                 foreach ($arg as $k => $v) {
                     if ($this->dumpValue($k) !== $this->dumpValue($k, false)) {
                         return false;
                     }
-                    if (!$v || ($v instanceof Reference && 'service_container' !== (string) $v)) {
+                    if (!$v || ($v instanceof Reference && 'service_container' === (string) $v)) {
                         continue;
                     }
                     if (!is_scalar($v) || $this->dumpValue($v) !== $this->dumpValue($v, false)) {
@@ -1396,16 +1408,16 @@ private function getServiceConditionals($value)
     /**
      * Builds service calls from arguments.
      */
-    private function getServiceCallsFromArguments(array $arguments, array &$calls, array &$behavior)
+    private function getServiceCallsFromArguments(array $arguments, array &$calls, array &$behavior, $isPreInstantiation)
     {
         foreach ($arguments as $argument) {
             if (is_array($argument)) {
-                $this->getServiceCallsFromArguments($argument, $calls, $behavior);
+                $this->getServiceCallsFromArguments($argument, $calls, $behavior, $isPreInstantiation);
             } elseif ($argument instanceof Reference) {
                 $id = (string) $argument;
 
                 if (!isset($calls[$id])) {
-                    $calls[$id] = 0;
+                    $calls[$id] = (int) $isPreInstantiation;
                 }
                 if (!isset($behavior[$id])) {
                     $behavior[$id] = $argument->getInvalidBehavior();

diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
@@ -1203,6 +1203,15 @@ public function testUninitializedReference()
         $this->assertEquals(array('foo1' => new \stdClass(), 'foo3' => new \stdClass()), iterator_to_array($bar->iter));
     }
 
+    public function testAlmostCircular()
+    {
+        $container = include __DIR__.'/Fixtures/containers/container_almost_circular.php';
+
+        $foo = $container->get('foo');
+
+        $this->assertSame($foo, $foo->bar->foobar->foo);
+    }
+
     public function testRegisterForAutoconfiguration()
     {
         $container = new ContainerBuilder();

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
@@ -764,6 +764,22 @@ public function testUninitializedReference()
         $this->assertEquals(array('foo1' => new \stdClass(), 'foo3' => new \stdClass()), iterator_to_array($bar->iter));
     }
 
+    public function testAlmostCircular()
+    {
+        $container = include self::$fixturesPath.'/containers/container_almost_circular.php';
+        $container->compile();
+        $dumper = new PhpDumper($container);
+
+        $this->assertStringEqualsFile(self::$fixturesPath.'/php/container_almost_circular.php', $dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Almost_Circular')));
+
+        require self::$fixturesPath.'/php/container_almost_circular.php';
+
+        $container = new \Symfony_DI_PhpDumper_Test_Almost_Circular();
+        $foo = $container->get('foo');
+
+        $this->assertSame($foo, $foo->bar->foobar->foo);
+    }
+
     public function testDumpHandlesLiteralClassWithRootNamespace()
     {
         $container = new ContainerBuilder();

diff --git a/...ony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php b/...ony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php
@@ -0,0 +1,19 @@
+<?php
+
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
+use Symfony\Component\DependencyInjection\Reference;
+
+$container = new ContainerBuilder();
+
+$container->register('foo', FooCircular::class)->setPublic(true)
+   ->addArgument(new Reference('bar'));
+
+$container->register('bar', BarCircular::class)
+    ->addMethodCall('addFoobar', array(new Reference('foobar')));
+
+$container->register('foobar', FoobarCircular::class)
+    ->addArgument(new Reference('foo'));
+
+return $container;
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php
@@ -110,3 +110,27 @@ public function __construct($lazyValues, $lazyEmptyValues)
         $this->lazyEmptyValues = $lazyEmptyValues;
     }
 }
+
+class FoobarCircular
+{
+    public function __construct(FooCircular $foo)
+    {
+        $this->foo = $foo;
+    }
+}
+
+class FooCircular
+{
+    public function __construct(BarCircular $bar)
+    {
+        $this->bar = $bar;
+    }
+}
+
+class BarCircular
+{
+    public function addFoobar(FoobarCircular $foobar)
+    {
+        $this->foobar = $foobar;
+    }
+}
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/container_almost_circular.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/container_almost_circular.php
@@ -0,0 +1,108 @@
+<?php
+
+use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\DependencyInjection\Container;
+use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
+use Symfony\Component\DependencyInjection\Exception\LogicException;
+use Symfony\Component\DependencyInjection\Exception\RuntimeException;
+use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;
+
+/**
+ * This class has been auto-generated
+ * by the Symfony Dependency Injection Component.
+ *
+ * @final since Symfony 3.3
+ */
+class Symfony_DI_PhpDumper_Test_Almost_Circular extends Container
+{
+    private $parameters;
+    private $targetDirs = array();
+
+    public function __construct()
+    {
+        $this->services = array();
+        $this->methodMap = array(
+            'bar' => 'getBarService',
+            'foo' => 'getFooService',
+            'foobar' => 'getFoobarService',
+        );
+        $this->privates = array(
+            'bar' => true,
+            'foobar' => true,
+        );
+
+        $this->aliases = array();
+    }
+
+    public function getRemovedIds()
+    {
+        return array(
+            'Psr\\Container\\ContainerInterface' => true,
+            'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
+        );
+    }
+
+    public function compile()
+    {
+        throw new LogicException('You cannot compile a dumped container that was already compiled.');
+    }
+
+    public function isCompiled()
+    {
+        return true;
+    }
+
+    public function isFrozen()
+    {
+        @trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Use the isCompiled() method instead.', __METHOD__), E_USER_DEPRECATED);
+
+        return true;
+    }
+
+    /**
+     * Gets the public 'foo' shared service.
+     *
+     * @return \FooCircular
+     */
+    protected function getFooService()
+    {
+        $a = ${($_ = isset($this->services['bar']) ? $this->services['bar'] : $this->getBarService()) && false ?: '_'};
+
+        if (isset($this->services['foo'])) {
+            return $this->services['foo'];
+        }
+
+        return $this->services['foo'] = new \FooCircular($a);
+    }
+
+    /**
+     * Gets the private 'bar' shared service.
+     *
+     * @return \BarCircular
+     */
+    protected function getBarService()
+    {
+        $this->services['bar'] = $instance = new \BarCircular();
+
+        $instance->addFoobar(${($_ = isset($this->services['foobar']) ? $this->services['foobar'] : $this->getFoobarService()) && false ?: '_'});
+
+        return $instance;
+    }
+
+    /**
+     * Gets the private 'foobar' shared service.
+     *
+     * @return \FoobarCircular
+     */
+    protected function getFoobarService()
+    {
+        $a = ${($_ = isset($this->services['foo']) ? $this->services['foo'] : $this->getFooService()) && false ?: '_'};
+
+        if (isset($this->services['foobar'])) {
+            return $this->services['foobar'];
+        }
+
+        return $this->services['foobar'] = new \FoobarCircular($a);
+    }
+}
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php
@@ -82,6 +82,10 @@ protected function getBarService()
     {
         $a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'};
 
+        if (isset($this->services['bar'])) {
+            return $this->services['bar'];
+        }
+
         $this->services['bar'] = $instance = new \Bar\FooClass('foo', $a, $this->getParameter('foo_bar'));
 
         $a->configure($instance);
@@ -182,7 +186,13 @@ protected function getDeprecatedServiceService()
      */
     protected function getFactoryServiceService()
     {
-        return $this->services['factory_service'] = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'}->getInstance();
+        $a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'};
+
+        if (isset($this->services['factory_service'])) {
+            return $this->services['factory_service'];
+        }
+
+        return $this->services['factory_service'] = $a->getInstance();
     }
 
     /**
@@ -192,7 +202,13 @@ protected function getFactoryServiceService()
      */
     protected function getFactoryServiceSimpleService()
     {
-        return $this->services['factory_service_simple'] = ${($_ = isset($this->services['factory_simple']) ? $this->services['factory_simple'] : $this->getFactorySimpleService()) && false ?: '_'}->getInstance();
+        $a = ${($_ = isset($this->services['factory_simple']) ? $this->services['factory_simple'] : $this->getFactorySimpleService()) && false ?: '_'};
+
+        if (isset($this->services['factory_service_simple'])) {
+            return $this->services['factory_service_simple'];
+        }
+
+        return $this->services['factory_service_simple'] = $a->getInstance();
     }
 
     /**
@@ -204,6 +220,10 @@ protected function getFooService()
     {
         $a = ${($_ = isset($this->services['foo.baz']) ? $this->services['foo.baz'] : $this->getFoo_BazService()) && false ?: '_'};
 
+        if (isset($this->services['foo'])) {
+            return $this->services['foo'];
+        }
+
         $this->services['foo'] = $instance = \Bar\FooClass::getInstance('foo', $a, array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo').'', 'foobar' => $this->getParameter('foo')), true, $this);
 
         $instance->foo = 'bar';
@@ -321,7 +341,13 @@ protected function getMethodCall1Service()
      */
     protected function getNewFactoryServiceService()
     {
-        $this->services['new_factory_service'] = $instance = ${($_ = isset($this->services['new_factory']) ? $this->services['new_factory'] : $this->getNewFactoryService()) && false ?: '_'}->getInstance();
+        $a = ${($_ = isset($this->services['new_factory']) ? $this->services['new_factory'] : $this->getNewFactoryService()) && false ?: '_'};
+
+        if (isset($this->services['new_factory_service'])) {
+            return $this->services['new_factory_service'];
+        }
+
+        $this->services['new_factory_service'] = $instance = $a->getInstance();
 
         $instance->foo = 'bar';