Skip to content

Commit 0247b1b

Browse files
nicolas-grekasnicolas-grekas
committed
bug symfony#59723 [DependencyInjection] Fix cloned lazy services not sharing their dependencies when dumped with PhpDumper (pvandommelen)
This PR was squashed before being merged into the 6.4 branch. Discussion ---------- [DependencyInjection] Fix cloned lazy services not sharing their dependencies when dumped with PhpDumper | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix symfony#59706 | License | MIT Fixes referenced services not being shared when lazy services are cloned before being initialized. Adds tests for both the ghost and proxy scenario's. This makes the inlining behaviour more conservative, which impacts the outputs of some other test cases. First commit adds a test, second commit has the fix. Third commit also considers the proxy case, which is also affected. I can squash if necessary. Targeted 6.4, but this also applies to 7.x. Commits ------- a60cff5 [DependencyInjection] Fix cloned lazy services not sharing their dependencies when dumped with PhpDumper
2 parents e1ce353 + a60cff5 commit 0247b1b

12 files changed

+168
-17
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,8 @@ private function isInlineableDefinition(string $id, Definition $definition): boo
224224
return false;
225225
}
226226

227-
return $this->container->getDefinition($srcId)->isShared();
227+
$srcDefinition = $this->container->getDefinition($srcId);
228+
229+
return $srcDefinition->isShared() && !$srcDefinition->isLazy();
228230
}
229231
}

‎src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2185,6 +2185,12 @@ private function isSingleUsePrivateNode(ServiceReferenceGraphNode $node): bool
21852185
if ($edge->isLazy() || !$value instanceof Definition || !$value->isShared()) {
21862186
return false;
21872187
}
2188+
2189+
// When the source node is a proxy or ghost, it will construct its references only when the node itself is initialized.
2190+
// Since the node can be cloned before being fully initialized, we do not know how often its references are used.
2191+
if ($this->getProxyDumper()->isProxyCandidate($value)) {
2192+
return false;
2193+
}
21882194
$ids[$edge->getSourceNode()->getId()] = true;
21892195
}
21902196

‎src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@
5555
use Symfony\Component\DependencyInjection\Tests\Compiler\Wither;
5656
use Symfony\Component\DependencyInjection\Tests\Compiler\WitherAnnotation;
5757
use Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition;
58+
use Symfony\Component\DependencyInjection\Tests\Fixtures\DependencyContainer;
59+
use Symfony\Component\DependencyInjection\Tests\Fixtures\DependencyContainerInterface;
5860
use Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithEnumAttribute;
5961
use Symfony\Component\DependencyInjection\Tests\Fixtures\FooUnitEnum;
6062
use Symfony\Component\DependencyInjection\Tests\Fixtures\FooWithAbstractArgument;
@@ -1677,6 +1679,59 @@ public function testWitherWithStaticReturnType()
16771679
$this->assertInstanceOf(Foo::class, $wither->foo);
16781680
}
16791681

