Skip to content

Commit

Permalink
bug #25180 [DI] Fix circular reference when using setters (nicolas-gr…
Browse files Browse the repository at this point in the history 
…ekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[DI] Fix circular reference when using setters

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

I did not manage to make a reproducing test case, yet @deguif provided me an app that I could play with to debug and fix.

Commits
-------

de5eecc [DI] Fix circular reference when using setters
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed Nov 29, 2017
2 parents ad3ddeb + de5eecc commit d90a3b5
Show file tree
Hide file tree
Showing 11 changed files with 404 additions and 284 deletions.
62 changes: 31 additions & 31 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
View file
Expand Up @@ -570,10 +570,13 @@ public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INV
return $this->doGet($id, $invalidBehavior);
}

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

if (isset($inlineServices[$id])) {
return $inlineServices[$id];
}
if (ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior) {
return parent::get($id, $invalidBehavior);
}
Expand All @@ -582,7 +585,7 @@ private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_
}

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

try {
Expand All @@ -599,7 +602,7 @@ private function doGet($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_
$this->{$loading}[$id] = true;

try {
$service = $this->createService($definition, new \SplObjectStorage(), $id);
$service = $this->createService($definition, $inlineServices, $id);
} finally {
unset($this->{$loading}[$id]);
}
Expand Down Expand Up @@ -1054,10 +1057,10 @@ public function findDefinition($id)
* @throws RuntimeException When the service is a synthetic service
* @throws InvalidArgumentException When configure callable is not callable
*/
private function createService(Definition $definition, \SplObjectStorage $inlinedDefinitions, $id = null, $tryProxy = true)
private function createService(Definition $definition, array &$inlineServices, $id = null, $tryProxy = true)
{
if (null === $id && isset($inlinedDefinitions[$definition])) {
return $inlinedDefinitions[$definition];
if (null === $id && isset($inlineServices[$h = spl_object_hash($definition)])) {
return $inlineServices[$h];
}

if ($definition instanceof ChildDefinition) {
Expand All @@ -1078,11 +1081,11 @@ private function createService(Definition $definition, \SplObjectStorage $inline
->instantiateProxy(
$this,
$definition,
$id, function () use ($definition, $inlinedDefinitions, $id) {
return $this->createService($definition, $inlinedDefinitions, $id, false);
$id, function () use ($definition, &$inlineServices, $id) {
return $this->createService($definition, $inlineServices, $id, false);
}
);
$this->shareService($definition, $proxy, $id, $inlinedDefinitions);
$this->shareService($definition, $proxy, $id, $inlineServices);

return $proxy;
}
Expand All @@ -1093,15 +1096,15 @@ private function createService(Definition $definition, \SplObjectStorage $inline
require_once $parameterBag->resolveValue($definition->getFile());
}

$arguments = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments())), $inlinedDefinitions);
$arguments = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getArguments())), $inlineServices);

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]), $inlinedDefinitions), $factory[1]);
$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));
}
Expand Down Expand Up @@ -1130,26 +1133,26 @@ private function createService(Definition $definition, \SplObjectStorage $inline

if ($tryProxy || !$definition->isLazy()) {
// share only if proxying failed, or if not a proxy
$this->shareService($definition, $service, $id, $inlinedDefinitions);
$this->shareService($definition, $service, $id, $inlineServices);
}

$properties = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getProperties())), $inlinedDefinitions);
$properties = $this->doResolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getProperties())), $inlineServices);
foreach ($properties as $name => $value) {
$service->$name = $value;
}

foreach ($definition->getMethodCalls() as $call) {
$this->callMethod($service, $call, $inlinedDefinitions);
$this->callMethod($service, $call, $inlineServices);
}

if ($callable = $definition->getConfigurator()) {
if (is_array($callable)) {
$callable[0] = $parameterBag->resolveValue($callable[0]);

if ($callable[0] instanceof Reference) {
$callable[0] = $this->doGet((string) $callable[0], $callable[0]->getInvalidBehavior());
$callable[0] = $this->doGet((string) $callable[0], $callable[0]->getInvalidBehavior(), $inlineServices);
} elseif ($callable[0] instanceof Definition) {
$callable[0] = $this->createService($callable[0], $inlinedDefinitions);
$callable[0] = $this->createService($callable[0], $inlineServices);
}
}

Expand All @@ -1173,14 +1176,14 @@ private function createService(Definition $definition, \SplObjectStorage $inline
*/
public function resolveServices($value)
{
return $this->doResolveServices($value, new \SplObjectStorage());
return $this->doResolveServices($value);
}

private function doResolveServices($value, \SplObjectStorage $inlinedDefinitions)
private function doResolveServices($value, array &$inlineServices = array())
{
if (is_array($value)) {
foreach ($value as $k => $v) {
$value[$k] = $this->doResolveServices($v, $inlinedDefinitions);
$value[$k] = $this->doResolveServices($v, $inlineServices);
}
} elseif ($value instanceof ServiceClosureArgument) {
$reference = $value->getValues()[0];
Expand Down Expand Up @@ -1223,9 +1226,9 @@ private function doResolveServices($value, \SplObjectStorage $inlinedDefinitions
return $count;
});
} elseif ($value instanceof Reference) {
$value = $this->doGet((string) $value, $value->getInvalidBehavior());
$value = $this->doGet((string) $value, $value->getInvalidBehavior(), $inlineServices);
} elseif ($value instanceof Definition) {
$value = $this->createService($value, $inlinedDefinitions);
$value = $this->createService($value, $inlineServices);
} elseif ($value instanceof Expression) {
$value = $this->getExpressionLanguage()->evaluate($value, array('container' => $this));
}
Expand Down Expand Up @@ -1540,20 +1543,20 @@ private function getProxyInstantiator()
return $this->proxyInstantiator;
}

private function callMethod($service, $call, \SplObjectStorage $inlinedDefinitions)
private function callMethod($service, $call, array &$inlineServices)
{
foreach (self::getServiceConditionals($call[1]) as $s) {
if (!$this->has($s)) {
return;
}
}
foreach (self::getInitializedConditionals($call[1]) as $s) {
if (!$this->doGet($s, ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE)) {
if (!$this->doGet($s, ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE, $inlineServices)) {
return;
}
}

call_user_func_array(array($service, $call[0]), $this->doResolveServices($this->getParameterBag()->unescapeValue($this->getParameterBag()->resolveValue($call[1])), $inlinedDefinitions));
call_user_func_array(array($service, $call[0]), $this->doResolveServices($this->getParameterBag()->unescapeValue($this->getParameterBag()->resolveValue($call[1])), $inlineServices));
}

/**
Expand All @@ -1563,14 +1566,11 @@ private function callMethod($service, $call, \SplObjectStorage $inlinedDefinitio
* @param object $service
* @param string|null $id
*/
private function shareService(Definition $definition, $service, $id, \SplObjectStorage $inlinedDefinitions)
private function shareService(Definition $definition, $service, $id, array &$inlineServices)
{
if (!$definition->isShared()) {
return;
}
if (null === $id) {
$inlinedDefinitions[$definition] = $service;
} else {
$inlineServices[null !== $id ? $id : spl_object_hash($definition)] = $service;

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

0 comments on commit d90a3b5

Please sign in to comment.