[Cache] Fix possible infinite loop in CachePoolPass
#53593
Conversation
|
The change itself looks good. But can you please add a test case to prevent future regressions? |
49c2536 to
58212ad
Compare
@xabbuh Done, but I had to make additional changes to the code. Either I misunderstood the intention, or this code is also broken on v5.4. If that's the case, please let me know, and I'll open a separate PR for v5.4, which should then be merged first. |
| $aliasedCacheClearerId = (string) $container->getAlias('cache.global_clearer'); | ||
| $notAliasedCacheClearerId = 'cache.global_clearer'; | ||
| while ($container->hasAlias($notAliasedCacheClearerId)) { | ||
| $notAliasedCacheClearerId = (string) $container->getAlias($notAliasedCacheClearerId); |
There was a problem hiding this comment.
The $clearers array should only use non-alias names for keys since $container->getDefinition() (on line 193) doesn't work with aliases.
|
Can you please send the PR you have in mind for 5.4? |
58212ad to
9aa89aa
Compare
9aa89aa to
f971882
Compare
|
@nicolas-grekas I've retargeted this PR to 5.4. A conflict is inevitable when merging with the upstream, so IMO, it would be easiest to resolve it then. Here's the resolved code: diff --git a/src/Symfony/Component/Cache/DependencyInjection/CachePoolPass.php b/src/Symfony/Component/Cache/DependencyInjection/CachePoolPass.php
index 5055ba9918..ab212e8ee2 100644
--- a/src/Symfony/Component/Cache/DependencyInjection/CachePoolPass.php
+++ b/src/Symfony/Component/Cache/DependencyInjection/CachePoolPass.php
@@ -181,11 +181,11 @@ class CachePoolPass implements CompilerPassInterface
$container->removeDefinition('cache.early_expiration_handler');
}
- $notAliasedCacheClearerId = $aliasedCacheClearerId = 'cache.global_clearer';
- while ($container->hasAlias('cache.global_clearer')) {
- $aliasedCacheClearerId = (string) $container->getAlias('cache.global_clearer');
+ $notAliasedCacheClearerId = 'cache.global_clearer';
+ while ($container->hasAlias($notAliasedCacheClearerId)) {
+ $notAliasedCacheClearerId = (string) $container->getAlias($notAliasedCacheClearerId);
}
- if ($container->hasDefinition($aliasedCacheClearerId)) {
+ if ($container->hasDefinition($notAliasedCacheClearerId)) {
$clearers[$notAliasedCacheClearerId] = $allPools;
}If needed, I'll create a new PR for 6.3. |
src/Symfony/Component/Cache/Tests/DependencyInjection/CachePoolPassTest.php
Outdated
Show resolved
Hide resolved
f971882 to
7cd9f93
Compare
|
Thank you @HypeMC. |
Introduced in #41353.
I'm not sure what the purpose of this code is exactly, since I haven't actually run into a problem, but it's definitely wrong this way.