1682+
public function testCloningLazyGhostWithDependency()
1683+
{
1684+
$container = new ContainerBuilder();
1685+
$container->register('dependency', \stdClass::class);
1686+
$container->register(DependencyContainer::class)
1687+
->addArgument(new Reference('dependency'))
1688+
->setLazy(true)
1689+
->setPublic(true);
1690+
1691+
$container->compile();
1692+
$dumper = new PhpDumper($container);
1693+
$dump = $dumper->dump(['class' => 'Symfony_DI_PhpDumper_Service_CloningLazyGhostWithDependency']);
1694+
eval('?>'.$dump);
1695+
1696+
$container = new \Symfony_DI_PhpDumper_Service_CloningLazyGhostWithDependency();
1697+
1698+
$bar = $container->get(DependencyContainer::class);
1699+
$this->assertInstanceOf(DependencyContainer::class, $bar);
1700+
1701+
$first_clone = clone $bar;
1702+
$second_clone = clone $bar;
1703+
1704+
$this->assertSame($first_clone->dependency, $second_clone->dependency);
1705+
}
1706+
1707+
public function testCloningProxyWithDependency()
1708+
{
1709+
$container = new ContainerBuilder();
1710+
$container->register('dependency', \stdClass::class);
1711+
$container->register(DependencyContainer::class)
1712+
->addArgument(new Reference('dependency'))
1713+
->setLazy(true)
1714+
->addTag('proxy', [
1715+
'interface' => DependencyContainerInterface::class,
1716+
])
1717+
->setPublic(true);
1718+
1719+
$container->compile();
1720+
$dumper = new PhpDumper($container);
1721+
$dump = $dumper->dump(['class' => 'Symfony_DI_PhpDumper_Service_CloningProxyWithDependency']);
1722+
eval('?>'.$dump);
1723+
1724+
$container = new \Symfony_DI_PhpDumper_Service_CloningProxyWithDependency();
1725+
1726+
$bar = $container->get(DependencyContainer::class);
1727+
$this->assertInstanceOf(DependencyContainerInterface::class, $bar);
1728+
1729+
$first_clone = clone $bar;
1730+
$second_clone = clone $bar;
1731+
1732+
$this->assertSame($first_clone->getDependency(), $second_clone->getDependency());
1733+
}
1734+
16801735
public function testCurrentFactoryInlining()
16811736
{
16821737
$container = new ContainerBuilder();

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/DependencyContainer.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
13+
14+
class DependencyContainer implements DependencyContainerInterface
15+
{
16+
public function __construct(
17+
public mixed $dependency,
18+
) {
19+
}
20+
21+
public function getDependency(): mixed
22+
{
23+
return $this->dependency;
24+
}
25+
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/DependencyContainerInterface.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
13+
14+
interface DependencyContainerInterface
15+
{
16+
public function getDependency(): mixed;
17+
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/child.expected.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ services:
1111
- container.decorator: { id: bar, inner: b }
1212
file: file.php
1313
lazy: true
14-
arguments: [!service { class: Class1 }]
14+
arguments: ['@b']
15+
b:
16+
class: Class1
1517
bar:
1618
alias: foo
1719
public: true

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/from_callable.expected.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,7 @@ services:
88
class: stdClass
99
public: true
1010
lazy: true
11-
arguments: [[!service { class: stdClass }, do]]
11+
arguments: [['@bar', do]]
1212
factory: [Closure, fromCallable]
13+
bar:
14+
class: stdClass

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/closure_proxy.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,6 @@ protected function createProxy($class, \Closure $factory)
5555
*/
5656
protected static function getClosureProxyService($container, $lazyLoad = true)
5757
{
58-
return $container->services['closure_proxy'] = new class(fn () => new \Symfony\Component\DependencyInjection\Tests\Compiler\Foo()) extends \Symfony\Component\DependencyInjection\Argument\LazyClosure implements \Symfony\Component\DependencyInjection\Tests\Compiler\SingleMethodInterface { public function theMethod() { return $this->service->cloneFoo(...\func_get_args()); } };
58+
return $container->services['closure_proxy'] = new class(fn () => ($container->privates['foo'] ??= new \Symfony\Component\DependencyInjection\Tests\Compiler\Foo())) extends \Symfony\Component\DependencyInjection\Argument\LazyClosure implements \Symfony\Component\DependencyInjection\Tests\Compiler\SingleMethodInterface { public function theMethod() { return $this->service->cloneFoo(...\func_get_args()); } };
5959
}
6060
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/lazy_closure.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ protected function createProxy($class, \Closure $factory)
5757
*/
5858
protected static function getClosure1Service($container, $lazyLoad = true)
5959
{
60-
return $container->services['closure1'] = (new class(fn () => new \Symfony\Component\DependencyInjection\Tests\Compiler\Foo()) extends \Symfony\Component\DependencyInjection\Argument\LazyClosure { public function cloneFoo(?\stdClass $bar = null): \Symfony\Component\DependencyInjection\Tests\Compiler\Foo { return $this->service->cloneFoo(...\func_get_args()); } })->cloneFoo(...);
60+
return $container->services['closure1'] = (new class(fn () => ($container->privates['foo'] ??= new \Symfony\Component\DependencyInjection\Tests\Compiler\Foo())) extends \Symfony\Component\DependencyInjection\Argument\LazyClosure { public function cloneFoo(?\stdClass $bar = null): \Symfony\Component\DependencyInjection\Tests\Compiler\Foo { return $this->service->cloneFoo(...\func_get_args()); } })->cloneFoo(...);
6161
}
6262

6363
/**
@@ -67,6 +67,6 @@ protected static function getClosure1Service($container, $lazyLoad = true)
6767
*/
6868
protected static function getClosure2Service($container, $lazyLoad = true)
6969
{
70-
return $container->services['closure2'] = (new class(fn () => new \Symfony\Component\DependencyInjection\Tests\Compiler\FooVoid()) extends \Symfony\Component\DependencyInjection\Argument\LazyClosure { public function __invoke(string $name): void { $this->service->__invoke(...\func_get_args()); } })->__invoke(...);
70+
return $container->services['closure2'] = (new class(fn () => ($container->privates['foo_void'] ??= new \Symfony\Component\DependencyInjection\Tests\Compiler\FooVoid())) extends \Symfony\Component\DependencyInjection\Argument\LazyClosure { public function __invoke(string $name): void { $this->service->__invoke(...\func_get_args()); } })->__invoke(...);
7171
}
7272
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_private.php

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -373,15 +373,13 @@ protected static function getManager2Service($container)
373373
*/
374374
protected static function getManager3Service($container, $lazyLoad = true)
375375
{
376-
$a = ($container->services['listener3'] ?? self::getListener3Service($container));
376+
$a = ($container->privates['connection3'] ?? self::getConnection3Service($container));
377377

378378
if (isset($container->services['manager3'])) {
379379
return $container->services['manager3'];
380380
}
381-
$b = new \stdClass();
382-
$b->listener = [$a];
383381

384-
return $container->services['manager3'] = new \stdClass($b);
382+
return $container->services['manager3'] = new \stdClass($a);
385383
}
386384

387385
/**
@@ -481,6 +479,34 @@ protected static function getBar6Service($container)
481479
return $container->privates['bar6'] = new \stdClass($a);
482480
}
483481

482+
/**
483+
* Gets the private 'connection3' shared service.
484+
*
485+
* @return \stdClass
486+
*/
487+
protected static function getConnection3Service($container)
488+
{
489+
$container->privates['connection3'] = $instance = new \stdClass();
490+
491+
$instance->listener = [($container->services['listener3'] ?? self::getListener3Service($container))];
492+
493+
return $instance;
494+
}
495+
496+
/**
497+
* Gets the private 'connection4' shared service.
498+
*
499+
* @return \stdClass
500+
*/
501+
protected static function getConnection4Service($container)
502+
{
503+
$container->privates['connection4'] = $instance = new \stdClass();
504+
505+
$instance->listener = [($container->services['listener4'] ?? self::getListener4Service($container))];
506+
507+
return $instance;
508+
}
509+
484510
/**
485511
* Gets the private 'doctrine.listener' shared service.
486512
*
@@ -572,13 +598,13 @@ protected static function getMailerInline_TransportFactory_AmazonService($contai
572598
*/
573599
protected static function getManager4Service($container, $lazyLoad = true)
574600
{
575-
$a = new \stdClass();
601+
$a = ($container->privates['connection4'] ?? self::getConnection4Service($container));
576602

577-
$container->privates['manager4'] = $instance = new \stdClass($a);
578-
579-
$a->listener = [($container->services['listener4'] ?? self::getListener4Service($container))];
603+
if (isset($container->privates['manager4'])) {
604+
return $container->privates['manager4'];
605+
}
580606

581-
return $instance;
607+
return $container->privates['manager4'] = new \stdClass($a);
582608
}
583609

584610
/**

0 commit comments

Comments
 (0)