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 e9acd68

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
bug #44399 Prevent infinite nesting of lazy ObjectManager instances when ObjectManager is reset (Ocramius)
This PR was merged into the 4.4 branch. Discussion ---------- Prevent infinite nesting of lazy `ObjectManager` instances when `ObjectManager` is reset | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | 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 Commits ------- 770425c Prevent infinite nesting of lazy `ObjectManager` instances when `ObjectManager` is reset
2 parents e0cadfb + 770425c commit e9acd68
Copy full SHA for e9acd68

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.