bug #28060 [DI] Fix false-positive circular ref leading to wrong exceptions or infinite loops at runtime (nicolas-grekas)

This PR was merged into the 3.4 branch. Discussion ---------- [DI] Fix false-positive circular ref leading to wrong exceptions or infinite loops at runtime | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28010, #27865 | License | MIT | Doc PR | - When circular loops involve references in properties, method calls or configurators, it is possible to properly instantiate the related services. The current logic is broken: `ContainerBuilder` considers some of these loops as self-referencing circular references, leading to a runtime exception, and in similar situations, `PhpDumper` generates code that turns to infinite loops at runtime 💥. These badly handled situations happen with inlined definitions. This PR fixes both classes by making them track which references are really part of the constructors' chain, including inline definitions. It also fixes dumping infinite loops when dumping circular loops involving lazy services while proxy-manager-bridge is not installed. Commits ------- e843bb86c8 [DI] Fix false-positive circular ref leading to wrong exceptions or infinite loops at runtime
2018-08-08 10:12:29 +02:00 · 2018-08-08 10:12:29 +02:00 · ba31bab47a
parent 2bae1832c7 e843bb86c8
commit ba31bab47a
9 changed files with 481 additions and 153 deletions
--- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveReferencesToAliasesPass.php
+++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveReferencesToAliasesPass.php
@ -66,7 +66,7 @@ class ResolveReferencesToAliasesPass extends AbstractRecursivePass
        $seen = array();
        while ($container->hasAlias($id)) {
            if (isset($seen[$id])) {
-                throw new ServiceCircularReferenceException($id, array_keys($seen));
+                throw new ServiceCircularReferenceException($id, array_merge(array_keys($seen), array($id)));
            }
            $seen[$id] = true;
            $id = $container->normalizeId($container->getAlias($id));
--- a/src/Symfony/Component/DependencyInjection/Container.php
+++ b/src/Symfony/Component/DependencyInjection/Container.php
@ -294,7 +294,7 @@ class Container implements ResettableContainerInterface
            }

            if (isset($this->loading[$id])) {
-                throw new ServiceCircularReferenceException($id, array_keys($this->loading));
+                throw new ServiceCircularReferenceException($id, array_merge(array_keys($this->loading), array($id)));
            }

            $this->loading[$id] = true;
--- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php
+++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php
@ -122,7 +122,6 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
    private $autoconfiguredInstanceof = array();

    private $removedIds = array();
-    private $alreadyLoading = array();

    private static $internalTypes = array(
        'int' => true,
@ -588,22 +587,32 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
        return $this->doGet($id, $invalidBehavior);
    }

-    private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, array &$inlineServices = array())
+    private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, array &$inlineServices = null, $isConstructorArgument = false)
    {
        $id = $this->normalizeId($id);

        if (isset($inlineServices[$id])) {
            return $inlineServices[$id];
        }
-        if (ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior) {
-            return parent::get($id, $invalidBehavior);
+        if (null === $inlineServices) {
+            $isConstructorArgument = true;
+            $inlineServices = array();
        }
-        if ($service = parent::get($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)) {
-            return $service;
+        try {
+            if (ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior) {
+                return parent::get($id, $invalidBehavior);
+            }
+            if ($service = parent::get($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)) {
+                return $service;
+            }
+        } catch (ServiceCircularReferenceException $e) {
+            if ($isConstructorArgument) {
+                throw $e;
+            }
        }

        if (!isset($this->definitions[$id]) && isset($this->aliasDefinitions[$id])) {
-            return $this->doGet((string) $this->aliasDefinitions[$id], $invalidBehavior, $inlineServices);
+            return $this->doGet((string) $this->aliasDefinitions[$id], $invalidBehavior, $inlineServices, $isConstructorArgument);
        }

        try {
@ -616,16 +625,17 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
            throw $e;
        }

-        $loading = isset($this->alreadyLoading[$id]) ? 'loading' : 'alreadyLoading';
-        $this->{$loading}[$id] = true;
-
-        try {
-            $service = $this->createService($definition, $inlineServices, $id);
-        } finally {
-            unset($this->{$loading}[$id]);
+        if ($isConstructorArgument) {
+            $this->loading[$id] = true;
        }

-        return $service;
+        try {
+            return $this->createService($definition, $inlineServices, $isConstructorArgument, $id);
+        } finally {
+            if ($isConstructorArgument) {
+                unset($this->loading[$id]);
+            }
+        }
    }

    /**
@ -1092,7 +1102,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
     * @throws RuntimeException         When the service is a synthetic service
     * @throws InvalidArgumentException When configure callable is not callable
     */
-    private function createService(Definition $definition, array &$inlineServices, $id = null, $tryProxy = true)
+    private function createService(Definition $definition, array &$inlineServices, $isConstructorArgument = false, $id = null, $tryProxy = true)
    {
        if (null === $id && isset($inlineServices[$h = spl_object_hash($definition)])) {
            return $inlineServices[$h];
@ -1110,16 +1120,14 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
            @trigger_error($definition->getDeprecationMessage($id), E_USER_DEPRECATED);
        }

-        if ($tryProxy && $definition->isLazy()) {
-            $proxy = $this
-                ->getProxyInstantiator()
-                ->instantiateProxy(
-                    $this,
-                    $definition,
-                    $id, function () use ($definition, &$inlineServices, $id) {
-                        return $this->createService($definition, $inlineServices, $id, false);
-                    }
-                );
+        if ($tryProxy && $definition->isLazy() && !$tryProxy = !($proxy = $this->proxyInstantiator) || $proxy instanceof RealServiceInstantiator) {
+            $proxy = $proxy->instantiateProxy(
+                $this,
+                $definition,
+                $id, function () use ($definition, &$inlineServices, $id) {
+                    return $this->createService($definition, $inlineServices, true, $id, false);
+                }
+            );
            $this->shareService($definition, $proxy, $id, $inlineServices);

            return $proxy;
@ -1131,19 +1139,21 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
            require_once $parameterBag->resolveValue($definition->getFile());
        }

-        $arguments = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments())), $inlineServices);
+        $arguments = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments())), $inlineServices, $isConstructorArgument);
+
+        if (null !== $factory = $definition->getFactory()) {
+            if (\is_array($factory)) {
+                $factory = array($this->doResolveServices($parameterBag->resolveValue($factory[0]), $inlineServices, $isConstructorArgument), $factory[1]);
+            } elseif (!\is_string($factory)) {
+                throw new RuntimeException(sprintf('Cannot create service "%s" because of invalid factory', $id));
+            }
+        }

        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->doResolveServices($parameterBag->resolveValue($factory[0]), $inlineServices), $factory[1]);
