Skip to content

Commit 84c97e5

Browse files
jderussejderusse
committed
Fix circular detection with multiple paths
1 parent eaddaff commit 84c97e5

File tree

6 files changed

+183
-3
lines changed

6 files changed

+183
-3
lines changed

‎Dumper/PhpDumper.php‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -413,32 +413,66 @@ private function analyzeReferences()
413413
$this->singleUsePrivateIds = array_diff_key($this->singleUsePrivateIds, $this->circularReferences);
414414
}
415415

416-
private function collectCircularReferences(string $sourceId, array $edges, array &$checkedNodes, array $path = [], bool $byConstructor = true): void
416+
private function collectCircularReferences(string $sourceId, array $edges, array &$checkedNodes, array &$loops = [], array $path = [], bool $byConstructor = true): void
417417
{
418418
$path[$sourceId] = $byConstructor;
419419
$checkedNodes[$sourceId] = true;
420420
foreach ($edges as $edge) {
421421
$node = $edge->getDestNode();
422422
$id = $node->getId();
423-
424423
if (!($definition = $node->getValue()) instanceof Definition || $sourceId === $id || ($edge->isLazy() && ($this->proxyDumper ?? $this->getProxyDumper())->isProxyCandidate($definition)) || $edge->isWeak()) {
425424
continue;
426425
}
427426

428427
if (isset($path[$id])) {
429428
$loop = null;
430429
$loopByConstructor = $edge->isReferencedByConstructor();
430+
$pathInLoop = [$id, []];
431431
foreach ($path as $k => $pathByConstructor) {
432432
if (null !== $loop) {
433433
$loop[] = $k;
434+
$pathInLoop[1][$k] = $pathByConstructor;
435+
$loops[$k][] = &$pathInLoop;
434436
$loopByConstructor = $loopByConstructor && $pathByConstructor;
435437
} elseif ($k === $id) {
436438
$loop = [];
437439
}
438440
}
439441
$this->addCircularReferences($id, $loop, $loopByConstructor);
440442
} elseif (!isset($checkedNodes[$id])) {
441-
$this->collectCircularReferences($id, $node->getOutEdges(), $checkedNodes, $path, $edge->isReferencedByConstructor());
443+
$this->collectCircularReferences($id, $node->getOutEdges(), $checkedNodes, $loops, $path, $edge->isReferencedByConstructor());
444+
} elseif (isset($loops[$id])) {
445+
// we already had detected loops for this edge
446+
// let's check if we have a common ancestor in one of the detected loops
447+
foreach ($loops[$id] as [$first, $loopPath]) {
448+
if (!isset($path[$first])) {
449+
continue;
450+
}
451+
// We have a common ancestor, let's fill the current path
452+
$fillPath = null;
453+
foreach ($loopPath as $k => $pathByConstructor) {
454+
if (null !== $fillPath) {
455+
$fillPath[$k] = $pathByConstructor;
456+
} elseif ($k === $id) {
457+
$fillPath = $path;
458+
$fillPath[$k] = $pathByConstructor;
459+
}
460+
}
461+
462+
// we can now build the loop
463+
$loop = null;
464+
$loopByConstructor = $edge->isReferencedByConstructor();
465+
foreach ($fillPath as $k => $pathByConstructor) {
466+
if (null !== $loop) {
467+
$loop[] = $k;
468+
$loopByConstructor = $loopByConstructor && $pathByConstructor;
469+
} elseif ($k === $first) {
470+
$loop = [];
471+
}
472+
}
473+
$this->addCircularReferences($first, $loop, true);
474+
break;
475+
}
442476
}
443477
}
444478
unset($path[$sourceId]);

‎Tests/ContainerBuilderTest.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,6 +1374,9 @@ public function testAlmostCircular($visibility)
13741374
$container = include __DIR__.'/Fixtures/containers/container_almost_circular.php';
13751375
$container->compile();
13761376

1377+
$pA = $container->get('pA');
1378+
$this->assertEquals(new \stdClass(), $pA);
1379+
13771380
$logger = $container->get('monolog.logger');
13781381
$this->assertEquals(new \stdClass(), $logger->handler);
13791382

‎Tests/Dumper/PhpDumperTest.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,9 @@ public function testAlmostCircular($visibility)
10541054

10551055
$container = new $container();
10561056

1057+
$pA = $container->get('pA');
1058+
$this->assertEquals(new \stdClass(), $pA);
1059+
10571060
$logger = $container->get('monolog.logger');
10581061
$this->assertEquals(new \stdClass(), $logger->handler);
10591062

‎Tests/Fixtures/containers/container_almost_circular.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,21 @@
99
$public = 'public' === $visibility;
1010
$container = new ContainerBuilder();
1111

12+
// multiple path detection
13+
14+
$container->register('pA', 'stdClass')->setPublic(true)
15+
->addArgument(new Reference('pB'))
16+
->addArgument(new Reference('pC'));
17+
18+
$container->register('pB', 'stdClass')->setPublic($public)
19+
->setProperty('d', new Reference('pD'));
20+
$container->register('pC', 'stdClass')->setPublic($public)
21+
->setLazy(true)
22+
->setProperty('d', new Reference('pD'));
23+
24+
$container->register('pD', 'stdClass')->setPublic($public)
25+
->addArgument(new Reference('pA'));
26+
1227
// monolog-like + handler that require monolog
1328

1429
$container->register('monolog.logger', 'stdClass')->setPublic(true)

‎Tests/Fixtures/php/services_almost_circular_private.php‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public function __construct()
4040
'manager2' => 'getManager2Service',
4141
'manager3' => 'getManager3Service',
4242
'monolog.logger' => 'getMonolog_LoggerService',
43+
'pA' => 'getPAService',
4344
'root' => 'getRootService',
4445
'subscriber' => 'getSubscriberService',
4546
];
@@ -87,6 +88,9 @@ public function getRemovedIds(): array
8788
'manager4' => true,
8889
'monolog.logger_2' => true,
8990
'multiuse1' => true,
91+
'pB' => true,
92+
'pC' => true,
93+
'pD' => true,
9094
'subscriber2' => true,
9195
];
9296
}
@@ -374,6 +378,28 @@ protected function getMonolog_LoggerService()
374378
return $instance;
375379
}
376380

