Skip to content

Commit d6e0af4

Browse files
nicolas-grekasnicolas-grekas
committed
bug #59810 [DependencyInjection] Defer check for circular references instead of skipping them (biozshock)
This PR was merged into the 6.4 branch. Discussion ---------- [DependencyInjection] Defer check for circular references instead of skipping them | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | License | MIT Allow Dependency injection to check for circular references for lazy services, if they ever appear as "non-lazy" later during the compilation. The issue happens when one of injected references happen to be in a service locator and injected into another service. Commits ------- 124087b [DependencyInjection] Defer check for circular references instead of skipping them.
2 parents b88f9b5 + 124087b commit d6e0af4

File tree

2 files changed

+43
-10
lines changed

2 files changed

+43
-10
lines changed

‎src/Symfony/Component/DependencyInjection/Compiler/CheckCircularReferencesPass.php

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ class CheckCircularReferencesPass implements CompilerPassInterface
2828
{
2929
private array $currentPath;
3030
private array $checkedNodes;
31+
private array $checkedLazyNodes;
3132

3233
/**
3334
* Checks the ContainerBuilder object for circular references.
@@ -59,22 +60,36 @@ private function checkOutEdges(array $edges): void
5960
$node = $edge->getDestNode();
6061
$id = $node->getId();
6162

62-
if (empty($this->checkedNodes[$id])) {
63-
// Don't check circular references for lazy edges
64-
if (!$node->getValue() || (!$edge->isLazy() && !$edge->isWeak())) {
65-
$searchKey = array_search($id, $this->currentPath);
66-
$this->currentPath[] = $id;
63+
if (!empty($this->checkedNodes[$id])) {
64+
continue;
65+
}
66+
67+
$isLeaf = !!$node->getValue();
68+
$isConcrete = !$edge->isLazy() && !$edge->isWeak();
69+
70+
// Skip already checked lazy services if they are still lazy. Will not gain any new information.
71+
if (!empty($this->checkedLazyNodes[$id]) && (!$isLeaf || !$isConcrete)) {
72+
continue;
73+
}
6774

68-
if (false !== $searchKey) {
69-
throw new ServiceCircularReferenceException($id, \array_slice($this->currentPath, $searchKey));
70-
}
75+
// Process concrete references, otherwise defer check circular references for lazy edges.
76+
if (!$isLeaf || $isConcrete) {
77+
$searchKey = array_search($id, $this->currentPath);
78+
$this->currentPath[] = $id;
7179

72-
$this->checkOutEdges($node->getOutEdges());
80+
if (false !== $searchKey) {
81+
throw new ServiceCircularReferenceException($id, \array_slice($this->currentPath, $searchKey));
7382
}
7483

84+
$this->checkOutEdges($node->getOutEdges());
85+
7586
$this->checkedNodes[$id] = true;
76-
array_pop($this->currentPath);
87+
unset($this->checkedLazyNodes[$id]);
88+
} else {
89+
$this->checkedLazyNodes[$id] = true;
7790
}
91+
92+
array_pop($this->currentPath);
7893
}
7994
}
8095
}

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckCircularReferencesPassTest.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,12 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
16+
use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
17+
use Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument;
1618
use Symfony\Component\DependencyInjection\Compiler\AnalyzeServiceReferencesPass;
1719
use Symfony\Component\DependencyInjection\Compiler\CheckCircularReferencesPass;
1820
use Symfony\Component\DependencyInjection\Compiler\Compiler;
21+
use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
1922
use Symfony\Component\DependencyInjection\ContainerBuilder;
2023
use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException;
2124
use Symfony\Component\DependencyInjection\Reference;
@@ -126,6 +129,21 @@ public function testProcessIgnoresLazyServices()
126129
$this->addToAssertionCount(1);
127130
}
128131

132+
public function testProcessDefersLazyServices()
133+
{
134+
$container = new ContainerBuilder();
135+
136+
$container->register('a')->addArgument(new ServiceLocatorArgument(new TaggedIteratorArgument('tag', needsIndexes: true)));
137+
$container->register('b')->addArgument(new Reference('c'))->addTag('tag');
138+
$container->register('c')->addArgument(new Reference('b'));
139+
140+
(new ServiceLocatorTagPass())->process($container);
141+
142+
$this->expectException(ServiceCircularReferenceException::class);
143+
144+
$this->process($container);
145+
}
146+
129147
public function testProcessIgnoresIteratorArguments()
130148
{
131149
$container = new ContainerBuilder();

0 commit comments

Comments
 (0)