-            } elseif (!\is_string($factory)) {
-                throw new RuntimeException(sprintf('Cannot create service "%s" because of invalid factory', $id));
-            }
-
+        if (null !== $factory) {
            $service = \call_user_func_array($factory, $arguments);

            if (!$definition->isDeprecated() && \is_array($factory) && \is_string($factory[0])) {
@ -1214,11 +1224,11 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
        return $this->doResolveServices($value);
    }

-    private function doResolveServices($value, array &$inlineServices = array())
+    private function doResolveServices($value, array &$inlineServices = array(), $isConstructorArgument = false)
    {
        if (\is_array($value)) {
            foreach ($value as $k => $v) {
-                $value[$k] = $this->doResolveServices($v, $inlineServices);
+                $value[$k] = $this->doResolveServices($v, $inlineServices, $isConstructorArgument);
            }
        } elseif ($value instanceof ServiceClosureArgument) {
            $reference = $value->getValues()[0];
@ -1261,9 +1271,9 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
                return $count;
            });
        } elseif ($value instanceof Reference) {
-            $value = $this->doGet((string) $value, $value->getInvalidBehavior(), $inlineServices);
+            $value = $this->doGet((string) $value, $value->getInvalidBehavior(), $inlineServices, $isConstructorArgument);
        } elseif ($value instanceof Definition) {
-            $value = $this->createService($value, $inlineServices);
+            $value = $this->createService($value, $inlineServices, $isConstructorArgument);
        } elseif ($value instanceof Parameter) {
            $value = $this->getParameter((string) $value);
        } elseif ($value instanceof Expression) {
@ -1584,20 +1594,6 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
        }
    }

-    /**
-     * Retrieves the currently set proxy instantiator or instantiates one.
-     *
-     * @return InstantiatorInterface
-     */
-    private function getProxyInstantiator()
-    {
-        if (!$this->proxyInstantiator) {
-            $this->proxyInstantiator = new RealServiceInstantiator();
-        }
-
-        return $this->proxyInstantiator;
-    }
-
    private function callMethod($service, $call, array &$inlineServices)
    {
        foreach (self::getServiceConditionals($call[1]) as $s) {
@ -1627,7 +1623,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface

        if (null !== $id && $definition->isShared()) {
            $this->services[$id] = $service;
-            unset($this->loading[$id], $this->alreadyLoading[$id]);
+            unset($this->loading[$id]);
        }
    }

--- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
+++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@ -138,6 +138,29 @@ class PhpDumper extends Dumper

        $this->initializeMethodNamesMap('Container' === $baseClass ? Container::class : $baseClass);

+        if ($this->getProxyDumper() instanceof NullDumper) {
+            (new AnalyzeServiceReferencesPass(true))->process($this->container);
+            $this->circularReferences = array();
+            $checkedNodes = array();
+            foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) {
+                $currentPath = array($id => $id);
+                $this->analyzeCircularReferences($node->getOutEdges(), $checkedNodes, $currentPath);
+            }
+            foreach ($this->circularReferences as $parent => $ids) {
+                $path = array($parent);
+                while (!isset($ids[$parent])) {
+                    foreach ($ids as $id) {
+                        $path[] = $id;
+                        $ids = $this->circularReferences[$id];
+                        break;
+                    }
+                }
+                $path[] = $parent.'". Try running "composer require symfony/proxy-manager-bridge';
+
+                throw new ServiceCircularReferenceException($parent, $path);
+            }
+        }
+
        (new AnalyzeServiceReferencesPass())->process($this->container);
        $this->circularReferences = array();
        $checkedNodes = array();
@ -286,21 +309,18 @@ EOF;
     *
     * @return string
     */
