8000 minor #48469 [DependencyInjection] Use WeakReference to break circula… · symfony/symfony@f35572f · GitHub
[go: up one dir, main page]
Skip to content

Commit f35572f

Browse files
fabpotfabpot
committed
minor #48469 [DependencyInjection] Use WeakReference to break circular references in the container (nicolas-grekas)
This PR was merged into the 6.3 branch. Discussion ---------- [DependencyInjection] Use WeakReference to break circular references in the container | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - While working on #48461, I realized that we could break many circular loops involving the container by using a `WeakReference` to access it. This happens when we generate closures. Conceptually, we generate this at the moment: ```php $internalClosure = function () { return $this->get('foo'); }; ``` And we'd generate this with this PR: ```php $containerRef = \WeakReference::create($this); $internalClosure = static function () use ($containerRef) { return $containerRef->get()->get('foo'); }; ``` This should reduce the pressure on the garbage collector. Commits ------- a3b7fca [DependencyInjection] Use WeakReference to break circular references in the container
2 parents 0c465c5 + a3b7fca commit f35572f

File tree

65 files changed

+1097
-843
lines changed
  • services_uninitialized_ref.php
  • services_unsupported_characters.php
  • services_url_env.php
  • services_wither.php
  • services_wither_lazy.php
  • services_wither_staticreturntype.php
  • composer.json

    • Some content is hidden

    Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

    65 files changed

    +1097
    -843
    lines changed

    ‎src/Symfony/Bridge/ProxyManager/LazyProxy/PhpDumper/ProxyDumper.php

    Lines changed: 4 additions & 3 deletions
    Original file line numberDiff line numberDiff line change
    @@ -53,16 +53,17 @@ public function getProxyFactoryCode(Definition $definition, string $id, string $
    5353
    $instantiation = 'return';
    5454

    5555
    if ($definition->isShared()) {
    56-
    $instantiation .= sprintf(' $this->%s[%s] =', $definition->isPublic() && !$definition->isPrivate() ? 'services' : 'privates', var_export($id, true));
    56+
    $instantiation .= sprintf(' $container->%s[%s] =', $definition->isPublic() && !$definition->isPrivate() ? 'services' : 'privates', var_export($id, true));
    5757
    }
    5858

    5959
    $proxifiedClass = new \ReflectionClass($this->proxyGenerator->getProxifiedClass($definition));
    6060
    $proxyClass = $this->getProxyClassName($proxifiedClass->name);
    6161

    6262
    return <<<EOF
    6363
    if (true === \$lazyLoad) {
    64-
    $instantiation \$this->createProxy('$proxyClass', function () {
    65-
    return \\$proxyClass::staticProxyConstructor(function (&\$wrappedInstance, \ProxyManager\Proxy\LazyLoadingInterface \$proxy) {
    64+
    $instantiation \$container->createProxy('$proxyClass', static function () use (\$containerRef) {
    65+
    return \\$proxyClass::staticProxyConstructor(static function (&\$wrappedInstance, \ProxyManager\Proxy\LazyLoadingInterface \$proxy) use (\$containerRef) {
    66+
    \$container = \$containerRef->get();
    6667
    \$wrappedInstance = $factoryCode;
    6768
    6869
    \$proxy->setProxyInitializer(null);

    ‎src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Fixtures/php/lazy_service_structure.txt

    Lines changed: 7 additions & 4 deletions
    Original file line numberDiff line numberDiff line change
    @@ -3,12 +3,15 @@
    33
    use %a
    44
    class LazyServiceProjectServiceContainer extends Container
    55
    {%a
    6-
    protected function getFooService($lazyLoad = true)
    6+
    protected static function getFooService($container, $lazyLoad = true)
    77
    {
    8+
    $containerRef = $container->ref;
    9+
    810
    if (true === $lazyLoad) {
    9-
    return $this->services['foo'] = $this->createProxy('stdClass_%s', function () {
    10-
    return %S\stdClass_%s(function (&$wrappedInstance, \ProxyManager\Proxy\LazyLoadingInterface $proxy) {
    11-
    $wrappedInstance = $this->getFooService(false);
    11+
    return $container->services['foo'] = $container->createProxy('stdClass_%s', static function () use ($containerRef) {
    12+
    return %S\stdClass_%s(static function (&$wrappedInstance, \ProxyManager\Proxy\LazyLoadingInterface $proxy) use ($containerRef) {
    13+
    $container = $containerRef->get();
    14+
    $wrappedInstance = self::getFooService($containerRef->get(), false);
    1215

    1316
    $proxy->setProxyInitializer(null);
    1417

    ‎src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/PhpDumper/Fixtures/proxy-factory.php

    Lines changed: 7 additions & 3 deletions
    Original file line numberDiff line numberDiff line change
    @@ -7,10 +7,14 @@
    77

    88
    public function getFooService($lazyLoad = true)
    99
    {
    10+
    $container = $this;
    11+
    $containerRef = \WeakReference::create($this);
    12+
    1013
    if (true === $lazyLoad) {
    11-
    return $this->privates['foo'] = $this->createProxy('SunnyInterface_1eff735', function () {
    12-
    return \SunnyInterface_1eff735::staticProxyConstructor(function (&$wrappedInstance, \ProxyManager\Proxy\LazyLoadingInterface $proxy) {
    13-
    $wrappedInstance = $this->getFooService(false);
    14+
    return $container->privates['foo'] = $container->createProxy('SunnyInterface_1eff735', static function () use ($containerRef) {
    15+
    return \SunnyInterface_1eff735::staticProxyConstructor(static function (&$wrappedInstance, \ProxyManager\Proxy\LazyLoadingInterface $proxy) use ($containerRef) {
    16+
    $container = $containerRef->get();
    17+
    $wrappedInstance = $container->getFooService(false);
    1418

    1519
    $proxy->setProxyInitializer(null);
    1620

    ‎src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/PhpDumper/ProxyDumperTest.php

    Lines changed: 8 additions & 5 deletions
    122
    Original file line numberDiff line numberDiff line change
    @@ -72,10 +72,10 @@ public function testGetProxyFactoryCode()
    7272

    7373
    $definition->setLazy(true);
    7474

    75-
    $code = $this->dumper->getProxyFactoryCode($definition, 'foo', '$this->getFoo2Service(false)');
    75+
    $code = $this->dumper->getProxyFactoryCode($definition, 'foo', '$container->getFoo2Service(false)');
    7676

    7777
    $this->assertStringMatchesFormat(
    78-
    '%A$wrappedInstance = $this->getFoo2Service(false);%w$proxy->setProxyInitializer(null);%A',
    78+
    '%A$wrappedInstance = $container->getFoo2Service(false);%w$proxy->setProxyInitializer(null);%A',
    7979
    $code
    8080
    );
    8181
    }
    @@ -87,9 +87,9 @@ public function testCorrectAssigning(Definition $definition, $access)
    8787
    {
    8888
    $definition->setLazy(true);
    8989

    90-
    $code = $this->dumper->getProxyFactoryCode($definition, 'foo', '$this->getFoo2Service(false)');
    90+
    $code = $this->dumper->getProxyFactoryCode($definition, 'foo', '$container->getFoo2Service(false)');
    9191

    92-
    $this->assertStringMatchesFormat('%A$this->'.$access.'[\'foo\'] = %A', $code);
    92+
    $this->assertStringMatchesFormat('%A$container->'.$access.'[\'foo\'] = %A', $code);
    9393
    }
    9494

    9595
    public function getPrivatePublicDefinitions()
    @@ -118,7 +118,7 @@ public function testGetProxyFactoryCodeForInterface()
    118118
    $definition->addTag('proxy', ['interface' => SunnyInterface::class]);
    119119

    120120
    $implem = "<?php\n\n".$this->dumper->getProxyCode($definition);
    121-
    $factory = $this->dumper->getProxyFactoryCode($definition, 'foo', '$this->getFooService(false)');
    121+
    $factory = $this->dumper->getProxyFactoryCode($definition, 'foo', '$container->getFooService(false)');
    122
    $factory = <<<EOPHP
    123123
    <?php
    124124
    @@ -129,6 +129,9 @@ public function testGetProxyFactoryCodeForInterface()
    129129
    130130
    public function getFooService(\$lazyLoad = true)
    131131
    {
    132+
    \$container = \$this;
    133+
    \$containerRef = \\WeakReference::create(\$this);
    134+
    132135
    {$factory} return new {$class}();
    133136
    }
    134137

    ‎src/Symfony/Bridge/ProxyManager/composer.json

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -18,7 +18,7 @@
    1818
    "require": {
    1919
    "php": ">=8.1",
    2020
    "friendsofphp/proxy-manager-lts": "^1.0.2",
    21-
    "symfony/dependency-injection": "^6.2",
    21+
    "symfony/dependency-injection": "^6.3",
    2222
    "symfony/deprecation-contracts": "^2.1|^3"
    2323
    },
    2424
    "require-dev": {

    ‎src/Symfony/Component/DependencyInjection/Container.php

    Lines changed: 20 additions & 18 deletions
    10000
    Original file line numberDiff line numberDiff line change
    @@ -63,6 +63,8 @@ class Container implements ContainerInterface, ResetInterface
    6363
    private bool $compiled = false;
    6464
    private \Closure $getEnv;
    6565

    66+
    private static $make;
    67+
    6668
    public function __construct(ParameterBagInterface $parameterBag = null)
    6769
    {
    6870
    $this->parameterBag = $parameterBag ?? new EnvPlaceholderParameterBag();
    @@ -135,7 +137,7 @@ public function set(string $id, ?object $service)
    135137
    if (isset($this->privates['service_container']) && $this->privates['service_container'] instanceof \Closure) {
    136138
    $initialize = $this->privates['service_container'];
    137139
    unset($this->privates['service_container']);
    138-
    $initialize();
    140+
    $initialize($this);
    139141
    }
    140142

    141143
    if ('service_container' === $id) {
    @@ -195,49 +197,49 @@ public function get(string $id, int $invalidBehavior = self::EXCEPTION_ON_INVALI
    195197
    {
    196198
    return $this->services[$id]
    197199
    ?? $this->services[$id = $this->aliases[$id] ?? $id]
    198-
    ?? ('service_container' === $id ? $this : ($this->factories[$id] ?? $this->make(...))($id, $invalidBehavior));
    200+
    ?? ('service_container' === $id ? $this : ($this->factories[$id] ?? self::$make ??= self::make(...))($this, $id, $invalidBehavior));
    199201
    }
    200202

    201203
    /**
    202204
    * Creates a service.
    203205
    *
    204206
    * As a separate method to allow "get()" to use the really fast `??` operator.
    205207
    */
    206-
    private function make(string $id, int $invalidBehavior)
    208+
    private static function make($container, string $id, int $invalidBehavior)
    207209
    {
    208-
    if (isset($this->loading[$id])) {
    209-
    throw new ServiceCircularReferenceException($id, array_merge(array_keys($this->loading), [$id]));
    210+
    if (isset($container->loading[$id])) {
    211+
    throw new ServiceCircularReferenceException($id, array_merge(array_keys($container->loading), [$id]));
    210212
    }
    211213

    212-
    $this->loading[$id] = true;
    214+
    $container->loading[$id] = true;
    213215

    214216
    try {
    215-
    if (isset($this->fileMap[$id])) {
    216-
    return /* self::IGNORE_ON_UNINITIALIZED_REFERENCE */ 4 === $invalidBehavior ? null : $this->load($this->fileMap[$id]);
    217-
    } elseif (isset($this->methodMap[$id])) {
    218-
    return /* self::IGNORE_ON_UNINITIALIZED_REFERENCE */ 4 === $invalidBehavior ? null : $this->{$this->methodMap[$id]}();
    217+
    if (isset($container->fileMap[$id])) {
    218+
    return /* self::IGNORE_ON_UNINITIALIZED_REFERENCE */ 4 === $invalidBehavior ? null : $container->load($container->fileMap[$id]);
    219+
    } elseif (isset($container->methodMap[$id])) {
    220+
    return /* self::IGNORE_ON_UNINITIALIZED_REFERENCE */ 4 === $invalidBehavior ? null : $container->{$container->methodMap[$id]}($container);
    219221
    }
    220222
    } catch (\Exception $e) {
    221-
    unset($this->services[$id]);
    223+
    unset($container->services[$id]);
    222224

    223225
    throw $e;
    224226
    } finally {
    225-
    unset($this->loading[$id]);
    227+
    unset($container->loading[$id]);
    226228
    }
    227229

    228230
    if (self::EXCEPTION_ON_INVALID_REFERENCE === $invalidBehavior) {
    229231
    if (!$id) {
    230232
    throw new ServiceNotFoundException($id);
    231233
    }
    232-
    if (isset($this->syntheticIds[$id])) {
    234+
    if (isset($container->syntheticIds[$id])) {
    233235
    throw new ServiceNotFoundException($id, null, null, [], sprintf('The "%s" service is synthetic, it needs to be set at boot time before it can be used.', $id));
    234236
    }
    235-
    if (isset($this->getRemovedIds()[$id])) {
    237+
    if (isset($container->getRemovedIds()[$id])) {
    236238
    throw new ServiceNotFoundException($id, null, null, [], sprintf('The "%s" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead.', $id));
    237239
    }
    238240

    239241
    $alternatives = [];
    240-
    foreach ($this->getServiceIds() as $knownId) {
    242+
    foreach ($container->getServiceIds() as $knownId) {
    241243
    if ('' === $knownId || '.' === $knownId[0]) {
    242244
    continue;
    243245
    }
    @@ -378,13 +380,13 @@ final protected function getService(string|false $registry, string $id, ?string
    378380
    return false !== $registry ? $this->{$registry}[$id] ?? null : null;
    379381
    }
    380382
    if (false !== $registry) {
    381-
    return $this->{$registry}[$id] ??= $load ? $this->load($method) : $this->{$method}();
    383+
    return $this->{$registry}[$id] ??= $load ? $this->load($method) : $this->{$method}($this);
    382384
    }
    383385
    if (!$load) {
    384-
    return $this->{$method}();
    386+
    return $this->{$method}($this);
    385387
    }
    386388

    387-
    return ($factory = $this->factories[$id] ?? $this->factories['service_container'][$id] ?? null) ? $factory() : $this->load($method);
    389+
    return ($factory = $this->factories[$id] ?? $this->factories['service_container'][$id] ?? null) ? $factory($this) : $this->load($method);
    388390
    }
    389391

    390392
    private function __clone()

    0 commit comments

    Comments
     (0)
    0