381+
/**
382+
* Gets the public 'pA' shared service.
383+
*
384+
* @return \stdClass
385+
*/
386+
protected function getPAService()
387+
{
388+
$a = new \stdClass();
389+
390+
$b = ($this->privates['pC'] ?? $this->getPCService());
391+
392+
if (isset($this->services['pA'])) {
393+
return $this->services['pA'];
394+
}
395+
396+
$this->services['pA'] = $instance = new \stdClass($a, $b);
397+
398+
$a->d = ($this->privates['pD'] ?? $this->getPDService());
399+
400+
return $instance;
401+
}
402+
377403
/**
378404
* Gets the public 'root' shared service.
379405
*
@@ -481,4 +507,34 @@ protected function getManager4Service($lazyLoad = true)
481507

482508
return $instance;
483509
}
510+
511+
/**
512+
* Gets the private 'pC' shared service.
513+
*
514+
* @return \stdClass
515+
*/
516+
protected function getPCService($lazyLoad = true)
517+
{
518+
$this->privates['pC'] = $instance = new \stdClass();
519+
520+
$instance->d = ($this->privates['pD'] ?? $this->getPDService());
521+
522+
return $instance;
523+
}
524+
525+
/**
526+
* Gets the private 'pD' shared service.
527+
*
528+
* @return \stdClass
529+
*/
530+
protected function getPDService()
531+
{
532+
$a = ($this->services['pA'] ?? $this->getPAService());
533+
534+
if (isset($this->privates['pD'])) {
535+
return $this->privates['pD'];
536+
}
537+
538+
return $this->privates['pD'] = new \stdClass($a);
539+
}
484540
}

‎Tests/Fixtures/php/services_almost_circular_public.php‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ public function __construct()
5353
'manager3' => 'getManager3Service',
5454
'monolog.logger' => 'getMonolog_LoggerService',
5555
'monolog.logger_2' => 'getMonolog_Logger2Service',
56+
'pA' => 'getPAService',
57+
'pB' => 'getPBService',
58+
'pC' => 'getPCService',
59+
'pD' => 'getPDService',
5660
'root' => 'getRootService',
5761
'subscriber' => 'getSubscriberService',
5862
];
@@ -558,6 +562,71 @@ protected function getMonolog_Logger2Service()
558562
return $instance;
559563
}
560564

565+
/**
566+
* Gets the public 'pA' shared service.
567+
*
568+
* @return \stdClass
569+
*/
570+
protected function getPAService()
571+
{
572+
$a = ($this->services['pB'] ?? $this->getPBService());
573+
574+
if (isset($this->services['pA'])) {
575+
return $this->services['pA'];
576+
}
577+
$b = ($this->services['pC'] ?? $this->getPCService());
578+
579+
if (isset($this->services['pA'])) {
580+
return $this->services['pA'];
581+
}
582+
583+
return $this->services['pA'] = new \stdClass($a, $b);
584+
}
585+
586+
/**
587+
* Gets the public 'pB' shared service.
588+
*
589+
* @return \stdClass
590+
*/
591+
protected function getPBService()
592+
{
593+
$this->services['pB'] = $instance = new \stdClass();
594+
595+
$instance->d = ($this->services['pD'] ?? $this->getPDService());
596+
597+
return $instance;
598+
}
599+
600+
/**
601+
* Gets the public 'pC' shared service.
602+
*
603+
* @return \stdClass
604+
*/
605+
protected function getPCService($lazyLoad = true)
606+
{
607+
$this->services['pC'] = $instance = new \stdClass();
608+
609+
$instance->d = ($this->services['pD'] ?? $this->getPDService());
610+
611+
return $instance;
612+
}
613+
614+
/**
615+
* Gets the public 'pD' shared service.
616+
*
617+
* @return \stdClass
618+
*/
619+
protected function getPDService()
620+
{
621+
$a = ($this->services['pA'] ?? $this->getPAService());
622+
623+
if (isset($this->services['pD'])) {
624+
return $this->services['pD'];
625+
}
626+
627+
return $this->services['pD'] = new \stdClass($a);
628+
}
629+
561630
/**
562631
* Gets the public 'root' shared service.
563632
*

0 commit comments

Comments
 (0)