Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 15d6e01

Browse filesBrowse files
OcramiusOcramius
committed
Prevent infinite nesting of lazy ObjectManager instances when ObjectManager is reset
This patch ensures that the `ObjectManager` that the `Symfony\Bridge\Doctrine\ManagerRegistry` replaces during `Symfony\Bridge\Doctrine\ManagerRegistry#resetService()` operations is a fresh non-lazy service. Before this change, `Symfony\Bridge\Doctrine\ManagerRegistry#resetService()` would replace the initialization of the lazy proxy with a new service each time, but that service being lazy, this led to an additional proxy nesting level at each service reset call. That leads to general issues around memory reliability, stack trace nesting (and therefore bigger logged traces) and potentially even stack overflow problems, when running with XDebug. The problem seems to only apply when the `symfony/dependency-injection` `Container` is compiled as a set of small factory files: that's because each generated factory has a boolean flag that indicates whether a lazy or non-lazy version of a service is requested, as introduced in the original implementation at #7890
1 parent daba023 commit 15d6e01
Copy full SHA for 15d6e01

File tree

2 files changed

+94
-1
lines changed
Filter options

2 files changed

+94
-1
lines changed

‎src/Symfony/Bridge/Doctrine/ManagerRegistry.php

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/ManagerRegistry.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ function (&$wrappedInstance, LazyLoadingInterface $manager) use ($name) {
6262
$name = $this->aliases[$name];
6363
}
6464
if (isset($this->fileMap[$name])) {
65-
$wrappedInstance = $this->load($this->fileMap[$name]);
65+
$wrappedInstance = $this->load($this->fileMap[$name], false);
6666
} else {
6767
$method = $this->methodMap[$name] ?? 'get'.strtr($name, $this->underscoreMap).'Service'; // BC with DI v3.4
6868
$wrappedInstance = $this->{$method}(false);

‎src/Symfony/Bridge/Doctrine/Tests/ManagerRegistryTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Tests/ManagerRegistryTest.php
+93Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,15 @@
1212
namespace Symfony\Bridge\Doctrine\Tests;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use ProxyManager\Proxy\LazyLoadingInterface;
16+
use ProxyManager\Proxy\ValueHolderInterface;
1517
use Symfony\Bridge\Doctrine\ManagerRegistry;
18+
use Symfony\Bridge\ProxyManager\LazyProxy\PhpDumper\ProxyDumper;
1619
use Symfony\Bridge\ProxyManager\Tests\LazyProxy\Dumper\PhpDumperTest;
20+
use Symfony\Component\DependencyInjection\ContainerBuilder;
21+
use Symfony\Component\DependencyInjection\ContainerInterface;
22+
use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
23+
use Symfony\Component\Filesystem\Filesystem;
1724

1825
class ManagerRegistryTest extends TestCase
1926
{
@@ -39,6 +46,92 @@ public function testResetService()
3946
$this->assertSame($foo, $container->get('foo'));
4047
$this->assertObjectNotHasAttribute('bar', $foo);
4148
}
49+
50+
/**
51+
* When performing an entity manager lazy service reset, the reset operations may re-use the container
52+
* to create a "fresh" service: when doing so, it can happen that the "fresh" service is itself a proxy.
53+
*
54+
* Because of that, the proxy will be populated with a wrapped value that is itself a proxy: repeating
55+
* the reset operation keeps increasing this nesting until the application eventually runs into stack
56+
* overflow or memory overflow operations, which can happen for long-running processes that rely on
57+
* services that are reset very often.
58+
*/
59+
public function testResetServiceWillNotNestFurtherLazyServicesWithinEachOther()
60+
{
61+
// This test scenario only applies to containers composed as a set of generated sources
62+
$this->dumpLazyServiceProjectAsFilesServiceContainer();
63+
64+
/** @var ContainerInterface $container */
65+
$container = new \LazyServiceProjectAsFilesServiceContainer();
66+
67+
$registry = new TestManagerRegistry(
68+
'irrelevant',
69+
[],
70+
['defaultManager' => 'foo'],
71+
'irrelevant',
72+
'defaultManager',
73+
'irrelevant'
74+
);
75+
$registry->setTestContainer($container);
76+
77+
$service = $container->get('foo');
78+
79+
self::assertInstanceOf(\stdClass::class, $service);
80+
self::assertInstanceOf(LazyLoadingInterface::class, $service);
81+
self::assertInstanceOf(ValueHolderInterface::class, $service);
82+
self::assertFalse($service->isProxyInitialized());
83+
84+
$service->initializeProxy();
85+
86+
self::assertTrue($container->initialized('foo'));
87+
self::assertTrue($service->isProxyInitialized());
88+
89+
$registry->resetManager();
90+
$service->initializeProxy();
91+
92+
$wrappedValue = $service->getWrappedValueHolderValue();
93+
self::assertInstanceOf(\stdClass::class, $wrappedValue);
94+
self::assertNotInstanceOf(LazyLoadingInterface::class, $wrappedValue);
95+
self::assertNotInstanceOf(ValueHolderInterface::class, $wrappedValue);
96+
}
97+
98+
/** @return void */
99+
private function dumpLazyServiceProjectAsFilesServiceContainer()
100+
{
101+
if (class_exists(\LazyServiceProjectAsFilesServiceContainer::class, false)) {
102+
return;
103+
}
104+
105+
$container = new ContainerBuilder();
106+
107+
$container->register('foo', \stdClass::class)
108+
->setPublic(true)
109+
->setLazy(true);
110+
$container->compile();
111+
112+
$fileSystem = new Filesystem();
113+
114+
$temporaryPath = $fileSystem->tempnam(sys_get_temp_dir(), 'symfonyManagerRegistryTest');
115+
$fileSystem->remove($temporaryPath);
116+
$fileSystem->mkdir($temporaryPath);
117+
118+
$dumper = new PhpDumper($container);
119+
120+
$dumper->setProxyDumper(new ProxyDumper());
121+
$containerFiles = $dumper->dump([
122+
'class' => 'LazyServiceProjectAsFilesServiceContainer',
123+
'as_files' => true,
124+
]);
125+
126+
array_walk(
127+
$containerFiles,
128+
static function (string $containerSources, string $fileName) use ($temporaryPath): void {
129+
(new Filesystem())->dumpFile($temporaryPath . '/'. $fileName, $containerSources);
130+
}
131+
);
132+
133+
require $temporaryPath . '/LazyServiceProjectAsFilesServiceContainer.php';
134+
}
42135
}
43136

44137
class TestManagerRegistry extends ManagerRegistry

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.