Skip to content

Commit 4afc290

Browse files
SanderMullerclaude
authored andcommitted
Cache a class's ancestors without the class itself
getAncestors() cached an array that contained $this, so every ClassReflection that had answered it referenced itself. PHPStan runs with gc_disable(), so a ClassReflection the reflection provider does not keep, like a withTypes() copy, then stayed in memory until the process ended, together with its template type map. The cache now leaves the class out, and getAncestors() adds it back first, so the returned array is the same. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent c7c28d4 commit 4afc290

2 files changed

Lines changed: 38 additions & 4 deletions

File tree

‎src/Reflection/ClassReflection.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2213,18 +2213,20 @@ public function getTemplateTags(): array
22132213
*/
22142214
public function getAncestors(): array
22152215
{
2216-
$ancestors = $this->ancestors;
2217-
2218-
if ($ancestors === null) {
2216+
if ($this->ancestors === null) {
22192217
$ancestors = [
22202218
$this->getName() => $this,
22212219
];
22222220
$this->collectAncestors($ancestors);
22232221

2222+
// the cached array must not contain $this: that would be a reference
2223+
// cycle, and with gc_disable() a ClassReflection the reflection provider
2224+
// does not keep, like a withTypes() copy, would then never be freed
2225+
unset($ancestors[$this->getName()]);
22242226
$this->ancestors = $ancestors;
22252227
}
22262228

2227-
return $ancestors;
2229+
return [$this->getName() => $this] + $this->ancestors;
22282230
}
22292231

22302232
/**

‎tests/PHPStan/Reflection/ClassReflectionTest.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Reflection;
44

5+
use ArrayObject;
56
use Attribute;
67
use Attributes\IsAttribute;
78
use Attributes\IsAttribute2;
@@ -31,13 +32,19 @@
3132
use PHPStan\Testing\PHPStanTestCase;
3233
use PHPStan\Testing\RuleTestCase;
3334
use PHPStan\Type\IntegerType;
35+
use PHPStan\Type\StringType;
3436
use PHPUnit\Framework\Attributes\DataProvider;
3537
use PHPUnit\Framework\Attributes\RequiresPhp;
3638
use PHPUnit\Framework\TestCase;
3739
use ReflectionClass;
40+
use WeakReference;
3841
use WrongClassConstantFile\SecuredRouter;
42+
use function array_key_first;
3943
use function array_map;
4044
use function array_values;
45+
use function gc_disable;
46+
use function gc_enable;
47+
use function gc_enabled;
4148

4249
class ClassReflectionTest extends PHPStanTestCase
4350
{
@@ -375,4 +382,29 @@ public function testDeprecatedAttribute(string $className, bool $expected): void
375382
$this->assertSame($expected, $classReflection->isDeprecated());
376383
}
377384

385+
public function testAncestorsDoNotKeepClassReflectionAlive(): void
386+
{
387+
$reflectionProvider = self::createReflectionProvider();
388+
$classReflection = $reflectionProvider->getClass(ArrayObject::class)->withTypes([new IntegerType(), new StringType()]);
389+
390+
$ancestors = $classReflection->getAncestors();
391+
$this->assertSame(ArrayObject::class, array_key_first($ancestors));
392+
$this->assertSame($classReflection, $ancestors[ArrayObject::class]);
393+
$this->assertSame($ancestors, $classReflection->getAncestors());
394+
unset($ancestors);
395+
396+
// PHPStan runs with gc_disable(), so a reference cycle would keep it alive
397+
$gcEnabled = gc_enabled();
398+
gc_disable();
399+
try {
400+
$weakReference = WeakReference::create($classReflection);
401+
unset($classReflection);
402+
$this->assertNull($weakReference->get());
403+
} finally {
404+
if ($gcEnabled) {
405+
gc_enable();
406+
}
407+
}
408+
}
409+
378410
}

0 commit comments

Comments
 (0)