Skip to content

Commit 58f2988

Browse files
nicolas-grekasnicolas-grekas
committed
[DependencyInjection] Fix deduplicating service instances in circular graphs
1 parent 9af18ce commit 58f2988

File tree

3 files changed

+51
-20
lines changed

3 files changed

+51
-20
lines changed

‎Dumper/PhpDumper.php‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
3333
use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException;
3434
use Symfony\Component\DependencyInjection\ExpressionLanguage;
35-
use Symfony\Component\DependencyInjection\LazyProxy\PhpDumper\DumperInterface as ProxyDumper;
35+
use Symfony\Component\DependencyInjection\LazyProxy\PhpDumper\DumperInterface;
3636
use Symfony\Component\DependencyInjection\LazyProxy\PhpDumper\NullDumper;
3737
use Symfony\Component\DependencyInjection\Loader\FileLoader;
3838
use Symfony\Component\DependencyInjection\Parameter;
@@ -95,9 +95,10 @@ class PhpDumper extends Dumper
9595
private $baseClass;
9696

9797
/**
98-
* @var ProxyDumper
98+
* @var DumperInterface
9999
*/
100100
private $proxyDumper;
101+
private $hasProxyDumper = false;
101102

102103
/**
103104
* {@inheritdoc}
@@ -114,9 +115,10 @@ public function __construct(ContainerBuilder $container)
114115
/**
115116
* Sets the dumper to be used when dumping proxies in the generated container.
116117
*/
117-
public function setProxyDumper(ProxyDumper $proxyDumper)
118+
public function setProxyDumper(DumperInterface $proxyDumper)
118119
{
119120
$this->proxyDumper = $proxyDumper;
121+
$this->hasProxyDumper = !$proxyDumper instanceof NullDumper;
120122
}
121123