-    private function addServiceLocalTempVariables($cId, Definition $definition, \SplObjectStorage $inlinedDefinitions, \SplObjectStorage $allInlinedDefinitions)
+    private function addServiceLocalTempVariables($cId, Definition $definition, \SplObjectStorage $inlinedDefinitions, array $serviceCalls, $preInstance = false)
    {
-        $allCalls = $calls = $behavior = array();
+        $calls = array();

-        foreach ($allInlinedDefinitions as $def) {
-            $arguments = array($def->getArguments(), $def->getFactory(), $def->getProperties(), $def->getMethodCalls(), $def->getConfigurator());
-            $this->getServiceCallsFromArguments($arguments, $allCalls, false, $cId, $behavior, $allInlinedDefinitions[$def]);
-        }
-
-        $isPreInstance = isset($inlinedDefinitions[$definition]) && isset($this->circularReferences[$cId]) && !$this->getProxyDumper()->isProxyCandidate($definition) && $definition->isShared();
        foreach ($inlinedDefinitions as $def) {
-            $this->getServiceCallsFromArguments(array($def->getArguments(), $def->getFactory()), $calls, $isPreInstance, $cId);
+            if ($preInstance && !$inlinedDefinitions[$def][1]) {
+                continue;
+            }
+            $this->getServiceCallsFromArguments(array($def->getArguments(), $def->getFactory()), $calls, $preInstance, $cId);
            if ($def !== $definition) {
                $arguments = array($def->getProperties(), $def->getMethodCalls(), $def->getConfigurator());
-                $this->getServiceCallsFromArguments($arguments, $calls, $isPreInstance && !$this->hasReference($cId, $arguments, true), $cId);
+                $this->getServiceCallsFromArguments($arguments, $calls, $preInstance && !$this->hasReference($cId, $arguments, true), $cId);
            }
        }
        if (!isset($inlinedDefinitions[$definition])) {
@ -309,23 +329,23 @@ EOF;
        }

        $code = '';
-        foreach ($calls as $id => $callCount) {
+        foreach ($calls as $id => list($callCount)) {
            if ('service_container' === $id || $id === $cId || isset($this->referenceVariables[$id])) {
                continue;
            }
-            if ($callCount <= 1 && $allCalls[$id] <= 1) {
+            if ($callCount <= 1 && $serviceCalls[$id][0] <= 1) {
                continue;
            }

            $name = $this->getNextVariableName();
            $this->referenceVariables[$id] = new Variable($name);

-            $reference = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE === $behavior[$id] ? new Reference($id, $behavior[$id]) : null;
+            $reference = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE === $serviceCalls[$id][1] ? new Reference($id, $serviceCalls[$id][1]) : null;
            $code .= sprintf("        \$%s = %s;\n", $name, $this->getServiceCall($id, $reference));
        }

        if ('' !== $code) {
-            if ($isPreInstance) {
+            if ($preInstance) {
                $code .= <<<EOTXT

        if (isset(\$this->services['$cId'])) {
@ -347,7 +367,7 @@ EOTXT;
            $node = $edge->getDestNode();
            $id = $node->getId();

-            if ($node->getValue() && ($edge->isLazy() || $edge->isWeak())) {
+            if ($node->getValue() && (($edge->isLazy() && !$this->getProxyDumper() instanceof NullDumper) || $edge->isWeak())) {
                // no-op
            } elseif (isset($currentPath[$id])) {
                foreach (array_reverse($currentPath) as $parentId) {
@ -425,23 +445,21 @@ EOTXT;
     *
     * @return string
     */
-    private function addServiceInclude($cId, Definition $definition, \SplObjectStorage $inlinedDefinitions)
+    private function addServiceInclude($cId, Definition $definition, \SplObjectStorage $inlinedDefinitions, array $serviceCalls)
    {
        $code = '';

        if ($this->inlineRequires && !$this->isHotPath($definition)) {
-            $lineage = $calls = $behavior = array();
+            $lineage = array();
            foreach ($inlinedDefinitions as $def) {
                if (!$def->isDeprecated() && \is_string($class = \is_array($factory = $def->getFactory()) && \is_string($factory[0]) ? $factory[0] : $def->getClass())) {
                    $this->collectLineage($class, $lineage);
                }
-                $arguments = array($def->getArguments(), $def->getFactory(), $def->getProperties(), $def->getMethodCalls(), $def->getConfigurator());
-                $this->getServiceCallsFromArguments($arguments, $calls, false, $cId, $behavior, $inlinedDefinitions[$def]);
            }

-            foreach ($calls as $id => $callCount) {
+            foreach ($serviceCalls as $id => list($callCount, $behavior)) {
                if ('service_container' !== $id && $id !== $cId
-                    && ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE !== $behavior[$id]
+                    && ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE !== $behavior
                    && $this->container->has($id)
                    && $this->isTrivialInstance($def = $this->container->findDefinition($id))
                    && \is_string($class = \is_array($factory = $def->getFactory()) && \is_string($factory[0]) ? $factory[0] : $def->getClass())
@ -476,24 +494,24 @@ EOTXT;
     * @throws RuntimeException                  When the factory definition is incomplete
     * @throws ServiceCircularReferenceException When a circular reference is detected
     */
-    private function addServiceInlinedDefinitions($id, Definition $definition, \SplObjectStorage $inlinedDefinitions, &$isSimpleInstance)
+    private function addServiceInlinedDefinitions($id, Definition $definition, \SplObjectStorage $inlinedDefinitions, &$isSimpleInstance, $preInstance = false)
    {
        $code = '';

        foreach ($inlinedDefinitions as $def) {
-            if ($definition === $def) {
+            if ($definition === $def || isset($this->definitionVariables[$def])) {
                continue;
            }
-            if ($inlinedDefinitions[$def] <= 1 && !$def->getMethodCalls() && !$def->getProperties() && !$def->getConfigurator() && false === strpos($this->dumpValue($def->getClass()), '$')) {
+            if ($inlinedDefinitions[$def][0] <= 1 && !$def->getMethodCalls() && !$def->getProperties() && !$def->getConfigurator() && false === strpos($this->dumpValue($def->getClass()), '$')) {
                continue;
            }
-            if (isset($this->definitionVariables[$def])) {
-                $name = $this->definitionVariables[$def];
-            } else {
-                $name = $this->getNextVariableName();
-                $this->definitionVariables[$def] = new Variable($name);
+            if ($preInstance && !$inlinedDefinitions[$def][1]) {
+                continue;
            }

+            $name = $this->getNextVariableName();
+            $this->definitionVariables[$def] = new Variable($name);
+
            // a construct like:
            // $a = new ServiceA(ServiceB $b); $b = new ServiceB(ServiceA $a);
            // this is an indication for a wrong implementation, you can circumvent this problem
@ -502,7 +520,7 @@ EOTXT;
            // $a = new ServiceA(ServiceB $b);
            // $b->setServiceA(ServiceA $a);
            if (isset($inlinedDefinitions[$definition]) && $this->hasReference($id, array($def->getArguments(), $def->getFactory()))) {
-                throw new ServiceCircularReferenceException($id, array($id));
+                throw new ServiceCircularReferenceException($id, array($id, '...', $id));
            }

            $code .= $this->addNewInstance($def, '$'.$name, ' = ', $id);
@ -558,6 +576,7 @@ EOTXT;
        }

        $code = $this->addNewInstance($definition, $return, $instantiation, $id);
+        $this->referenceVariables[$id] = new Variable('instance');

        if (!$isSimpleInstance) {
            $code .= "\n";
@ -657,8 +676,6 @@ EOTXT;
     */
    private function addServiceInlinedDefinitionsSetup($id, Definition $definition, \SplObjectStorage $inlinedDefinitions, $isSimpleInstance)
    {
-        $this->referenceVariables[$id] = new Variable('instance');
-
        $code = '';
        foreach ($inlinedDefinitions as $def) {
            if ($definition === $def || !$this->hasReference($id, array($def->getProperties(), $def->getMethodCalls(), $def->getConfigurator()), true)) {
@ -668,7 +685,7 @@ EOTXT;
            // if the instance is simple, the return statement has already been generated
            // so, the only possible way to get there is because of a circular reference
            if ($isSimpleInstance) {
-                throw new ServiceCircularReferenceException($id, array($id));
+                throw new ServiceCircularReferenceException($id, array($id, '...', $id));
            }

            $name = (string) $this->definitionVariables[$def];
@ -805,6 +822,7 @@ EOF;
        $inlinedDefinitions = $this->getDefinitionsFromArguments(array($definition));
        $constructorDefinitions = $this->getDefinitionsFromArguments(array($definition->getArguments(), $definition->getFactory()));
        $otherDefinitions = new \SplObjectStorage();
+        $serviceCalls = array();

        foreach ($inlinedDefinitions as $def) {
            if ($def === $definition || isset($constructorDefinitions[$def])) {
@ -812,16 +830,21 @@ EOF;
            } else {
                $otherDefinitions[$def] = $inlinedDefinitions[$def];
            }
+            $arguments = array($def->getArguments(), $def->getFactory(), $def->getProperties(), $def->getMethodCalls(), $def->getConfigurator());
+            $this->getServiceCallsFromArguments($arguments, $serviceCalls, false, $id);
        }

        $isSimpleInstance = !$definition->getProperties() && !$definition->getMethodCalls() && !$definition->getConfigurator();
+        $preInstance = isset($this->circularReferences[$id]) && !$this->getProxyDumper()->isProxyCandidate($definition) && $definition->isShared();

        $code .=
-            $this->addServiceInclude($id, $definition, $inlinedDefinitions).
-            $this->addServiceLocalTempVariables($id, $definition, $constructorDefinitions, $inlinedDefinitions).
-            $this->addServiceInlinedDefinitions($id, $definition, $constructorDefinitions, $isSimpleInstance).
+            $this->addServiceInclude($id, $definition, $inlinedDefinitions, $serviceCalls).
+            $this->addServiceLocalTempVariables($id, $definition, $constructorDefinitions, $serviceCalls, $preInstance).
+            $this->addServiceInlinedDefinitions($id, $definition, $constructorDefinitions, $isSimpleInstance, $preInstance).
            $this->addServiceInstance($id, $definition, $isSimpleInstance).
-            $this->addServiceLocalTempVariables($id, $definition, $otherDefinitions, $inlinedDefinitions).
+            $this->addServiceLocalTempVariables($id, $definition, $constructorDefinitions->offsetUnset($definition) ?: $constructorDefinitions, $serviceCalls).
+            $this->addServiceLocalTempVariables($id, $definition, $otherDefinitions, $serviceCalls).
+            $this->addServiceInlinedDefinitions($id, $definition, $constructorDefinitions, $isSimpleInstance).
            $this->addServiceInlinedDefinitions($id, $definition, $otherDefinitions, $isSimpleInstance).
            $this->addServiceInlinedDefinitionsSetup($id, $definition, $inlinedDefinitions, $isSimpleInstance).
            $this->addServiceProperties($definition).
@ -1558,30 +1581,29 @@ EOF;

    /**
     * Builds service calls from arguments.
+     *
+     * Populates $calls with "referenced id" => ["reference count", "invalid behavior"] pairs.
     */
-    private function getServiceCallsFromArguments(array $arguments, array &$calls, $isPreInstance, $callerId, array &$behavior = array(), $step = 1)
+    private function getServiceCallsFromArguments(array $arguments, array &$calls, $preInstance, $callerId)
    {
        foreach ($arguments as $argument) {
            if (\is_array($argument)) {
-                $this->getServiceCallsFromArguments($argument, $calls, $isPreInstance, $callerId, $behavior, $step);
+                $this->getServiceCallsFromArguments($argument, $calls, $preInstance, $callerId);
            } elseif ($argument instanceof Reference) {
                $id = $this->container->normalizeId($argument);

                if (!isset($calls[$id])) {
-                    $calls[$id] = (int) ($isPreInstance && isset($this->circularReferences[$callerId][$id]));
-                }
-                if (!isset($behavior[$id])) {
-                    $behavior[$id] = $argument->getInvalidBehavior();
+                    $calls[$id] = array((int) ($preInstance && isset($this->circularReferences[$callerId][$id])), $argument->getInvalidBehavior());
                } else {
-                    $behavior[$id] = min($behavior[$id], $argument->getInvalidBehavior());
+                    $calls[$id][1] = min($calls[$id][1], $argument->getInvalidBehavior());
                }

-                $calls[$id] += $step;
+                ++$calls[$id][0];
            }
        }
    }

-    private function getDefinitionsFromArguments(array $arguments, \SplObjectStorage $definitions = null)
+    private function getDefinitionsFromArguments(array $arguments, $isConstructorArgument = true, \SplObjectStorage $definitions = null)
    {
        if (null === $definitions) {
            $definitions = new \SplObjectStorage();
@ -1589,22 +1611,23 @@ EOF;

        foreach ($arguments as $argument) {
            if (\is_array($argument)) {
-                $this->getDefinitionsFromArguments($argument, $definitions);
+                $this->getDefinitionsFromArguments($argument, $isConstructorArgument, $definitions);
            } elseif (!$argument instanceof Definition) {
                // no-op
            } elseif (isset($definitions[$argument])) {
-                $definitions[$argument] = 1 + $definitions[$argument];
+                $def = $definitions[$argument];
+                $definitions[$argument] = array(1 + $def[0], $isConstructorArgument || $def[1]);
            } else {
-                $definitions[$argument] = 1;
-                $this->getDefinitionsFromArguments($argument->getArguments(), $definitions);
-                $this->getDefinitionsFromArguments(array($argument->getFactory()), $definitions);
-                $this->getDefinitionsFromArguments($argument->getProperties(), $definitions);
-                $this->getDefinitionsFromArguments($argument->getMethodCalls(), $definitions);
-                $this->getDefinitionsFromArguments(array($argument->getConfigurator()), $definitions);
+                $definitions[$argument] = array(1, $isConstructorArgument);
+                $this->getDefinitionsFromArguments($argument->getArguments(), $isConstructorArgument, $definitions);
+                $this->getDefinitionsFromArguments(array($argument->getFactory()), $isConstructorArgument, $definitions);
+                $this->getDefinitionsFromArguments($argument->getProperties(), false, $definitions);
+                $this->getDefinitionsFromArguments($argument->getMethodCalls(), false, $definitions);
+                $this->getDefinitionsFromArguments(array($argument->getConfigurator()), false, $definitions);
                // move current definition last in the list
-                $nbOccurences = $definitions[$argument];
+                $def = $definitions[$argument];
                unset($definitions[$argument]);
-                $definitions[$argument] = $nbOccurences;
+                $definitions[$argument] = $def;
            }
        }

@ -1640,7 +1663,7 @@ EOF;
                    return true;
                }

-                if (!$deep || isset($visited[$argumentId]) || !isset($this->circularReferences[$id][$argumentId])) {
+                if (!$deep || isset($visited[$argumentId]) || !isset($this->circularReferences[$argumentId])) {
                    continue;
                }

--- a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php
@ -1379,6 +1379,12 @@ class ContainerBuilderTest extends TestCase

        $foo5 = $container->get('foo5');
        $this->assertSame($foo5, $foo5->bar->foo);
+
+        $manager = $container->get('manager');
+        $this->assertEquals(new \stdClass(), $manager);
+
+        $manager = $container->get('manager2');
+        $this->assertEquals(new \stdClass(), $manager);
    }

    public function provideAlmostCircular()
--- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
@ -23,6 +23,7 @@ use Symfony\Component\DependencyInjection\ContainerInterface as SymfonyContainer
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
 use Symfony\Component\DependencyInjection\EnvVarProcessorInterface;
+use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException;
 use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
 use Symfony\Component\DependencyInjection\Parameter;
 use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
@ -530,50 +531,22 @@ class PhpDumperTest extends TestCase
        $container->compile();

        $dumper = new PhpDumper($container);
-        $dumper->dump();
-
-        $this->addToAssertionCount(1);
-    }
-
-    public function testCircularReferenceAllowanceForInlinedDefinitionsForLazyServices()
-    {
-        /*
-         *   test graph:
-         *              [connection] -> [event_manager] --> [entity_manager](lazy)
-         *                                                           |
-         *                                                           --(call)- addEventListener ("@lazy_service")
-         *
-         *              [lazy_service](lazy) -> [entity_manager](lazy)
-         *
-         */
-
-        $container = new ContainerBuilder();
-
-        $eventManagerDefinition = new Definition('stdClass');
-
-        $connectionDefinition = $container->register('connection', 'stdClass')->setPublic(true);
-        $connectionDefinition->addArgument($eventManagerDefinition);
-
-        $container->register('entity_manager', 'stdClass')
-            ->setPublic(true)
-            ->setLazy(true)
-            ->addArgument(new Reference('connection'));
-
-        $lazyServiceDefinition = $container->register('lazy_service', 'stdClass');
-        $lazyServiceDefinition->setPublic(true);
-        $lazyServiceDefinition->setLazy(true);
-        $lazyServiceDefinition->addArgument(new Reference('entity_manager'));
-
-        $eventManagerDefinition->addMethodCall('addEventListener', array(new Reference('lazy_service')));
-
-        $container->compile();
-
-        $dumper = new PhpDumper($container);
-
        $dumper->setProxyDumper(new \DummyProxyDumper());
        $dumper->dump();

        $this->addToAssertionCount(1);
+
+        $dumper = new PhpDumper($container);
+
+        $message = 'Circular reference detected for service "bar", path: "bar -> foo -> bar". Try running "composer require symfony/proxy-manager-bridge".';
+        if (method_exists($this, 'expectException')) {
+            $this->expectException(ServiceCircularReferenceException::class);
+            $this->expectExceptionMessage($message);
+        } else {
+            $this->setExpectedException(ServiceCircularReferenceException::class, $message);
+        }
+
+        $dumper->dump();
    }

    public function testDedupLazyProxy()
@ -851,6 +824,12 @@ class PhpDumperTest extends TestCase

        $foo5 = $container->get('foo5');
        $this->assertSame($foo5, $foo5->bar->foo);
+
+        $manager = $container->get('manager');
+        $this->assertEquals(new \stdClass(), $manager);
+
+        $manager = $container->get('manager2');
+        $this->assertEquals(new \stdClass(), $manager);
    }

    public function provideAlmostCircular()
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/containers/container_almost_circular.php
@ -55,4 +55,50 @@ $container->register('bar5', 'stdClass')->setPublic($public)
    ->addArgument(new Reference('foo5'))
    ->setProperty('foo', new Reference('foo5'));

+// doctrine-like event system + some extra
+
+$container->register('manager', 'stdClass')->setPublic(true)
+    ->addArgument(new Reference('connection'));
+
+$container->register('logger', 'stdClass')->setPublic(true)
+    ->addArgument(new Reference('connection'))
+    ->setProperty('handler', (new Definition('stdClass'))->addArgument(new Reference('manager')))
+;
+$container->register('connection', 'stdClass')->setPublic(true)
+    ->addArgument(new Reference('dispatcher'))
+    ->addArgument(new Reference('config'));
+
+$container->register('config', 'stdClass')->setPublic(false)
+    ->setProperty('logger', new Reference('logger'));
+
+$container->register('dispatcher', 'stdClass')->setPublic($public)
+    ->setLazy($public)
+    ->setProperty('subscriber', new Reference('subscriber'));
+
+$container->register('subscriber', 'stdClass')->setPublic(true)
+    ->addArgument(new Reference('manager'));
+
+// doctrine-like event system + some extra (bis)
+
+$container->register('manager2', 'stdClass')->setPublic(true)
+    ->addArgument(new Reference('connection2'));
+
+$container->register('logger2', 'stdClass')->setPublic(false)
+    ->addArgument(new Reference('connection2'))
+    ->setProperty('handler2', (new Definition('stdClass'))->addArgument(new Reference('manager2')))
+;
+$container->register('connection2', 'stdClass')->setPublic(true)
+    ->addArgument(new Reference('dispatcher2'))
+    ->addArgument(new Reference('config2'));
+
+$container->register('config2', 'stdClass')->setPublic(false)
+    ->setProperty('logger2', new Reference('logger2'));
+
+$container->register('dispatcher2', 'stdClass')->setPublic($public)
+    ->setLazy($public)
+    ->setProperty('subscriber2', new Reference('subscriber2'));
+
+$container->register('subscriber2', 'stdClass')->setPublic(false)
+    ->addArgument(new Reference('manager2'));
+
 return $container;
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_private.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_private.php
@ -25,10 +25,16 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container
        $this->methodMap = array(
            'bar2' => 'getBar2Service',
            'bar3' => 'getBar3Service',
+            'connection' => 'getConnectionService',
+            'connection2' => 'getConnection2Service',
            'foo' => 'getFooService',
            'foo2' => 'getFoo2Service',
            'foo5' => 'getFoo5Service',
            'foobar4' => 'getFoobar4Service',
+            'logger' => 'getLoggerService',
+            'manager' => 'getManagerService',
+            'manager2' => 'getManager2Service',
+            'subscriber' => 'getSubscriberService',
        );

        $this->aliases = array();
@ -41,10 +47,16 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container
            'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
            'bar' => true,
            'bar5' => true,
+            'config' => true,
+            'config2' => true,
+            'dispatcher' => true,
+            'dispatcher2' => true,
            'foo4' => true,
            'foobar' => true,
            'foobar2' => true,
            'foobar3' => true,
+            'logger2' => true,
+            'subscriber2' => true,
        );
    }

@ -95,6 +107,49 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container
        return $instance;
    }

+    /**
+     * Gets the public 'connection' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getConnectionService()
+    {
+        $a = new \stdClass();
+
+        $b = new \stdClass();
+
+        $this->services['connection'] = $instance = new \stdClass($a, $b);
+
+        $a->subscriber = ${($_ = isset($this->services['subscriber']) ? $this->services['subscriber'] : $this->getSubscriberService()) && false ?: '_'};
+        $b->logger = ${($_ = isset($this->services['logger']) ? $this->services['logger'] : $this->getLoggerService()) && false ?: '_'};
+
+        return $instance;
+    }
+
+    /**
+     * Gets the public 'connection2' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getConnection2Service()
+    {
+        $a = new \stdClass();
+
+        $b = new \stdClass();
+
+        $this->services['connection2'] = $instance = new \stdClass($a, $b);
+
+        $c = ${($_ = isset($this->services['manager2']) ? $this->services['manager2'] : $this->getManager2Service()) && false ?: '_'};
+
+        $d = new \stdClass($instance);
+
+        $a->subscriber2 = new \stdClass($c);
+        $d->handler2 = new \stdClass($c);
+        $b->logger2 = $d;
+
+        return $instance;
+    }
+
    /**
     * Gets the public 'foo' shared service.
     *
@ -136,7 +191,7 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container
    {
        $this->services['foo5'] = $instance = new \stdClass();

-        $a = new \stdClass(${($_ = isset($this->services['foo5']) ? $this->services['foo5'] : $this->getFoo5Service()) && false ?: '_'});
+        $a = new \stdClass($instance);

        $a->foo = $instance;

@ -160,4 +215,72 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Private extends Container

        return $instance;
    }
+
+    /**
+     * Gets the public 'logger' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getLoggerService()
+    {
+        $a = ${($_ = isset($this->services['connection']) ? $this->services['connection'] : $this->getConnectionService()) && false ?: '_'};
+
+        if (isset($this->services['logger'])) {
+            return $this->services['logger'];
+        }
+
+        $this->services['logger'] = $instance = new \stdClass($a);
+
+        $instance->handler = new \stdClass(${($_ = isset($this->services['manager']) ? $this->services['manager'] : $this->getManagerService()) && false ?: '_'});
+
+        return $instance;
+    }
+
+    /**
+     * Gets the public 'manager' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getManagerService()
+    {
+        $a = ${($_ = isset($this->services['connection']) ? $this->services['connection'] : $this->getConnectionService()) && false ?: '_'};
+
+        if (isset($this->services['manager'])) {
+            return $this->services['manager'];
+        }
+
+        return $this->services['manager'] = new \stdClass($a);
+    }
+
+    /**
+     * Gets the public 'manager2' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getManager2Service()
+    {
+        $a = ${($_ = isset($this->services['connection2']) ? $this->services['connection2'] : $this->getConnection2Service()) && false ?: '_'};
+
+        if (isset($this->services['manager2'])) {
+            return $this->services['manager2'];
+        }
+
+        return $this->services['manager2'] = new \stdClass($a);
+    }
+
+    /**
+     * Gets the public 'subscriber' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getSubscriberService()
+    {
+        $a = ${($_ = isset($this->services['manager']) ? $this->services['manager'] : $this->getManagerService()) && false ?: '_'};
+
+        if (isset($this->services['subscriber'])) {
+            return $this->services['subscriber'];
+        }
+
+        return $this->services['subscriber'] = new \stdClass($a);
+    }
 }
--- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php
@ -26,6 +26,10 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
            'bar' => 'getBarService',
            'bar3' => 'getBar3Service',
            'bar5' => 'getBar5Service',
+            'connection' => 'getConnectionService',
+            'connection2' => 'getConnection2Service',
+            'dispatcher' => 'getDispatcherService',
+            'dispatcher2' => 'getDispatcher2Service',
            'foo' => 'getFooService',
            'foo2' => 'getFoo2Service',
            'foo4' => 'getFoo4Service',
@ -34,6 +38,10 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
            'foobar2' => 'getFoobar2Service',
            'foobar3' => 'getFoobar3Service',
            'foobar4' => 'getFoobar4Service',
+            'logger' => 'getLoggerService',
+            'manager' => 'getManagerService',
+            'manager2' => 'getManager2Service',
+            'subscriber' => 'getSubscriberService',
        );

        $this->aliases = array();
@ -45,6 +53,10 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
            'Psr\\Container\\ContainerInterface' => true,
            'Symfony\\Component\\DependencyInjection\\ContainerInterface' => true,
            'bar2' => true,
+            'config' => true,
+            'config2' => true,
+            'logger2' => true,
+            'subscriber2' => true,
        );
    }

@ -115,6 +127,81 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container
        return $instance;
    }

+    /**
+     * Gets the public 'connection' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getConnectionService()
+    {
+        $a = ${($_ = isset($this->services['dispatcher']) ? $this->services['dispatcher'] : $this->getDispatcherService()) && false ?: '_'};
+
+        if (isset($this->services['connection'])) {
+            return $this->services['connection'];
+        }
+
+        $b = new \stdClass();
+
+        $this->services['connection'] = $instance = new \stdClass($a, $b);
+
+        $b->logger = ${($_ = isset($this->services['logger']) ? $this->services['logger'] : $this->getLoggerService()) && false ?: '_'};
+
+        return $instance;
+    }
+
+    /**
+     * Gets the public 'connection2' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getConnection2Service()
+    {
+        $a = ${($_ = isset($this->services['dispatcher2']) ? $this->services['dispatcher2'] : $this->getDispatcher2Service()) && false ?: '_'};
+
+        if (isset($this->services['connection2'])) {
+            return $this->services['connection2'];
+        }
+
+        $b = new \stdClass();
+
+        $this->services['connection2'] = $instance = new \stdClass($a, $b);
+
+        $c = new \stdClass($instance);
+
+        $c->handler2 = new \stdClass(${($_ = isset($this->services['manager2']) ? $this->services['manager2'] : $this->getManager2Service()) && false ?: '_'});
+        $b->logger2 = $c;
+
+        return $instance;
+    }
+
+    /**
+     * Gets the public 'dispatcher' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getDispatcherService($lazyLoad = true)
+    {
+        $this->services['dispatcher'] = $instance = new \stdClass();
+
+        $instance->subscriber = ${($_ = isset($this->services['subscriber']) ? $this->services['subscriber'] : $this->getSubscriberService()) && false ?: '_'};
+
+        return $instance;
+    }
+
+    /**
+     * Gets the public 'dispatcher2' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getDispatcher2Service($lazyLoad = true)
+    {
+        $this->services['dispatcher2'] = $instance = new \stdClass();
+
+        $instance->subscriber2 = new \stdClass(${($_ = isset($this->services['manager2']) ? $this->services['manager2'] : $this->getManager2Service()) && false ?: '_'});
+
+        return $instance;
+    }
+
    /**
     * Gets the public 'foo' shared service.
     *
@ -232,4 +319,72 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container

        return $instance;
    }
+
+    /**
+     * Gets the public 'logger' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getLoggerService()
+    {
+        $a = ${($_ = isset($this->services['connection']) ? $this->services['connection'] : $this->getConnectionService()) && false ?: '_'};
+
+        if (isset($this->services['logger'])) {
+            return $this->services['logger'];
+        }
+
+        $this->services['logger'] = $instance = new \stdClass($a);
+
+        $instance->handler = new \stdClass(${($_ = isset($this->services['manager']) ? $this->services['manager'] : $this->getManagerService()) && false ?: '_'});
+
+        return $instance;
+    }
+
+    /**
+     * Gets the public 'manager' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getManagerService()
+    {
+        $a = ${($_ = isset($this->services['connection']) ? $this->services['connection'] : $this->getConnectionService()) && false ?: '_'};
+
+        if (isset($this->services['manager'])) {
+            return $this->services['manager'];
+        }
+
+        return $this->services['manager'] = new \stdClass($a);
+    }
+
+    /**
+     * Gets the public 'manager2' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getManager2Service()
+    {
+        $a = ${($_ = isset($this->services['connection2']) ? $this->services['connection2'] : $this->getConnection2Service()) && false ?: '_'};
+
+        if (isset($this->services['manager2'])) {
+            return $this->services['manager2'];
+        }
+
+        return $this->services['manager2'] = new \stdClass($a);
+    }
+
+    /**
+     * Gets the public 'subscriber' shared service.
+     *
+     * @return \stdClass
+     */
+    protected function getSubscriberService()
+    {
+        $a = ${($_ = isset($this->services['manager']) ? $this->services['manager'] : $this->getManagerService()) && false ?: '_'};
+
+        if (isset($this->services['subscriber'])) {
+            return $this->services['subscriber'];
+        }
+
+        return $this->services['subscriber'] = new \stdClass($a);
+    }
 }