10000 bug #32809 Don't add object-value of static properties in the signatu… · symfony/symfony@5c67a67 · GitHub
[go: up one dir, main page]
Skip to content

Commit 5c67a67

Browse files
nicolas-grekasnicolas-grekas
committed
bug #32809 Don't add object-value of static properties in the signature of container metadata-cache (arjenm)
This PR was merged into the 3.4 branch. Discussion ---------- Don't add object-value of static properties in the signature of container metadata-cache | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | No ticket created, see below | License | MIT Running bin/console cache:clear can take a very long time and/or uses explosive amounts of memory if large protected/public static objects are available in classes for which metadata will be generated during the 'container dump'. I.e. in \Symfony\Component\HttpKernel\Kernel::dumpContainer eventually a call to \Symfony\Component\Config\ResourceCheckerConfigCache::write is done. That writes a serialized version of the $metadata. In that serialize a call to \Symfony\Component\Config\Resource\ReflectionClassResource::generateSignature for each service will eventually do a print_r on the class's (default) properties. Class properties can also be (public/protected) static. PHP Reflection doesn't differentiate between those in that context. And _those_ static objects can also have non-scalar values, including objects. If such an object is very large, for instance a service that references the Container (with many services, some also referencing the container, etc), the print_r will run into very deep recursions. I.e. it will either take "forever" or use so much memory the OOM killer is activated (it got to 25GB...). This scenario only triggers when a cache existed prior to running cache:clear, i.e. when 'isFresh' is called. When first removing the cache dir (or running cache:clear with --no-warmup) and than running cache:clear does not trigger the behavior, apparently the hash is created in a saver time in that scenario. This sample project offers code that will prime a static property into a service (the trick is having a command with such a class-reference/service): https://github.com/arjenm/symfony-service-recursion Add a var_dump to the `$defaults = $class->getDefaultProperties();` in \Symfony\Component\Config\Resource\ReflectionClassResource::generateSignature to see it in effect. The first cache:clear will show a few null's; the second run - without removing the cache - will show a container instead of null. Its still a tiny container, so it'll not trigger serious issues. This PR prevents an object "default value" for a property from being fed to 'print_r'. Since normally an object-property should be null on a "fresh" look at a non-instantiated class it should be save to use (i.e. no BC). Commits ------- a80e56c Don't add value of (default/static) objects to the signature
2 parents 6763172 + a80e56c commit 5c67a67

File tree

2 files changed

+15
-1
lines changed
  • Tests/Resource

    • 2 files changed

    +15
    -1
    lines changed

    ‎src/Symfony/Component/Config/Resource/ReflectionClassResource.php

    Lines changed: 1 addition & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -140,7 +140,7 @@ private function generateSignature(\ReflectionClass $class)
    140140

    141141
    foreach ($class->getProperties(\ReflectionProperty::IS_PUBLIC | \ReflectionProperty::IS_PROTECTED) as $p) {
    142142
    yield $p->getDocComment().$p;
    143-
    yield print_r(isset($defaults[$p->name]) ? $defaults[$p->name] : null, true);
    143+
    yield print_r(isset($defaults[$p->name]) && !\is_object($defaults[$p->name]) ? $defaults[$p->name] : null, true);
    144144
    }
    145145
    }
    146146

    ‎src/Symfony/Component/Config/Tests/Resource/ReflectionClassResourceTest.php

    Lines changed: 14 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -170,6 +170,15 @@ public function testServiceSubscriber()
    170170
    $res = new ReflectionClassResource(new \ReflectionClass(TestServiceSubscriber::class));
    171171
    $this->assertTrue($res->isFresh(0));
    172172
    }
    173+
    174+
    public function testIgnoresObjectsInSignature()
    175+
    {
    176+
    $res = new ReflectionClassResource(new \ReflectionClass(TestServiceWithStaticProperty::class));
    177+
    $this->assertTrue($res->isFresh(0));
    178+
    179+
    TestServiceWithStaticProperty::$initializedObject = new TestServiceWithStaticProperty();
    180+
    $this->assertTrue($res->isFresh(0));
    181+
    }
    173182
    }
    174183

    175184
    interface DummyInterface
    @@ -195,3 +204,8 @@ public static function getSubscribedServices()
    195204
    return self::$subscribedServices;
    196205
    }
    197206
    }
    207+
    208+
    class TestServiceWithStaticProperty
    209+
    {
    210+
    public static $initializedObject;
    211+
    }

    0 commit comments

    Comments
     (0)
    0