122124
/**
@@ -174,7 +176,7 @@ public function dump(array $options = [])
174176

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

177-
if ($this->getProxyDumper() instanceof NullDumper) {
179+
if (!$this->hasProxyDumper) {
178180
(new AnalyzeServiceReferencesPass(true, false))->process($this->container);
179181
try {
180182
(new CheckCircularReferencesPass())->process($this->container);
@@ -407,7 +409,7 @@ class %s extends {$options['class']}
407409
/**
408410
* Retrieves the currently set proxy dumper or instantiates one.
409411
*/
410-
private function getProxyDumper(): ProxyDumper
412+
private function getProxyDumper(): DumperInterface
411413
{
412414
if (!$this->proxyDumper) {
413415
$this->proxyDumper = new NullDumper();
@@ -418,7 +420,7 @@ private function getProxyDumper(): ProxyDumper
418420

419421
private function analyzeReferences()
420422
{
421-
(new AnalyzeServiceReferencesPass(false, !$this->getProxyDumper() instanceof NullDumper))->process($this->container);
423+
(new AnalyzeServiceReferencesPass(false, $this->hasProxyDumper))->process($this->container);
422424
$checkedNodes = [];
423425
$this->circularReferences = [];
424426
$this->singleUsePrivateIds = [];
@@ -445,13 +447,13 @@ private function collectCircularReferences(string $sourceId, array $edges, array
445447
foreach ($edges as $edge) {
446448
$node = $edge->getDestNode();
447449
$id = $node->getId();
448-
if ($sourceId === $id || !$node->getValue() instanceof Definition || $edge->isLazy() || $edge->isWeak()) {
450+
if ($sourceId === $id || !$node->getValue() instanceof Definition || $edge->isWeak()) {
449451
continue;
450452
}
451453

452454
if (isset($path[$id])) {
453455
$loop = null;
454-
$loopByConstructor = $edge->isReferencedByConstructor();
456+
$loopByConstructor = $edge->isReferencedByConstructor() && !$edge->isLazy();
455457
$pathInLoop = [$id, []];
456458
foreach ($path as $k => $pathByConstructor) {
457459
if (null !== $loop) {
@@ -465,7 +467,7 @@ private function collectCircularReferences(string $sourceId, array $edges, array
465467
}
466468
$this->addCircularReferences($id, $loop, $loopByConstructor);
467469
} elseif (!isset($checkedNodes[$id])) {
468-
$this->collectCircularReferences($id, $node->getOutEdges(), $checkedNodes, $loops, $path, $edge->isReferencedByConstructor());
470+
$this->collectCircularReferences($id, $node->getOutEdges(), $checkedNodes, $loops, $path, $edge->isReferencedByConstructor() && !$edge->isLazy());
469471
} elseif (isset($loops[$id])) {
470472
// we already had detected loops for this edge
471473
// let's check if we have a common ancestor in one of the detected loops
@@ -486,7 +488,7 @@ private function collectCircularReferences(string $sourceId, array $edges, array
486488

487489
// we can now build the loop
488490
$loop = null;
489-
$loopByConstructor = $edge->isReferencedByConstructor();
491+
$loopByConstructor = $edge->isReferencedByConstructor() && !$edge->isLazy();
490492
foreach ($fillPath as $k => $pathByConstructor) {
491493
if (null !== $loop) {
492494
$loop[] = $k;
@@ -988,7 +990,7 @@ private function addInlineReference(string $id, Definition $definition, string $
988990
return '';
989991
}
990992

991-
$hasSelfRef = isset($this->circularReferences[$id][$targetId]) && !isset($this->definitionVariables[$definition]);
993+
$hasSelfRef = isset($this->circularReferences[$id][$targetId]) && !isset($this->definitionVariables[$definition]) && !($this->hasProxyDumper && $definition->isLazy());
992994

993995
if ($hasSelfRef && !$forConstructor && !$forConstructor = !$this->circularReferences[$id][$targetId]) {
994996
$code = $this->addInlineService($id, $definition, $definition);
@@ -1031,7 +1033,7 @@ private function addInlineService(string $id, Definition $definition, Definition
10311033

10321034
if ($isSimpleInstance = $isRootInstance = null === $inlineDef) {
10331035
foreach ($this->serviceCalls as $targetId => [$callCount, $behavior, $byConstructor]) {
1034-
if ($byConstructor && isset($this->circularReferences[$id][$targetId]) && !$this->circularReferences[$id][$targetId]) {
1036+
if ($byConstructor && isset($this->circularReferences[$id][$targetId]) && !$this->circularReferences[$id][$targetId] && !($this->hasProxyDumper && $definition->isLazy())) {
10351037
$code .= $this->addInlineReference($id, $definition, $targetId, $forConstructor);
10361038
}
10371039
}

‎Tests/Fixtures/php/services_almost_circular_private.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,13 @@ protected function getBar6Service()
491491
*/
492492
protected function getDoctrine_ListenerService()
493493
{
494-
return $this->privates['doctrine.listener'] = new \stdClass(($this->services['doctrine.entity_manager'] ?? $this->getDoctrine_EntityManagerService()));
494+
$a = ($this->services['doctrine.entity_manager'] ?? $this->getDoctrine_EntityManagerService());
495+
496+
if (isset($this->privates['doctrine.listener'])) {
497+
return $this->privates['doctrine.listener'];
498+
}
499+
500+
return $this->privates['doctrine.listener'] = new \stdClass($a);
495501
}
496502

497503
/**

‎Tests/Fixtures/php/services_almost_circular_public.php‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -285,11 +285,16 @@ protected function getDoctrine_EntityListenerResolverService()
285285
*/
286286
protected function getDoctrine_EntityManagerService()
287287
{
288-
$a = new \stdClass();
289-
$a->resolver = ($this->services['doctrine.entity_listener_resolver'] ?? $this->getDoctrine_EntityListenerResolverService());
290-
$a->flag = 'ok';
288+
$a = ($this->services['doctrine.entity_listener_resolver'] ?? $this->getDoctrine_EntityListenerResolverService());
289+
290+
if (isset($this->services['doctrine.entity_manager'])) {
291+
return $this->services['doctrine.entity_manager'];
292+
}
293+
$b = new \stdClass();
294+
$b->resolver = $a;
295+
$b->flag = 'ok';
291296

292-
return $this->services['doctrine.entity_manager'] = \FactoryChecker::create($a);
297+
return $this->services['doctrine.entity_manager'] = \FactoryChecker::create($b);
293298
}
294299

295300
/**
@@ -299,7 +304,13 @@ protected function getDoctrine_EntityManagerService()
299304
*/
300305
protected function getDoctrine_ListenerService()
301306
{
302-
return $this->services['doctrine.listener'] = new \stdClass(($this->services['doctrine.entity_manager'] ?? $this->getDoctrine_EntityManagerService()));
307+
$a = ($this->services['doctrine.entity_manager'] ?? $this->getDoctrine_EntityManagerService());
308+
309+
if (isset($this->services['doctrine.listener'])) {
310+
return $this->services['doctrine.listener'];
311+
}
312+
313+
return $this->services['doctrine.listener'] = new \stdClass($a);
303314
}
304315

305316
/**
@@ -495,7 +506,13 @@ protected function getLoggerService()
495506
*/
496507
protected function getMailer_TransportService()
497508
{
498-
return $this->services['mailer.transport'] = ($this->services['mailer.transport_factory'] ?? $this->getMailer_TransportFactoryService())->create();
509+
$a = ($this->services['mailer.transport_factory'] ?? $this->getMailer_TransportFactoryService());
510+
511+
if (isset($this->services['mailer.transport'])) {
512+
return $this->services['mailer.transport'];
513+
}
514+
515+
return $this->services['mailer.transport'] = $a->create();
499516
}
500517

501518
/**
@@ -518,7 +535,13 @@ protected function getMailer_TransportFactoryService()
518535
*/
519536
protected function getMailer_TransportFactory_AmazonService()
520537
{
521-
return $this->services['mailer.transport_factory.amazon'] = new \stdClass(($this->services['monolog.logger_2'] ?? $this->getMonolog_Logger2Service()));
538+
$a = ($this->services['monolog.logger_2'] ?? $this->getMonolog_Logger2Service());
539+
540+
if (isset($this->services['mailer.transport_factory.amazon'])) {
541+
return $this->services['mailer.transport_factory.amazon'];
542+
}
543+
544+
return $this->services['mailer.transport_factory.amazon'] = new \stdClass($a);
522545
}
523546

524547
/**

0 commit comments

Comments
 (0)